Skip to content

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

Open
merlimat wants to merge 1 commit into
apache:masterfrom
merlimat:mmerli/fix-resend-request-test
Open

[fix][test] Add timeout to initial receives in ResendRequestTest.testSharedSingleAckedPartitionedTopic#25828
merlimat wants to merge 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
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.

1 participant