-
Notifications
You must be signed in to change notification settings - Fork 25k
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
Upgrade GCS SDK to 1.117.1 #74938
Upgrade GCS SDK to 1.117.1 #74938
Conversation
We're behind the ugprade schedule by quite a bit here, upgrading to the latest version and adjusting our test fixture accordingly.
Pinging @elastic/es-distributed (Team:Distributed) |
if ("bytes */*".equals(range) == false) { | ||
final int start = getContentRangeStart(range); | ||
final int end = getContentRangeEnd(range); | ||
exchange.getResponseHeaders().add("Range", String.format(Locale.ROOT, "bytes=%d-%d", start, end)); |
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.
This may actually fix the JDK-8 issue we've been seeing (I'm not 100% on this yet). The header format was broken here and the retries tests with the new SDK caught this now. I wonder if this was the spot that made the JDK-8 URL client spontaneously close connections on header parse failures (as it turns out it doesn't log/thrown on those in any way). I'll revisit this in a follow-up on 7.x.
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.
Good catch!
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.
Exception handling in HttpServer
is pretty debugging-unfriendly, see #68967.
|
||
blob = CompositeBytesReference.of(blob, requestBody); | ||
blobs.put(blobName, blob); | ||
|
||
if (limit == null) { | ||
exchange.getResponseHeaders().add("Range", String.format(Locale.ROOT, "bytes=%d/%d", start, end)); | ||
if ("bytes */*".equals(range) == false) { |
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.
The new SDK sends the unknown range header to determine where to continue uploading now so we have to deal with that :) For now I just made us not sent a range back but a follow-up would be helpful here to properly account for the bytes we've already seen so that the existing entry from blobs
is reflected in the response correctly.
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, thanks Armin!
if ("bytes */*".equals(range) == false) { | ||
final int start = getContentRangeStart(range); | ||
final int end = getContentRangeEnd(range); | ||
exchange.getResponseHeaders().add("Range", String.format(Locale.ROOT, "bytes=%d-%d", start, end)); |
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.
Good catch!
Thanks Francisco! |
We're behind the upgrade schedule by quite a bit here, upgrading to the latest version
and adjusting our test fixture accordingly.