Rollback db transaction when background task is interrupted #2019
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jonahkagan
force-pushed
the
jonah/fix-task-reset-rollback
branch
from
12b7819 to
5a2686b
Compare
jonahkagan
commented
Oct 28, 2024
| # Each task should have run exactly once | ||
| assert sorted(results) == expected_sorted_results | ||
| # Tasks should not have run in order, since some got interrupted and reset | ||
| assert results != expected_sorted_results |
There was a problem hiding this comment.
While I like what this assertion is trying to test, it relies on non-deterministic timing for the tasks to get executed out of order. I couldn't think of an easy way to make this work deterministically, so to prevent flaky tests I'm removing it.
Previously, when a task got interrupted, we let the process die immediately, which would rollback the transaction automatically. Now, we try to reset the task, which involves clearing the task started_at timestamp and then commiting the current transaction to save that change to the db. So in order to ensure that any in-progress changes from the task itself are not committed alongside those changes, we need to rollback the current transaction and start a new one. I updated the tests for task resetting to cover this behavior, which they previously didn't cover because they didn't use the db.
jonahkagan
force-pushed
the
jonah/fix-task-reset-rollback
branch
from
5a2686b to
92c7a6a
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Bugfix for #1990
Previously, when a task got interrupted, we let the process die immediately, which would rollback the transaction automatically.
Now, we try to reset the task, which involves clearing the task started_at timestamp and then commiting the current transaction to save that change to the db. So in order to ensure that any in-progress changes from the task itself are not committed alongside those changes, we need to rollback the current transaction and start a new one.
I updated the tests for task resetting to cover this behavior, which they previously didn't cover because they didn't use the db.