-
Notifications
You must be signed in to change notification settings - Fork 26
feat: Allow sync to be cancelled when in progress #1334
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
In some cases, when the resource channel is processing a large collection of objects, the sync process can continue even if the context cancel has been triggered. This is problematic for billing and usage since a sync can significantly overshoot the remaining rows quota. To allow an in-progress sync to abort quicker, we need to modify a number of channel patterns from using `range` to using `select` - which will allow the `context.Done` channel to be monitored. fixes: cloudquery/cloudquery-issues#750
scheduler/scheduler.go
Outdated
| select { | ||
| case res <- &message.SyncInsert{Record: resourceToRecord(resource)}: | ||
| case <-ctx.Done(): | ||
| return nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not return ctx.Err()?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No good reason, I'll look at doing that 🙏
⏱️ Benchmark results
|
hermanschaaf
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
🤖 I have created a release *beep* *boop* --- ## [4.17.0](v4.16.1...v4.17.0) (2023-10-30) ### Features * Add IsPaid flag to table definition ([#1327](#1327)) ([ffd14bf](ffd14bf)) * Add OnBeforeSend hook ([#1325](#1325)) ([023ebbc](023ebbc)) * Adding a batch updater to allow usage updates to be batched ([#1326](#1326)) ([0301ed7](0301ed7)) * Adding quota monitoring for premium plugins ([#1333](#1333)) ([b7a2ca5](b7a2ca5)) * Allow sync to be cancelled when in progress ([#1334](#1334)) ([6d7be0b](6d7be0b)) ### Bug Fixes * **deps:** Update github.com/cloudquery/arrow/go/v14 digest to 50d3871 ([#1337](#1337)) ([f15a89d](f15a89d)) * **deps:** Update github.com/cloudquery/arrow/go/v14 digest to f46436f ([#1329](#1329)) ([ee24384](ee24384)) * **deps:** Update module github.com/cloudquery/cloudquery-api-go to v1.4.2 ([#1335](#1335)) ([2ecd2a1](2ecd2a1)) * **deps:** Update module github.com/cloudquery/plugin-pb-go to v1.13.0 ([#1332](#1332)) ([5553f85](5553f85)) * **deps:** Update module github.com/cloudquery/plugin-pb-go to v1.13.1 ([#1336](#1336)) ([b782ee7](b782ee7)) * **deps:** Update module google.golang.org/grpc to v1.58.3 [SECURITY] ([#1331](#1331)) ([43f60c2](43f60c2)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
In some cases, when the resource channel is processing a large collection of objects, the sync process can continue even if the context cancel has been triggered. This is problematic for billing and usage since a sync can significantly overshoot the remaining rows quota. To allow an in-progress sync to abort quicker, we need to modify a number of channel patterns from using
rangeto usingselect- which will allow thecontext.Donechannel to be monitored.This PR also adds the
OnSyncFinisherhook which is called when the sync has finished adding messages. This allows the client code to call the batch updater close method without missing any updates.fixes: https://github.com/cloudquery/cloudquery-issues/issues/750