Skip to content

Conversation

@donoghuc
Copy link
Contributor

@donoghuc donoghuc commented Oct 20, 2025

Local compaction depends on completion order and timing. Under certain cases node count can reach or exceed sqrt(concurrency) before the final compaction is triggered. The test for intermediate state was failing fairly often in CI. This commit updates the test for intermediate state to simply be less than concurrency. This should satisfy the intent of the test, with the added security of the rest of the test asserting the finalized state is less than 2 nodes.

Closes #200

Local compaction depends on completion order and timing. Under certain cases
node count can reach or exceed `sqrt(concurrency)` before the final compaction
is triggered. The test for intermediate state was failing fairly often in CI.
This commit updates the test for intermediate state to simply be less than
`concurrency`. This should satisfy the intent of the test, with the added
security of the rest of the test asserting the finalized state is less than 2
nodes.
@donoghuc donoghuc requested a review from yaauie October 20, 2025 22:13
@donoghuc
Copy link
Contributor Author

Based on my understanding of #186 this is a reasonable change. Though i'm not exactly sure why the original limit was introduced. In CI (and very rarely locally) i was able to repro it where its exactly 10.

Copy link
Contributor

@yaauie yaauie left a comment

Choose a reason for hiding this comment

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

These changes look sensible.

I've added a suggestion to clarify the comment for the section. I can't figure out why my original constraint was as tight as it was, since the implementation never made any relevant guarantees.

@donoghuc donoghuc merged commit a0b6638 into logstash-plugins:main Oct 22, 2025
3 checks passed
@donoghuc donoghuc mentioned this pull request Oct 22, 2025
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.

Triage flakey tests

2 participants