Skip to content
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

Do not process batches if skip file detected in restore_helper #103

Merged
merged 23 commits into from
Sep 9, 2024

Conversation

dkovalev1
Copy link

@dkovalev1 dkovalev1 commented Aug 9, 2024

Do not process batches if skip file detected in restore_helper.

When performing restore with resize, batches are used for each table. If the
error happens during restore, there is no need to continue with remaining
batches for this particular table. To signal about an error to the helpers,
main process created skip files.

This patch adds skip of the remaining batches in restore helper when skip file
is detected.

@whitehawk

This comment was marked as resolved.

@dkovalev1
Copy link
Author

Please update the description to none-draft grade

ok

@whitehawk

This comment was marked as resolved.

helper/restore_helper.go Outdated Show resolved Hide resolved
@RekGRpth

This comment was marked as resolved.

@dkovalev1
Copy link
Author

How about to restore loop over batches (as it was before)?

Before that commit there were different lists of oids and batches, so loop in the loop looked natural. Now there is combined oidWithBatchList and IMHO one loop is looking much more relevant.

@RekGRpth

This comment was marked as resolved.

@dkovalev1
Copy link
Author

Removing oidWithBatchList would bring significant changes into the code and unrelated to the current issue tests. The code will not be smaller or more clean since replicated tables should be processed as well.
The problem with extra create/close of pipes can be achieved with much easier way.

helper/restore_helper.go Show resolved Hide resolved
helper/restore_helper.go Outdated Show resolved Hide resolved
helper/restore_helper.go Show resolved Hide resolved
helper/restore_helper.go Show resolved Hide resolved
integration/helper_test.go Outdated Show resolved Hide resolved
integration/helper_test.go Outdated Show resolved Hide resolved
integration/helper_test.go Show resolved Hide resolved
@RekGRpth

This comment was marked as outdated.

integration/helper_test.go Outdated Show resolved Hide resolved
integration/helper_test.go Outdated Show resolved Hide resolved
integration/helper_test.go Outdated Show resolved Hide resolved
integration/helper_test.go Outdated Show resolved Hide resolved
integration/helper_test.go Outdated Show resolved Hide resolved
integration/helper_test.go Outdated Show resolved Hide resolved
Co-authored-by: Georgy Shelkovy <[email protected]>
integration/helper_test.go Outdated Show resolved Hide resolved
Co-authored-by: Georgy Shelkovy <[email protected]>
@dkovalev1 dkovalev1 merged commit ae78091 into master Sep 9, 2024
2 checks passed
@dkovalev1 dkovalev1 deleted the ADBDEV-6012 branch September 9, 2024 10:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants