Generate datastore grpc client outside request handler

[cobookman]

Generating a datastore grpc client for each request is not required and can lead to auth issues.
Here is one of the auth issues in question: googleapis/google-cloud-python#3085

Also note the following issue which is causing the client to need resetting every 30 mins: googleapis/google-cloud-python#2896. This will hopefully be fixed soon.

[theacodes]

Datastore isn't always gRPC, and when it isn't, it certainly isn't threadsafe. I'm actually not sure if gRPC datastore is completely threadsafe. @dhermes @lukesneeringer

[dhermes]

I am also not sure if gRPC clients are threadsafe. Who would know?

[cobookman]

Not sure on Python implementation. But other languages have threadsafe grpc clients. https://groups.google.com/forum/m/#!topic/grpc-io/-jA_JCiugM8 googleapis/google-cloud-ruby#816 I can raise the grpc clients threadsafety in Python internally if need be.

…

On Fri, Mar 10, 2017, 10:33 AM Danny Hermes ***@***.***> wrote: I am also not sure if gRPC clients are threadsafe. Who would know? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#846 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABqI55tTq1Sgc8Mv0SP9DD2IBhjp0F6Sks5rkZeKgaJpZM4MY5K8> .

[theacodes]

@cobookman the gRPC level stuff is threadsafe, I'm just not sure if google-cloud-python's Client object is.

[cobookman]

I know that in other languages creating a grpc client in each request handler can cause authentication errors to spawn.

[cobookman]

Anyone here have a contact who can inform us if the python grpc client is threadsafe?

[lukesneeringer]

Tagging @nathanielmanistaatgoogle, who might know.

[nathanielmanistaatgoogle]

gRPC Python is designed for multithreaded use cases and only a few of its operations aren't thread-safe (usually for performance reasons). What specific objects, classes, methods, and functions are under consideration in this circumstance?

[cobookman]

@nathanielmanistaatgoogle making a client connection to datastore in a single thread, then sharing that datastore client across threads (aka webhandlers).

[theacodes]

Again, to clarify, I'm not concerned with the grpc-level objects, I'm more concerned with the stuff implemented here which @lukesneeringer and @dhermes should be able to comment on.

Finally, even if the gRPC implementation is thread-safe, the fallback http implementation is not. Considering it's possible for users to use the non-thread-safe path, I'd prefer if we kept the sample as-is.

[nathanielmanistaatgoogle]

@cobookman: I think it's highly likely that the client connection uses gRPC Python (grpc-module code elements) in a thread-safe way, but I'd need to know more (probably from @lukesneeringer, @dhermes, or @jonparrott) to be absolutely certain. Is there no sharing of RPCs between threads? If yes (each RPC is conducted in exactly one thread, even if multiple threads are making RPCs) then everything is good.

The only real client-side thread-safety issue for gRPC Python that I can imagine is if a response-streaming RPC is invoked and then the response iterator returned by that RPC invocation is passed to multiple threads which each try to iterate over it. That's a reasonable enough thing to want to do in theory, but it's rare enough in practice that it made sense for us to decide that we weren't going to incur lock acquisition for every case just to support that case. And it's easy enough to write a serializing decorator object for an iterator.

[cobookman]

Given @nathanielmanistaatgoogle's comments. Should we continue merging this change in or do we want to close it due to the small chance of a python client not using gRPC.

[theacodes]

I'd still like for @lukesneeringer to chime in on the threadsafety of the datastore client class itself.

[theacodes]

Closing due to inactivity, but if this is still relevant please feel free to open a new PR.

Fwiw, I think that the Datastore client is now thread-safe.