Skip to content

Fix GCS Mock Range Downloads#52804

Merged
original-brownbear merged 2 commits intoelastic:masterfrom
original-brownbear:fix-range-gcs-download-master
Feb 26, 2020
Merged

Fix GCS Mock Range Downloads#52804
original-brownbear merged 2 commits intoelastic:masterfrom
original-brownbear:fix-range-gcs-download-master

Conversation

@original-brownbear
Copy link
Copy Markdown
Contributor

@original-brownbear original-brownbear commented Feb 26, 2020

We were not correctly respecting the download range which lead
to the GCS SDK client closing the connection at times.

Closes #51446

@original-brownbear
Fix GCS Mock Range Downloads
45dfcb3 
We were not correctly respecting the download range which lead
to the GCS SDK client closing the connection at times.

Closes #51446
@original-brownbear original-brownbear added >test Issues or PRs that are addressing/adding tests :Distributed/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs v8.0.0 v7.7.0 v7.6.1 labels Feb 26, 2020
@elasticmachine
Copy link
Copy Markdown
Collaborator

elasticmachine commented Feb 26, 2020

Pinging @elastic/es-distributed (:Distributed/Snapshot/Restore)

@original-brownbear
Copy link
Copy Markdown
Contributor Author

original-brownbear commented Feb 26, 2020

@tlrx I'm not 100% positive this fixes the issue that caused the last #51446 failures but it's the only thing I could find going wrong at the moment that causes connection drops.
(I could reproduce the failure in the issue once after O(10k) iterations and can't any more with this fix so I'm somewhat optimistic)
If anything it should stabilize the behavior of the mock http server because it removes a source of blocking in general because it does its writes in a blocking fashion (sets the client sockets to blocking before writing) and this change makes it so we never write more than the buffer size making the IO loop block a lot less.

@original-brownbear
Copy link
Copy Markdown
Contributor Author

original-brownbear commented Feb 26, 2020

Bam random find :) => This might actually be the culprit in the linked issue e6565ee :)

I'd still like to fix the other issue in here as well.

tlrx
tlrx approved these changes Feb 26, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@tlrx tlrx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for persevering on this Armin!

@original-brownbear
Copy link
Copy Markdown
Contributor Author

original-brownbear commented Feb 26, 2020

Np + thanks for reviewing Tanguy!

@original-brownbear original-brownbear merged commit 97f8f4a into elastic:master Feb 26, 2020
@original-brownbear original-brownbear deleted the fix-range-gcs-download-master branch February 26, 2020 16:04
This was referenced Feb 26, 2020
Merged
Merged
original-brownbear added a commit that referenced this pull request Feb 26, 2020
@original-brownbear
Fix GCS Mock Range Downloads (#52804) (#52831)
55a830f 
We were not correctly respecting the download range which lead
to the GCS SDK client closing the connection at times.
Also, fixes another instance of failing to drain the request fully before sending the response headers.

Closes #51446
original-brownbear added a commit that referenced this pull request Feb 26, 2020
@original-brownbear
Fix GCS Mock Range Downloads (#52804) (#52830)
f7d71b5 
We were not correctly respecting the download range which lead
to the GCS SDK client closing the connection at times.
Also, fixes another instance of failing to drain the request fully before sending the response headers.

Closes #51446
@original-brownbear original-brownbear restored the fix-range-gcs-download-master branch August 6, 2020 18:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tlrx tlrx tlrx approved these changes

Assignees

No one assigned

Labels

:Distributed/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs >test Issues or PRs that are addressing/adding tests v7.6.1 v7.7.0 v8.0.0-alpha1

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

org.elasticsearch.repositories.gcs.GoogleCloudStorageBlobStoreRepositoryTests testSnapshotWithLargeSegmentFiles

5 participants

@original-brownbear @elasticmachine @tlrx @jakelandis @ywelsch