BlobReadChannel should fail if content comes from different generations.

[aozarov]

Feedback from @Capstan:

If someone updates the object when the client is in the middle of a read and hadn’t specified a precondition, you will simply read from the new object. You need to record the generation upon the first read and then choose what the behavior is for future reads… if the generation still exists (e.g., it’s now a history object), do you still allow reads or do you balk (because the original request is talking about the “current” object which has changed)?

[aozarov]

I think we should do 2 things (could be done independently).

1. Provide a way to specify generation in Storage.reader and if provided stick to it (We should include generation as part of the BlobId. #363 should help with that).

2. If a generation is not provided, fail if we detect that generation was changed between rpc reads
   (this could be done by changing StorageRpc.read to return a tuple (StorageObject, byte[]).

[mziccard]

Sounds cool, point 1) should be easy once #363 is fixed.

[mziccard]

Regarding 2) I could not find a way of getting both data and metadata with the same request. If this is not possible (please confirm) should we implement 2) by getting the metadata before getting blob's data? I was thinking of something like (only if generation is null):

• First request
  1. Get blob latest metadata and store generation
  2. Get blob data for generation
  3. Return a pair (generation, data)
• Other requests
  1. Get Blob latest metadata and check that generation == generation (if not throw)
  2. Get blob data for generation
  3. Return a pair (generation, data)

[Capstan]

@BrandonY, do we not yield the actual generation via the JSON API when you ask for a generationless object with alt=media? Boo. We should fix that at least.

[aozarov]

@Capstan I also could not find a way to get both metadata and content in one request (I mean besides using a batch which I think is not going to work due to race condition).

Also, I don't think you can do that with the XML API (or at least I was not able to).

However, with some code change, we can get the etag (in the XML api we can also get X-Goog-Hash).
Do you think using (and comparing) the etag would be a sufficient check instead of using the generation value?

If so, @mziccard this code can return it:

Get req = storage.objects().get(from.getBucket(), from.getName());
req.setIfMetagenerationMatch(IF_METAGENERATION_MATCH.getLong(options))
  .setIfMetagenerationNotMatch(IF_METAGENERATION_NOT_MATCH.getLong(options))
  .setIfGenerationMatch(IF_GENERATION_MATCH.getLong(options))
  .setIfGenerationNotMatch(IF_GENERATION_NOT_MATCH.getLong(options));
StringBuilder range = new StringBuilder();
range.append("bytes=").append(position).append("-").append(position + bytes - 1);
req.getRequestHeaders().setRange(range.toString());
ByteArrayOutputStream output = new ByteArrayOutputStream();
req.executeMedia().download(output);
String etag = req.getLastResponseHeaders().getETag();
byte[] bytes = output.toByteArray();

BTW, the above code also "fix" #290 by not using the MediaHttpDownloader.

[Capstan]

I know the XML API yields the generation you happen to read in the x-goog-generation header on data read responses.

[mziccard]

Sorry @Capstan I am not very familiar with the XML API. It seems to me that it does not support the "*generation-not-match" options in general and that Object Download does not support the x-goog-if-metageneration-match, can you confirm any of this?

If that's true switching to the XML api for range reads will force us to drop some of the options that we now allow users to set.

[Capstan]

True, it does not support the *generation-not-match options. Object download should support x-goog-if-metageneration-match. If not, that's a bug (certainly a docs bug, possibly a service bug).

[Capstan]

sigh Found the internal issue tracking it. :/

You can specify not via preconditions, but via the generation urlparam to specifically get that generation.

[aozarov]

Oh, yes, I see now that x-goog-generation is returned together with the content for XML API.

@Capstan though not a big code change I would rather not use the XML API for read operations if that does not support all the options that the Json API supports.

Do you think that using the object's etag (which is available in the response of the Json API) for checking
that content has not changed between read calls is not sufficient or not going to work?

[Capstan]

That should be sufficient. We don't claim that the ETag won't change if the content stays the same, but the content changes, the ETag should change.

[aozarov]

Oh, but we don't want to have false negative as that would trigger an undesired read failure.

How likely is that ("etag changes but content is the same") to happen and what can trigger it?

[Capstan]

If someone re-uploads the exact same object contents, it'll get a new ETag. Our ETag for JSON API is based on generation + metageneration.

[aozarov]

I see. I think it is reasonable if we fail the read in that case and I don't think it is a common use-case.
In that case I am fine with going with the ETag check. @Capstan are you fine with it as well?

And we can change that once you support returning the generation in the Json content (alt=media) reads.

[Capstan]

Sure. Other less GCS aware clients are doing the same because that's what the Internet uses. :)

[aozarov]

Ok, great. @mziccard are you fine with it as well?