Skip to content

Conversation

Dream95
Copy link

@Dream95 Dream95 commented Oct 8, 2025

Fixes #24788

Motivation

The test is flaky.

Reproduce the issue:
Add Thread.sleep after sendAsync to avoid batching sends.

 for (int i = 0; i < 50; i++) {
       producer.newMessage(transaction).value(i).sendAsync();
       Thread.sleep(1);
}

The issue occurs because the in this test, the TransactionBufferTestImpl class extends TopicTransactionBuffer and declares a field with the same name publishFuture.
Therefore, in the appendBufferToTxn method, getPublishFuture() retrieves the publishFuture from the subclass, while the assignment publishFuture = future modifies the publishFuture belonging to the parent class.

 CompletableFuture<Position> future = getPublishFuture().thenCompose(ignore -> {
            ......
        }).whenComplete(((position, throwable) -> buffer.release()));
publishFuture = future;

Modifications

Use ‘setPublishFuture(future)’ to replace ‘publishFuture = future’

Verifying this change

  • Make sure that the change passes the CI checks.

This change is already covered by existing tests.

Does this pull request potentially affect one of the following parts:

  • Dependencies (add or upgrade a dependency)
  • The public API
  • The schema
  • The default values of configurations
  • The threading model
  • The binary protocol
  • The REST endpoints
  • The admin CLI options
  • The metrics
  • Anything that affects deployment

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

Matching PR in forked repository

PR in forked repository: Dream95#1

@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Oct 8, 2025
Copy link
Contributor

@Technoboy- Technoboy- left a comment

Choose a reason for hiding this comment

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

Please also delete the useless field publishFuture, transactionBufferFuture defined in the TransactionBufferTestImpl

@Technoboy- Technoboy- added this to the 4.2.0 milestone Oct 9, 2025
@Dream95 Dream95 force-pushed the fix-flaky-test-24788 branch from 78c24a0 to f571cc6 Compare October 9, 2025 14:09
@Dream95 Dream95 requested a review from Technoboy- October 10, 2025 08:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

doc-not-needed Your PR changes do not impact docs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Flaky-test: TopicTransactionBufferTest.testMessagePublishInOrder

2 participants