add option for specifying timeout in minutes and gtid position together with last known pk #139
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
maxenglander
force-pushed
the
maxeng-source-opt-read-duration
branch
from
b1c28a4 to
d55dac4
Compare
Signed-off-by: Max Englander <max@planetscale.com>
maxenglander
force-pushed
the
maxeng-source-opt-read-duration
branch
from
d55dac4 to
dae88cc
Compare
Signed-off-by: Max Englander <max@planetscale.com>
| table := s.Stream | ||
| readDuration := 5 * time.Minute | ||
| timeout := 5 * time.Minute | ||
| if timeoutSeconds := ps.TimeoutSeconds; timeoutSeconds != nil { |
There was a problem hiding this comment.
The current implementation does not enforce the minimum timeout of 5 minutes as required. Consider adding a condition to set the timeout to 5 minutes if the provided value is less than that.
Collaborator
Author
There was a problem hiding this comment.
no thanks, it's useful to be able to use values less than 5m in testing
nickvanw
approved these changes
Apr 7, 2025
Currently when airbyte-source syncs it wipes out `gtid` information from the shard gtid before passing it along in the VStream request. This means that, upon retry, it's possible for the connector to lose rows that are inserted between the `gtid` of the last `vgtid` event it received in one attempt and the `@@gtid_executed` of the next sync attempt. This new option changes the current behavior so that `gtid` is always passed along in the VStream request, if it is available. This makes for a safer retry. However, it does mean that it is possible for the connector to received duplicates of the same row. How it will look in Airbyte UI after upgrading to a version with this change. <img width="849" alt="image" src="https://github.com/user-attachments/assets/5d699e82-3e37-4459-a833-365f717cea15" /> Signed-off-by: Max Englander <max@planetscale.com>
maxenglander
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add two new options.
Specifying timeout in minutes
Make the current hardcoded 5 minute read duration configurable. Keep 5 minutes as the default and minimum. This is how it will look on connectors that upgrade to a version containing this change:
GTID position with last known PK
Currently when airbyte-source syncs it wipes out
gtidinformation from the shard gtid before passing it along in the VStream request. This means that, upon retry, it's possible for the connector to lose rows that are inserted between thegtidof the lastvgtidevent it received in one attempt and the@@gtid_executedof the next sync attempt.This new option changes the current behavior so that
gtidis always passed along in the VStream request, if it is available. This makes for a safer retry. However, it does mean that it is possible for the connector to received duplicates of the same row.How it will look in Airbyte UI after upgrading to a version with this change.