GODRIVER-1825 Select the server with fewer in-progress operations. #867
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
matthewdale
force-pushed
the
godriver1825-server-load
branch
2 times, most recently
from
fff2d40 to
4dcc245
Compare
matthewdale
force-pushed
the
godriver1825-server-load
branch
2 times, most recently
from
4dd7cd3 to
d212fa8
Compare
kevinAlbs
approved these changes
Mar 10, 2022
x/mongo/driver/topology/pool.go
Outdated
| // came from this pool. Do this in checkIn() instead of checkInNoEvent() because the latter is | ||
| // called by createConnections() and is not necessarily a check-in of an in-use connection. Use | ||
| // an int64 instead of a uint64 to mitigate the impact of any possible bugs that could cause the | ||
| // uint64 to underflow, which would effectively make the server unselectable. |
There was a problem hiding this comment.
I agree with the rationale for using an int64 rather than an uint64. I think that is a good precaution.
One suggested edit:
Suggested change
| // uint64 to underflow, which would effectively make the server unselectable. | |
| // uint64 to underflow, which would make the server much less selectable. |
A server could still be selected if it has the lowest average latency.
There was a problem hiding this comment.
Good suggestion, will update.
benjirewis
reviewed
Mar 11, 2022
matthewdale
changed the title
matthewdale
force-pushed
the
godriver1825-server-load
branch
from
8be0f17 to
3d75312
Compare
matthewdale
force-pushed
the
godriver1825-server-load
branch
from
3d75312 to
2abcf79
Compare
matthewdale
force-pushed
the
godriver1825-server-load
branch
2 times, most recently
from
bf81967 to
9904e77
Compare
matthewdale
force-pushed
the
godriver1825-server-load
branch
from
9904e77 to
34cbc49
Compare
benjirewis
approved these changes
Mar 17, 2022
kevinAlbs
approved these changes
Mar 17, 2022
| // Sleep for 100ms to allow all server state discovery to complete. We need both servers to | ||
| // be selectable when we start running the test or the distribution of selected servers will | ||
| // be skewed. Unfortunately there's not currently another signal we can block on. | ||
| time.Sleep(100 * time.Millisecond) |
There was a problem hiding this comment.
Suggested offline, consider waiting for two ServerHeartbeatSucceededEvent events rather than sleep.
There was a problem hiding this comment.
Actually used TopologyDescriptionChangedEvents to confirm that the topology has exactly 2 Mongos type instances before continuing to run the test.
x/mongo/driver/topology/server.go
Outdated
| connImpl, err := s.pool.checkOut(ctx) | ||
| // Increment the operation count before calling checkOut to make sure that all connection | ||
| // requests are included in the operation count, including those in the wait queue. If we got an | ||
| // error insted of a connection, immediately decrement the operation count. |
There was a problem hiding this comment.
Suggested change
| // error insted of a connection, immediately decrement the operation count. | |
| // error instead of a connection, immediately decrement the operation count. |
There was a problem hiding this comment.
Good catch, will update.
matthewdale
force-pushed
the
godriver1825-server-load
branch
from
ce7b1c2 to
0a56eba
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
GODRIVER-1825
Update
Topology.SelectServer()to pick 2 random servers from the list of selectable servers, and return the one with fewer in-progress operations to more evenly balance load across the selectable servers.Changes:
operationCountcounter inServerthat keeps track of the number of requests for connections. Note thatoperationCountis not decremented while a connection is pinned to a transaction until the transaction is committed or aborted.Topology.SelectServer()and return the one with fewer in-progress operations.