Skip to content

Yield updated instead of initial nested cursor to persist#366

Merged
Mangara merged 1 commit into
Shopify:mainfrom
bdewater:nested-iteration-v4
Mar 30, 2023
Merged

Yield updated instead of initial nested cursor to persist#366
Mangara merged 1 commit into
Shopify:mainfrom
bdewater:nested-iteration-v4

Conversation

@bdewater
Copy link
Copy Markdown
Contributor

@bdewater bdewater commented Mar 29, 2023

Otherwise we reprocess the last successfully processed object again when resuming. In a pathologic case, if a nested job is interrupted every time after a single each_iteration completes we'd never progress: the cursor would stay at [nil, nil].

I spelled it out in #364 😅 but I originally worked on this months before and only realized it now.

Follow-up to #310

Otherwise we reprocess the last succesfully processed object again when resuming.

In a pathologic case, if a nested job is interrupted every time after a single `each_iteration` completes we'd never progress: the cursor would stay at [nil, nil].
# reset cursor at the index of the nested enumerator that just finished, so we don't skip items when that
# index is reused in the next nested iteration
@cursors[next_index] = nil
@cursors[index] = cursor_from_enumerator
Copy link
Copy Markdown
Contributor Author

@bdewater bdewater Mar 29, 2023

Choose a reason for hiding this comment

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

This change is not necessary to make the tests pass, but it felt more correct/clear inside the else block - it's analogous to how iterate_with_enumerator updates the cursor here:

each_iteration(object_from_enumerator, *arguments)
self.cursor_position = cursor_from_enumerator

Copy link
Copy Markdown
Contributor

@Mangara Mangara left a comment

Choose a reason for hiding this comment

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

Good catch! I like that we can remove the dup from the instrumented tags too.

@Mangara Mangara merged commit a8422fa into Shopify:main Mar 30, 2023
@bdewater bdewater deleted the nested-iteration-v4 branch March 30, 2023 14:18
@lavoiesl lavoiesl mentioned this pull request Aug 15, 2023
@shopify-shipit shopify-shipit Bot temporarily deployed to rubygems August 23, 2023 19:41 Inactive
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.

2 participants