Fix flaky SharedTsBlockQueue concurrency test#18244
Open
Caideyipi wants to merge 1 commit into
Open
Conversation
JackieTien97
left a comment
Contributor
There was a problem hiding this comment.
One non-blocking suggestion to improve failure diagnostics.
| return null; | ||
| }); | ||
|
|
||
| sender.get(30, TimeUnit.SECONDS); |
Contributor
There was a problem hiding this comment.
Could we wait for whichever worker completes first here? If the receiver fails before the sender finishes, it stops freeing queue memory; the sender can then block once the pool fills, so this Future.get call reports a 30-second TimeoutException and receiver.get is never reached. That hides the receiver's actual exception, even though this change aims to propagate worker failures. Using an ExecutorCompletionService with a shared deadline (and cancelling the sibling task on failure) would make this fail fast and preserve the root cause.
This file contains hidden or 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
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.
Description
SharedTsBlockQueueTest.concurrencyTestused a fixed JUnit timeout while polling counters, printing progress, and recursively scheduling listener tasks. Under CI load, the test could time out without surfacing worker failures, and its executor was never shut down.This change makes the test deterministic by:
Future.getTesting
mvn -o -nsu -Ddevelocity.off=true spotless:check -pl iotdb-core/datanodeSharedTsBlockQueueTestdirectly: 2 tests passedA full reactor test attempt was blocked before reaching DataNode by an existing generated Thrift compilation error:
TaskType.javacould not resolvejavax.annotation.