Skip to content

fix: Prevent deadlock in transformer#2130

Merged
przste-go merged 1 commit intomainfrom
fix-transformer-deadlock
Apr 28, 2025
Merged

fix: Prevent deadlock in transformer#2130
przste-go merged 1 commit intomainfrom
fix-transformer-deadlock

Conversation

@przste-go
Copy link
Copy Markdown

@przste-go przste-go commented Apr 25, 2025

Summary

Currently sync method has 3 gorutines running in single errorGroup let's call them G1,G2 and G3
G1-

	eg.Go(func() error {
   	if err := s.Plugin.Transform(gctx, recvRecords, sendRecords); err != nil {
   		return status.Error(codes.Internal, err.Error())
   	}
   	return nil
   })

runs the transformer logic which consumes records from recvRecords and pushes them to sendRecords both of which are unbuffered channels. Gorutine g2 consumes record produced by G1

record := range sendRecords
(..)

when gorutine g2 returns an error, code executing in G1 may hang indefinietly trying to write into channel that has no associated error.


Use the following steps to ensure your PR is ready to be reviewed

  • Read the contribution guidelines 🧑‍🎓
  • Run go fmt to format your code 🖊
  • Lint your changes via golangci-lint run 🚨 (install golangci-lint here)
  • Update or add tests 🧪
  • Ensure the status checks below are successful ✅

@github-actions github-actions bot added fix and removed fix labels Apr 25, 2025
@przste-go przste-go changed the title fix: Prevent deadlock in trasnformer fix: Prevent deadlock in transformer Apr 28, 2025
@github-actions github-actions bot added fix and removed fix labels Apr 28, 2025
@przste-go przste-go marked this pull request as ready for review April 28, 2025 08:13
@przste-go przste-go requested review from a team and murarustefaan April 28, 2025 08:13
@murarustefaan murarustefaan requested a review from erezrokah April 28, 2025 08:35
Copy link
Copy Markdown
Member

@murarustefaan murarustefaan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job on catching this!

@przste-go przste-go merged commit a65b101 into main Apr 28, 2025
16 checks passed
@przste-go przste-go deleted the fix-transformer-deadlock branch April 28, 2025 09:18
przste-go pushed a commit that referenced this pull request Apr 28, 2025
#### Summary

<!-- 🎉 Thank you for making CloudQuery awesome by submitting a PR 🎉 -->
Following up on the transformer hanging issue reported by a customer and
addressed in PR
[#2130](#2130):
Currently, the transformer plugin can increase the message size beyond
the limit enforced in the sync method of plugin-sdk
[1](https://github.com/cloudquery/plugin-sdk/blob/bc50fd3d2be414edba8f8ad5bb7739a012840bf1/internal/servers/plugin/v3/plugin.go#L267).
To accommodate scenarios where transformers or destination plugins need
to handle larger messages than the source plugin, we should consider
increasing this limit to provide more flexibility.

<!--
Explain what problem this PR addresses
-->

---

Use the following steps to ensure your PR is ready to be reviewed

- [ ] Read the [contribution guidelines](../blob/main/CONTRIBUTING.md)
🧑‍🎓
- [ ] Run `go fmt` to format your code 🖊
- [ ] Lint your changes via `golangci-lint run` 🚨 (install golangci-lint
[here](https://golangci-lint.run/usage/install/#local-installation))
- [ ] Update or add tests 🧪
- [ ] Ensure the status checks below are successful ✅

---------

Co-authored-by: Bartosz Lesniewski <bartosz@cloudquery.io>
kodiakhq bot pushed a commit that referenced this pull request Apr 28, 2025
🤖 I have created a release *beep* *boop*
---


## [4.79.0](v4.78.0...v4.79.0) (2025-04-28)


### Features

* Add transformer to update table description with its table options ([#2128](#2128)) ([2387b57](2387b57))
* Show plugin version in plugin server logs ([#2124](#2124)) ([be08606](be08606))


### Bug Fixes

* **deps:** Update module github.com/cloudquery/plugin-pb-go to v1.26.10 ([#2132](#2132)) ([775d537](775d537))
* Prevent deadlock in transformer ([#2130](#2130)) ([a65b101](a65b101))

---
This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants