Support gcs caching for parallel processing

[namanjain]

No description provided.

[jqmp]

Would you mind adding some text to your commit message explaining what problem this addresses, and how? I'm assuming there's an issue where the GCS client and bucket are not being sent from one process to another, but it seems like we could solve that by adding some custom pickling behavior. But maybe this caching is also intended to speed up tests?

[jqmp]

Also, if this fixes any of the tests, should we remove some noparallel annotations?

[namanjain]

Also, if this fixes any of the tests, should we remove some noparallel annotations?

This change fixes the gcs persistence tests and I already removed no_parallel annotation from it 😄

[jqmp]

Also, if this fixes any of the tests, should we remove some noparallel annotations?

This change fixes the gcs persistence tests and I already removed no_parallel annotation from it 😄

Oops, somehow I missed that -- sorry!

[namanjain]

Would you mind adding some text to your commit message explaining what problem this addresses, and how? I'm assuming there's an issue where the GCS client and bucket are not being sent from one process to another, but it seems like we could solve that by adding some custom pickling behavior. But maybe this caching is also intended to speed up tests?

Whoops, I probably lost my original commit message with explanation here 😬. I'll add one.

Yeah, the issue is that pickling of client objects is not supported by google cloud storage library. This was raised by someone before and the authors decided to not support client pickling. They just added an error message if someone tried to pickle it.

I tried the workaround of setting client._datastore_api_internal = None but that still didn't work for me. Maybe the way they throw the error, they don't even check for datastore anymore and throw an error as soon as they see a client.

I can also see why caching is tripping you a bit. I'll add some explanation as code comment too but I'm caching buckets since that's what the GcsTool finally uses. Since we can't send the bucket over (as it contains the client too), we'll end up creating a bucket object for every function call on GcsTool which turned out to be pretty inefficient even in our persistent tests and roughly increased the test time by 50% (15 secs).

[namanjain]

Would you mind adding some text to your commit message explaining what problem this addresses, and how? I'm assuming there's an issue where the GCS client and bucket are not being sent from one process to another, but it seems like we could solve that by adding some custom pickling behavior. But maybe this caching is also intended to speed up tests?

Whoops, I probably lost my original commit message with explanation here 😬. I'll add one.

Yeah, the issue is that pickling of client objects is not supported by google cloud storage library. This was raised by someone before and the authors decided to not support client pickling. They just added an error message if someone tried to pickle it.

I tried the workaround of setting client._datastore_api_internal = None but that still didn't work for me. Maybe the way they throw the error, they don't even check for datastore anymore and throw an error as soon as they see a client.

I can also see why caching is tripping you a bit. I'll add some explanation as code comment too but I'm caching buckets since that's what the GcsTool finally uses. Since we can't send the bucket over (as it contains the client too), we'll end up creating a bucket object for every function call on GcsTool which turned out to be pretty inefficient even in our persistent tests and roughly increased the test time by 50% (15 secs).

As we talked today morning, I changed it to use custom pickling logic by recreating the client objects when unserializing the GcsTool object in subprocess using __getstate__ and __setstate__.

[jqmp]

LGTM!