Direct retries to another mongos if one is available #1367
Merged
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
Implemented the change and unit tests JAVA-4254, JAVA-5320
JAVA-4254, JAVA-5320
stIncMale requested review from a team, katcharov and vbabanin and removed request for a team JAVA-4254, JAVA-5320
JAVA-4254, JAVA-5320
vbabanin reviewed Apr 26, 2024
driver-core/src/main/com/mongodb/internal/connection/OperationContext.java Outdated Show resolved Hide resolved
driver-core/src/main/com/mongodb/internal/operation/AsyncOperationHelper.java Outdated Show resolved Hide resolved
driver-core/src/main/com/mongodb/internal/connection/OperationContext.java Show resolved Hide resolved
katcharov requested changes May 1, 2024
driver-core/src/main/com/mongodb/internal/connection/OperationContext.java Outdated Show resolved Hide resolved
driver-core/src/main/com/mongodb/internal/connection/BaseCluster.java Outdated Show resolved Hide resolved
driver-core/src/main/com/mongodb/internal/connection/BaseCluster.java Outdated Show resolved Hide resolved
driver-core/src/main/com/mongodb/internal/connection/OperationContext.java Outdated Show resolved Hide resolved
driver-core/src/main/com/mongodb/internal/connection/OperationContext.java Outdated Show resolved Hide resolved
driver-core/src/main/com/mongodb/internal/connection/OperationContext.java Outdated Show resolved Hide resolved
driver-core/src/main/com/mongodb/internal/connection/OperationContext.java Outdated Show resolved Hide resolved
driver-core/src/main/com/mongodb/internal/connection/OperationContext.java Outdated Show resolved Hide resolved
driver-core/src/main/com/mongodb/internal/operation/AsyncOperationHelper.java Outdated Show resolved Hide resolved
…Context.java Let's put various checks (validation, preconditions...) at the top of methods, with the operation at the bottom. Co-authored-by: Maxim Katcharov <maxim.katcharov@mongodb.com>
JAVA-4254
… side effects JAVA-4254
…tion The new approach allows us to later refactor all other logic inside one or more `ServerSelector`s. See the comment left in the code for more details on the new approach. JAVA-4254
…verSelector` JAVA-4254
…uirements JAVA-4254
JAVA-4254
katcharov reviewed May 8, 2024
driver-core/src/main/com/mongodb/internal/connection/BaseCluster.java Outdated Show resolved Hide resolved
driver-core/src/main/com/mongodb/internal/connection/BaseCluster.java Outdated Show resolved Hide resolved
driver-core/src/main/com/mongodb/internal/connection/BaseCluster.java Outdated Show resolved Hide resolved
driver-core/src/main/com/mongodb/internal/connection/BaseCluster.java Outdated Show resolved Hide resolved
driver-core/src/main/com/mongodb/internal/selector/OperationCountMinimizingServerSelector.java Outdated Show resolved Hide resolved
driver-core/src/main/com/mongodb/internal/connection/OperationContext.java Outdated Show resolved Hide resolved
driver-core/src/main/com/mongodb/internal/connection/OperationContext.java Outdated Show resolved Hide resolved
driver-core/src/main/com/mongodb/internal/connection/OperationContext.java Outdated Show resolved Hide resolved
driver-core/src/main/com/mongodb/internal/connection/OperationContext.java Show resolved Hide resolved
driver-core/src/main/com/mongodb/internal/operation/AsyncOperationHelper.java Outdated Show resolved Hide resolved
JAVA-4254
… the two last lines of the corresponding comment, as they are not informative. JAVA-4254
katcharov approved these changes May 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
3 participants
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.
Given that the current specification wording is internally inconsistent, I confirmed the actual intent with the spec author and created DRIVERS-2901 Clarify the intent behind the list of deprioritized mongos'es and fix the pseudocode.
Performance considerations
Our
ServerSelectorandClusterDescriptionAPI do not allow us to implement an efficient pipeline (CompositeServerSelector) ofServerSelectors: we can neither mutate theList<ServerDescription>in place, nor mutateClusterDescription, nor even reuse the sameClusterDescriptionif a selector did not filter anything out. This PR added one more selector required by the specification logic, and introduced two more selectors by refactoring server selection code that wasn't expressed in terms ofServerSelectors. As a result, it is conceivable that the PR has negative performance impact.Additionally, due to this refactoring d25010d, each server selection iteration now involves copying the map of
Servers maintained by aCluster. While that copying does not entail locking (all hail the CHM!), it may still have additional negative performance impact.If we indeed notice performance degradation, we may try mitigating the issue by introducing
InternalServerSelector extends ServerSelector(or a subclass ofClusterDescriptionthat allows mutatinggetServerDescriptions, or both), which allows for a more optimal chaining, and use it for everything but the application-specific selector. This is assuming, of course, that the would be degradation is caused to a large extent by the inefficient selector chaining, and not by the CHM copying.JAVA-4254, JAVA-5320