Skip to content
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

Fix GCS Mock Range Downloads #52804

Conversation

original-brownbear
Copy link
Member

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

Closes #51446

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

Closes elastic#51446
@original-brownbear original-brownbear added >test Issues or PRs that are addressing/adding tests :Distributed Coordination/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
Collaborator

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

@original-brownbear
Copy link
Member Author

@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
Member Author

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.

Copy link
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
Member Author

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
original-brownbear added a commit to original-brownbear/elasticsearch that referenced this pull request Feb 26, 2020
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 elastic#51446
original-brownbear added a commit to original-brownbear/elasticsearch that referenced this pull request Feb 26, 2020
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 elastic#51446
original-brownbear added a commit that referenced this pull request Feb 26, 2020
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
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
Labels
:Distributed Coordination/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
5 participants