-
Notifications
You must be signed in to change notification settings - Fork 123
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
feat(spanner): add manual affinity in grpc-gcp #3170
base: main
Are you sure you want to change the base?
Conversation
@@ -111,14 +114,15 @@ public void run(CommandLine commandLine) { | |||
System.out.printf("Transaction type: %s\n", transactionType); | |||
System.out.printf("Use Multiplexed Sessions: %s\n", useMultiplexedSession); | |||
System.out.printf("Wait between queries: %dms\n", waitMillis); | |||
System.out.printf("Using gRPC-GCP extension: %s\n", enableGrpcGcpExtension); |
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.
flushing (auto-flush on \n
) with sysout in benchmark apps may add extra IO overhead.
<dependency> | ||
<groupId>com.google.cloud</groupId> | ||
<artifactId>grpc-gcp</artifactId> | ||
<version>1.6.1</version> |
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.
don't we centralize the versions or create separate dependencyManagement?
on upgrades, it may be hard to find and update all such version tags
@@ -162,6 +162,7 @@ | |||
<dependency> | |||
<groupId>com.google.cloud</groupId> | |||
<artifactId>grpc-gcp</artifactId> | |||
<version>1.6.1</version> |
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.
same
<dependency> | ||
<groupId>com.google.cloud</groupId> | ||
<artifactId>grpc-gcp</artifactId> | ||
<version>1.6.1</version> |
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.
you have dependency management above
@psinghbay1 I have been doing a couple of testing due to which there are many unnecessary changes in this PR. I am converting this PR to draft to avoid confusion |
Thank you for the clarification! |
@harshachinta Is this still in active development? Or can we close it? |
Earlier, gRPC-GCP maintains affinity using the ApiConfig by looking into the gRPC messages to get an affinity key.
This PR sets the affinity manually to gRPC-GCP via call options. Reason - With multiplexed sessions we need to maintain transaction to channel affinity, and with regular sessions we need to maintain session to channel affinity.