-
Notifications
You must be signed in to change notification settings - Fork 25.2k
Make GoogleCloudStorageRetryingInputStream request same generation on resume #127626
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
base: main
Are you sure you want to change the base?
Make GoogleCloudStorageRetryingInputStream request same generation on resume #127626
Conversation
Pinging @elastic/es-distributed-coordination (Team:Distributed Coordination) |
Hi @nicktindall, I've created a changelog YAML for you. |
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.
Looks good, I just had the one question about the assertion in parseGenerationHeader()
, the rest were a few small nits. Might be good to get a look from other folks with GCS code experience though.
.../src/main/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageRetryingInputStream.java
Outdated
Show resolved
Hide resolved
test/fixtures/gcs-fixture/src/main/java/fixture/gcs/MockGcsBlobStore.java
Outdated
Show resolved
Hide resolved
test/fixtures/gcs-fixture/src/main/java/fixture/gcs/MockGcsBlobStore.java
Outdated
Show resolved
Hide resolved
test/fixtures/gcs-fixture/src/main/java/fixture/gcs/MockGcsBlobStore.java
Outdated
Show resolved
Hide resolved
…bStore.java Co-authored-by: Jeremy Dahlgren <[email protected]>
…bStore.java Co-authored-by: Jeremy Dahlgren <[email protected]>
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.
LGTM
When we resume a download due to an IOException, we now specify the generation of the object that we were originally downloading in the request so that we can detect if the object changed.
Some decisions that were made
generation
parameter rather thanifGenerationMatch
. The former requests a specific generation where the latter requests the latest generation and triggers an error if its not the one specified. This distinction is only important if the bucket we're fetching from has object versioning enabled. If the object is overwritten and object versioning is enabled, resume withgeneration
specified will succeed because the prior version will still be available, but resume withifGenerationMatch
specified would fail because the latest version is no longer the one we were downloading.ifMetagenerationMatch
orEtag
s instead, but I don't think we want that.Closes ES-11432
I marked this PR as "non-issue" because currently we never update objects with different contents. This is just future proofing.