Skip to content

Stabilize test HttpBandwidthLimitingTest.testDynamicOutboundRateUpdateSharedServers [4.x]#5864

Merged
vietj merged 1 commit intoeclipse-vertx:4.xfrom
ahus1:is-5863-stabilize-testDynamicOutboundRateUpdateSharedServers-4.x
Dec 30, 2025
Merged

Stabilize test HttpBandwidthLimitingTest.testDynamicOutboundRateUpdateSharedServers [4.x]#5864
vietj merged 1 commit intoeclipse-vertx:4.xfrom
ahus1:is-5863-stabilize-testDynamicOutboundRateUpdateSharedServers-4.x

Conversation

@ahus1
Copy link
Contributor

@ahus1 ahus1 commented Dec 27, 2025

Only a server that has completed the listening is ready to update the traffic shaping.

Closes #5863

Motivation:

This both fixes the test that now only acts on fully initialized server instances, as well as adds additional safety measures on TCPServerBase.java.

As 4.x is AFAIK in maintenance mode, I'm happy to discuss if those changes to TCPServerBase should change some behavior people might depend on. Looking at the TCPServerBase.listen() code, it was a bit surprising to me that a failing bind-future on the actual server will reset the listening flag ...

bindFuture.onFailure(err -> {
if (shared) {
synchronized (sharedNetServers) {
sharedNetServers.remove(id);
}
}
listening = false;
});

... while it will not do the same on the other servers ....

main.bindFuture.onComplete(promise);

Happy to discuss how to shape this PR, and what should be handled in follow-up issues.

…eSharedServers

Only a server that has completed the listening is ready to
update the traffic shaping.

Closes eclipse-vertx#5863
@ahus1 ahus1 force-pushed the is-5863-stabilize-testDynamicOutboundRateUpdateSharedServers-4.x branch from 680c98c to de73a91 Compare December 27, 2025 12:28
@ahus1 ahus1 marked this pull request as ready for review December 27, 2025 16:43
@ahus1 ahus1 changed the title Stabilize test HttpBandwidthLimitingTest.testDynamicOutboundRateUpdateSharedServers Stabilize test HttpBandwidthLimitingTest.testDynamicOutboundRateUpdateSharedServers [4.x] Dec 27, 2025
@vietj vietj added this to the 4.5.24 milestone Dec 29, 2025
@ahus1 ahus1 requested a review from vietj December 29, 2025 11:33
@vietj vietj merged commit da78d5c into eclipse-vertx:4.x Dec 30, 2025
7 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