Skip to content

[fix][test] Add timeout to initial receives in ResendRequestTest.testSharedSingleAckedPartitionedTopic - #25828

Merged
lhotari merged 1 commit into
apache:masterfrom
merlimat:mmerli/fix-resend-request-test
May 20, 2026
Merged

[fix][test] Add timeout to initial receives in ResendRequestTest.testSharedSingleAckedPartitionedTopic#25828
lhotari merged 1 commit into
apache:masterfrom
merlimat:mmerli/fix-resend-request-test

Conversation

@merlimat

Copy link
Copy Markdown
Contributor

Motivation

ResendRequestTest.testSharedSingleAckedPartitionedTopic creates a partitioned topic with 3 partitions, produces 10 messages via RoundRobinPartition (so ~3-4 per partition), and subscribes two consumers with a Shared subscription and receiverQueueSize=7. Because the receive queue is larger than the per-partition message count, the broker can dispatch every message to whichever consumer connected first, leaving the other consumer's queue empty.

The test then calls the two initial receives without a timeout:

Message<byte[]> message1 = consumer1.receive();
Message<byte[]> message2 = consumer2.receive();

If consumer2 got nothing, that second call blocks forever and the test trips the 60s timeOut:

org.testng.internal.thread.ThreadTimeoutException: Method ... didn't finish within the time-out 60000
    ...
    at GrowableArrayBlockingQueue.take(GrowableArrayBlockingQueue.java:200)
    at MultiTopicsConsumerImpl.internalReceive(MultiTopicsConsumerImpl.java:388)
    at ConsumerBase.receive(ConsumerBase.java:282)
    at ResendRequestTest.testSharedSingleAckedPartitionedTopic(ResendRequestTest.java:517)

Example failure: https://scans.gradle.com/s/by57j4rphce62/tests/task/:pulsar-broker:test/details/org.apache.pulsar.broker.service.ResendRequestTest/testSharedSingleAckedPartitionedTopic/2/output

Modifications

Use timeouts on the initial receives, matching the loop below them. The assertEquals(messageCount1 + messageCount2, totalMessages) check at the end of the receive loop still verifies that both consumers together got all 10 messages, so coverage isn't lost by tolerating a null on the very first receive.

Verifying this change

This change is already covered by ResendRequestTest.testSharedSingleAckedPartitionedTopic. Locally I ran 5 times with --rerun-tasks; all passed.

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

…SharedSingleAckedPartitionedTopic

The test creates a partitioned topic with 3 partitions, produces 10
messages via RoundRobinPartition (so ~3-4 per partition), and subscribes
two consumers with a Shared subscription and receiverQueueSize=7. Because
the receive queue is larger than the per-partition message count, the
broker can dispatch every message to whichever consumer connected first,
leaving the other consumer's queue empty.

The test then calls consumer1.receive() followed by consumer2.receive()
with no timeout. If consumer2 got nothing, that second call blocks
forever and the test times out:

  org.testng.internal.thread.ThreadTimeoutException: Method ... didn't finish within the time-out 60000
      ...
      at GrowableArrayBlockingQueue.take(GrowableArrayBlockingQueue.java:200)
      at MultiTopicsConsumerImpl.internalReceive(MultiTopicsConsumerImpl.java:388)
      at ConsumerBase.receive(ConsumerBase.java:282)
      at ResendRequestTest.testSharedSingleAckedPartitionedTopic(ResendRequestTest.java:517)

Use timeouts on the initial receives like the loop below does. The
totalMessages assertion at the end of the receive loop still verifies
that both consumers together got all 10 messages, so we don't lose
coverage by allowing a null on the first receive.

Example failure:
https://scans.gradle.com/s/by57j4rphce62/tests/task/:pulsar-broker:test/details/org.apache.pulsar.broker.service.ResendRequestTest/testSharedSingleAckedPartitionedTopic/2/output
@lhotari
lhotari merged commit 55e6022 into apache:master May 20, 2026
42 of 43 checks passed
@merlimat
merlimat deleted the mmerli/fix-resend-request-test branch May 20, 2026 08:06
@lhotari lhotari added this to the 5.0.0-M1 milestone May 22, 2026
lhotari pushed a commit that referenced this pull request May 22, 2026
…SharedSingleAckedPartitionedTopic (#25828)

(cherry picked from commit 55e6022)
lhotari pushed a commit that referenced this pull request May 22, 2026
…SharedSingleAckedPartitionedTopic (#25828)

(cherry picked from commit 55e6022)
nodece pushed a commit to ascentstream/pulsar that referenced this pull request May 27, 2026
…SharedSingleAckedPartitionedTopic (apache#25828)

(cherry picked from commit 55e6022)
priyanshu-ctds pushed a commit to datastax/pulsar that referenced this pull request Jun 9, 2026
…SharedSingleAckedPartitionedTopic (apache#25828)

(cherry picked from commit 55e6022)
(cherry picked from commit 3ed4a9c)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants