Skip to content

(fix/test) flaky testAdvisoryForwardingDuplexNC#1832

Merged
jbonofre merged 1 commit intoapache:mainfrom
gurpartap3697:fix/AdvisoryViaNetworkTest-testAdvisoryForwardingDuplexNC-flaky-test
Mar 26, 2026
Merged

(fix/test) flaky testAdvisoryForwardingDuplexNC#1832
jbonofre merged 1 commit intoapache:mainfrom
gurpartap3697:fix/AdvisoryViaNetworkTest-testAdvisoryForwardingDuplexNC-flaky-test

Conversation

@gurpartap3697
Copy link
Contributor

Description:

In a Duplex Network Connector configuration, Broker A initiates the connection, but Broker B must then initialize a sub-bridge for the reverse path. If messages are sent immediately after Broker A reports a connection, the reverse path on Broker B may not yet be fully established. Since Advisory messages are non-persistent and "fire-and-forget," they are dropped if the bridge demand/metadata exchange is incomplete.

Adding a verification step for Broker B ensures that both sides of the duplex transport have completed their BrokerInfo exchange and are ready to forward advisories before the test producer is started.

Error:

[ERROR] Failures: 
[ERROR]   AdvisoryViaNetworkTest>CombinationTestSupport.runBare:113->CombinationTestSupport.runBare:107->testAdvisoryForwardingDuplexNC:329 expected number of messages when received expected:<2> but was:<0>

Testing:

Ran testAdvisoryForwardingDuplexNC 30 times locally; 100% pass rate after the fix (previously failed ~70% of the time).

networkBridge.setDuplex(true);
startAllBrokers();
verifyPeerBrokerInfo(brokers.get("A"), 1);
verifyPeerBrokerInfo(brokers.get("B"), 1);
Copy link
Member

Choose a reason for hiding this comment

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

Good catch.

In a duplex connector, Broker A initiates the connection but Broker B must also establish a sub-bridge for the reverse path. The existing code only waited for A's peer info, creating a race where advisories sent before B's bridge was ready would be silently dropped (fire-and-forget semantics).

@jbonofre jbonofre merged commit eec6689 into apache:main Mar 26, 2026
28 of 30 checks passed
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.

2 participants