Skip to content

Conversation

trinity-1686a
Copy link
Contributor

Description

fix #5588

How was this PR tested?

added a test reproducing what was failing before, and making sure it passes now

@trinity-1686a
Copy link
Contributor Author

looks like this made actors::merge_executor::tests::test_delete_all very flaky. I suspect this is because when no split is published, the merge might not get acknowledged properly to the Tracker

@guilload guilload requested a review from fulmicoton January 17, 2025 18:43
Copy link
Collaborator

@fulmicoton fulmicoton left a comment

Choose a reason for hiding this comment

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

can you squash the commits and have a good commit message describing your solution?

@trinity-1686a
Copy link
Contributor Author

currently this makes one test very flaky (fails more often than not) and i haven't figured out why yet. i'd rather fix that before merging

@@ -88,7 +89,7 @@ pub struct DeleteTaskPlanner {
/// a merge operation is dropped after the publish of the split that underwent
/// the delete operation.
/// The inventory is used to avoid sending twice the same delete operation.
ongoing_delete_operations_inventory: Inventory<MergeOperation>,
ongoing_delete_operations_inventory: Inventory<RecordUnacknoledgedDrop<MergeOperation>>,
Copy link
Collaborator

Choose a reason for hiding this comment

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

typo.

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.

Same merge planned twice
2 participants