Skip to content

Conversation

@grodowski
Copy link
Member

@grodowski grodowski commented Feb 18, 2025

Issue link: github#1499 and associated upstream PR: github#1507.

@grodowski grodowski force-pushed the grodowski/coding-chimp/on-retry-hook branch from e98fb12 to e018cc3 Compare April 10, 2025 11:26
grodowski and others added 6 commits April 10, 2025 13:31
CalculateNextIterationRangeEndValues needs to be recomputed on every retry in case of configuration (e.g. chunk-size)
changes were made by onBatchCopyRetry hooks.
…e insert completes

- extract MigrationContext.SetNextIterationRangeValues outside of applyCopyRowsFunc, so that it doesn't run on retries
- add an integration test for Migrator with retry hooks

Co-authored-by: Bastian Bartmann <[email protected]>
@grodowski grodowski force-pushed the grodowski/coding-chimp/on-retry-hook branch from e018cc3 to a48cae1 Compare April 10, 2025 11:32
@grodowski grodowski changed the base branch from master to shopify May 13, 2025 11:19
@grodowski grodowski force-pushed the shopify branch 9 times, most recently from a5224bd to d74db84 Compare May 15, 2025 09:35
@grodowski grodowski changed the base branch from shopify to master May 15, 2025 09:42
@grodowski grodowski marked this pull request as ready for review May 15, 2025 09:42
@grodowski grodowski changed the title OnRetry hook Create a hook to capture copy batch errors and retries May 15, 2025
Copy link

@riccardo-casazza riccardo-casazza left a comment

Choose a reason for hiding this comment

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

If we reach the chunk size = 10 (the minimum supported by gh-ost), the schema migrations fails definitely, right?

@grodowski
Copy link
Member Author

Yep, retryBatchCopyWithHooks errors after retries are exhausted

@grodowski grodowski merged commit 9cccf86 into master May 16, 2025
6 checks passed
grodowski added a commit that referenced this pull request Nov 4, 2025
* Execute hook on every batch insert retry

Co-authored-by: Bastian Bartmann <[email protected]>

* Expose the last error message to the onBatchCopyRetry hook

Co-authored-by: Bastian Bartmann <[email protected]>

* Remove double retries

CalculateNextIterationRangeEndValues needs to be recomputed on every retry in case of configuration (e.g. chunk-size)
changes were made by onBatchCopyRetry hooks.

* include dev.yml (temp for Shopify)

* Update doc/hooks.md

* Remove dev.yml

* Fix retry issue where MigrationIterationRangeMinValues advances before insert completes

- extract MigrationContext.SetNextIterationRangeValues outside of applyCopyRowsFunc, so that it doesn't run on retries
- add an integration test for Migrator with retry hooks

Co-authored-by: Bastian Bartmann <[email protected]>

* Add localtest that expects gh-ost to fail on exhausted retries

* Rename method

---------

Co-authored-by: Bastian Bartmann <[email protected]>
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