New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Diagnostics Improvements for quorumAckedLSN, CurrentReplicaSetSize, and replicaStatusList #39844
base: main
Are you sure you want to change the base?
Conversation
…aStatusList includes all replicas
API change check APIView has identified API level changes in this PR and created following API reviews. |
...osmos/azure-cosmos/src/main/java/com/azure/cosmos/implementation/directconnectivity/Uri.java
Outdated
Show resolved
Hide resolved
...osmos/azure-cosmos/src/main/java/com/azure/cosmos/implementation/directconnectivity/Uri.java
Outdated
Show resolved
Hide resolved
...osmos/azure-cosmos/src/main/java/com/azure/cosmos/implementation/directconnectivity/Uri.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please see comments...
...osmos/azure-cosmos/src/main/java/com/azure/cosmos/implementation/directconnectivity/Uri.java
Outdated
Show resolved
Hide resolved
...osmos/azure-cosmos/src/main/java/com/azure/cosmos/implementation/directconnectivity/Uri.java
Outdated
Show resolved
Hide resolved
/*** | ||
* Attention: This is only used for fault injection to easier decide whether the address is primary address. | ||
* @param primary | ||
*/ | ||
public void setPrimary(boolean primary) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is only called right after a Uri is created and removed the fault injection comment because using it now for the diagnostics
...ure-cosmos/src/main/java/com/azure/cosmos/implementation/directconnectivity/StoreReader.java
Outdated
Show resolved
Hide resolved
...ure-cosmos/src/main/java/com/azure/cosmos/implementation/directconnectivity/StoreReader.java
Outdated
Show resolved
Hide resolved
...smos/src/main/java/com/azure/cosmos/implementation/directconnectivity/ConsistencyWriter.java
Outdated
Show resolved
Hide resolved
...smos/src/main/java/com/azure/cosmos/implementation/directconnectivity/ConsistencyWriter.java
Outdated
Show resolved
Hide resolved
...ure-cosmos/src/main/java/com/azure/cosmos/implementation/directconnectivity/StoreReader.java
Outdated
Show resolved
Hide resolved
...osmos/azure-cosmos/src/main/java/com/azure/cosmos/implementation/directconnectivity/Uri.java
Outdated
Show resolved
Hide resolved
@@ -24,6 +24,10 @@ public class Uri { | |||
private volatile Instant lastUnhealthyPendingTimestamp; | |||
private volatile Instant lastTransitionToUnhealthyTimestamp; | |||
private volatile boolean isPrimary; | |||
public static final String ATTEMPTING = "Attempting"; | |||
public static final String NOT_ATTEMPTING = "NotAttempting"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Question for the native english speakers :-) - i would prefer "Ignoring" - but if "NotAttempting" better explains it is definitely ok as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also think ignoring is clearer.
@@ -45,6 +46,20 @@ public Mono<Uri> resolvePrimaryUriAsync(RxDocumentServiceRequest request, boolea | |||
}); | |||
} | |||
|
|||
public Mono<Uri> resolvePrimaryUriAsync(RxDocumentServiceRequest request, boolean forceAddressRefresh, Map<Uri, String> replicaStatuses) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the other resolvePrimaryUriAsync method is only used in tests method? If that is the case, we can remove it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is not only used in tests. It is used when performing a background refresh of the primary address.
Description
Diagnostics now has two new fields inside of the storeResult which is
quorumAckedLSN
andcurrrentReplicaSetSize
. The replicaStatusList field was changed to use the following format, and it should display all the replicas. The attempting means it could have been looked at in the operation that happened. It also includes whether the replica is a primary or secondary now.The following is an example of a diagnostic now
Fixes #39775 #39762
All SDK Contribution checklist:
General Guidelines and Best Practices
Testing Guidelines