-
Notifications
You must be signed in to change notification settings - Fork 9
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
Permit binding failures to bypass logging #14
Comments
Hi @jskeet , thanks for the feedback! Just to make sure we are on the same page, so do you mean, in the case of PartitionRead or PartitionQuery method, even it uses the same session name to make the request, it can actually use different channels? If that's the case, does it mean that our current implementation (which forces those method to use the same channel because of the same session name) will cause a performance issue? |
No, it's not PartitionRead or PartitionQuery that have the issue - it's ExecuteSql. Basically the process is:
It's the last step that's a potential problem. The different machines won't have a channel for that session, because the session was started on an entirely different machine - which means that if the TODO is ever fixed, Grpc.Gcp would log an error, despite nothing being wrong. |
You are right. Thanks for pointing that out! I'll keep this issue open. When working on this TODO, we can either check for the exist of partition id (but we need to figure out all other cases with similar conditions), or lower the log level with detailed log messages. |
Currently there's a TODO to log a failure if a request looks like it should be bound via a key, but we don't have that key in the dictionary.
I know of at least one situation where that would be problematic: in Spanner, most requests on the same session should use the same gRPC channel that created the session... but partitioned reads don't have this requirement. It's expected just for this case that a single session ID is used on multiple machines or processes. In this particular at least this can be detected by the presence of a partition ID in the request, but I don't know whether that would be universally true.
This is a pretty extreme case, and currently there's no problem as there's only a TODO for logging, but I wanted to raise it early.
The text was updated successfully, but these errors were encountered: