-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Remove CausalOrdering
from task-run-recorder
#15244
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That test makes this easy to trust!
record_task_run_event_mock.assert_awaited_with(event) | ||
@pytest.mark.parametrize( | ||
"event_order", | ||
list(permutations(["PENDING", "RUNNING", "COMPLETED"])), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔥
@pytest.mark.parametrize( | ||
"event_order", | ||
list(permutations(["PENDING", "RUNNING", "COMPLETED"])), | ||
ids=lambda x: "->".join(x), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❤️
CodSpeed Performance ReportMerging #15244 will not alter performanceComparing Summary
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
niiiiice thank you!
Related to ongoing investigations in #15153 |
The
task-run-recorder
consumer takes in task run events, converts them into task runs and task run states, and inserts them into the database. Previously we ensured that these inserts are forced to be done in order of the events occurring through theCausalOrdering
.In practice, the only benefit is that if someone were to observe the actual task run state inserts, a
COMPLETED
row would never exist before aRUNNING
row. Additionally as long as we are using a queue that is always ordered (currently the only option is an in memory queue), events will never come out of order. Additionally the final state of the task run will always be reported correctly.This PR removes the ordering as there is no real benefit and it is likely adding unnecessary strain to the server.