Repository navigation
Ensure worker re-initializes after startup failure - #1804
Conversation
Move startingClientVersion assignment after enter() succeeds in MigrationStateMachineImpl.initialize(). Previously, startingClientVersion was set before enter(), so if enter() threw a DependencyException (e.g. GSI creation timeout), the retry path saw a non-null startingClientVersion and skipped re-initialization entirely. The worker entered a zombie state — holding and renewing leases but not processing records. Also add an idempotency guard in initializeStartupComponents() to prevent duplicate thread creation if the method is called again after a successful first run. Revert GSI timeout from 3600s workaround to the original 600s now that the root cause is fixed. Includes 6 new unit tests covering retry behavior for all client version paths (3x, upgradeFrom2x, phase1, 3xWithRollback).
| // Extending the timeout to a long time to ensure GSI is created as a short term fix and will revisit | ||
| // on the proper fix in the future | ||
| final long timeoutSeconds = 3600L; | ||
| final long timeoutSeconds = 600L; |
There was a problem hiding this comment.
How long does GSI initialization take? From my testing previously, a 4 shard new application took 15-20 mins to initialize the GSI, so we should probably just set the timeout to longer than that to avoid unnecessary retries. We can reduce it though
There was a problem hiding this comment.
I just reverted to what we had initially. For me it took about same time 15-20 minutes. I read https://docs.aws.amazon.com/amazondynamodb/latest/developerguide/GSI.OnlineOps.html, GSI creation time depends on few factors like size of table, number of items in the table, etc.
I agree with your point though. I will change it back to 3600L
| startingState.enter(ClientVersion.CLIENT_VERSION_INIT); | ||
|
|
||
| // Only set after enter() succeeds — this is the guard that prevents | ||
| // double-initialization. Setting it before enter() caused the zombie bug. |
There was a problem hiding this comment.
Do we need the second sentence comment?
There was a problem hiding this comment.
I can clean the comment
ehasah-aws
changed the title
Issue #, if available:
Description of changes:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.