-
Notifications
You must be signed in to change notification settings - Fork 12
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
Fix: Import data not considering the initiated cutover during resumption #2369
base: main
Are you sure you want to change the base?
Conversation
…ether cutover detected by importer and use it
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! @sanyamsinghal can you also pls take a look. Since this is a critical part, want two sets of eyes on this!
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.
Assuming we have verified the fix - resumability for all three cases(normal, FF, FB)
CutoverToSourceProcessedBySourceImporter bool `json:"CutoverToSourceProcessedBySourceImporter"` | ||
|
||
//All the cutover detectedc flags by importer when they processed the cutover event |
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.
what does detected mean here? does it mean that cutover is complete?
I think that is not the case. processed flags are those.
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.
please make the comments a bit more descriptive in that case.
Minor nit: order of variables here
- Requested
- Detected
- Processed
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.
what does detected mean here? does it mean that cutover is complete?
It means that the cutover is detected by the importer command by receiving the cutover event in the queue.
return err | ||
} | ||
if ok { | ||
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.
add a log line before return
Describe the changes in this pull request
Fix - During the resumption of import data, check if the cutover is initiated before continuing the streaming. If cutover is initiated, but all the events are not processed by importer yet, on resumption, need to check if cutover event is processed by importer or not (reading the last processed queue segment by that importer and checking the last Event in that segment)
Describe if there are any user-facing changes
In resumption case of import data, it will only continue in the case there are some events left to ingest and cutover is not initiated.
How was this pull request tested?
Manually tested all the three importers (to target, to source, and to source-replica)
Does your PR have changes that can cause upgrade issues?