Skip to content

Server Pool Improvement#1443

Merged
scottf merged 14 commits intomainfrom
connection-marked-failed
Oct 1, 2025
Merged

Server Pool Improvement#1443
scottf merged 14 commits intomainfrom
connection-marked-failed

Conversation

@scottf
Copy link
Contributor

@scottf scottf commented Sep 30, 2025

It was reported that the just disconnected server was the first server to attempt to be connected to after a disconnect. Even though the server had been put at the end of the pool when it was connected, discovery of servers randomized the list, making it possible to have been put in the first spot.

This is fixed when there is a disconnection. When a disconnection occurs the server pool is now told that the current server connection has "failed". Telling the pool that the server connection failed used to only happen during the connect or reconnect process and discovery after connection would shuffle the list.

Now on disconnection, we tell the pool that the server failed. On this "failed" function call, the pool used to just mark the server as a fail. But now it also moves it to the end of the list, just in case it had been shuffled to the top.

@scottf scottf requested a review from MauriceVanVeen October 1, 2025 00:36
Copy link
Member

@MauriceVanVeen MauriceVanVeen left a comment

Choose a reason for hiding this comment

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

LGTM

uses: actions/checkout@v4
- name: Build and Test
run: chmod +x gradlew && ./gradlew clean test jacocoTestReport coveralls
# run: chmod +x gradlew && ./gradlew --refresh-dependencies clean test jacocoTestReport coveralls
Copy link
Member

Choose a reason for hiding this comment

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

Can this be removed?

id 'maven-publish'
id 'jacoco'
id 'com.github.kt3k.coveralls' version '2.12.0'
// id 'com.github.kt3k.coveralls' version '2.12.2'
Copy link
Member

Choose a reason for hiding this comment

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

Can this be removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's left in as a reminder that I need to replace it.

@scottf scottf merged commit 4943615 into main Oct 1, 2025
4 checks passed
@scottf scottf deleted the connection-marked-failed branch October 1, 2025 10:43
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