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

Upgrade GCS SDK to 1.104.0 #52839

Merged

Conversation

original-brownbear
Copy link
Member

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

Upgrading the GCS SDK to the most recent version.
Adjusting (i.e. improving and bringing it closer to the real thing) the REST mock accordingly.
This should significantly boost performance by pulling in
googleapis/java-core#86 in some cases.

Upgrading the GCS SDK to the most recent version.
Adjusting (i.e. improving) the REST mock accordingly.
This should significantly boost performance by pulling in
googleapis/java-core#86 in some cases.
@original-brownbear original-brownbear added :Distributed/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs >upgrade v8.0.0 v7.7.0 labels Feb 26, 2020
@elasticmachine
Copy link
Collaborator

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

@@ -331,7 +333,7 @@ public void testWriteBlobWithReadTimeouts() {

public void testWriteLargeBlob() throws IOException {
// See {@link BaseWriteChannel#DEFAULT_CHUNK_SIZE}
final int defaultChunkSize = 8 * 256 * 1024;
final int defaultChunkSize = 60 * 256 * 1024;
Copy link
Member Author

Choose a reason for hiding this comment

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

This got way increased in googleapis/java-core#86 :)

@@ -199,6 +199,8 @@ StorageOptions createStorageOptions(final GoogleCloudStorageClientSettings clien
final HttpTransportOptions httpTransportOptions) {
StorageOptions options = super.createStorageOptions(clientSettings, httpTransportOptions);
return options.toBuilder()
.setHost(options.getHost())
Copy link
Member Author

Choose a reason for hiding this comment

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

For some reason the toBuilder keeps track of all settings but these two in the latest version so we have to manually set these again.

@@ -266,7 +268,7 @@ protected String requestUniqueId(HttpExchange exchange) {
}

final String range = exchange.getRequestHeaders().getFirst("Content-Range");
return exchange.getRemoteAddress().toString()
return exchange.getRemoteAddress().getHostString()
Copy link
Member Author

Choose a reason for hiding this comment

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

There seems to be a behavior change with retries here. The retries are now executed across multiple different TCP connections instead of reusing the same connection. This means we can't key them by the full address including the port any longer because that would allow for more retries than we assume.

// removes the trailing end "\r\n--__END_OF_PART__--\r\n" which is 23 bytes long
int len = fullRequestBody.length() - startPos - 23;
content = Tuple.tuple(name, fullRequestBody.slice(startPos, len));
while (isEndOfPart(fullRequestBody, endPos) == false) {
Copy link
Member Author

Choose a reason for hiding this comment

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

The new SDK isn't just sending END_OF_PART${uuid} so this needed to become a little smarter.
See related SDK code https://github.com/googleapis/google-http-java-client/blob/master/google-http-client/src/main/java/com/google/api/client/http/MultipartContent.java#L35

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

@original-brownbear
Copy link
Member Author

Thanks Tanguy!

@original-brownbear original-brownbear merged commit 40bd334 into elastic:master Mar 5, 2020
@original-brownbear original-brownbear deleted the upgrade-gcs-dep branch March 5, 2020 08:10
original-brownbear added a commit to original-brownbear/elasticsearch that referenced this pull request Mar 5, 2020
Upgrading the GCS SDK to the most recent version.
Adjusting (i.e. improving) the REST mock accordingly.
This should significantly boost performance by pulling in
googleapis/java-core#86 in some cases.
original-brownbear added a commit that referenced this pull request Mar 5, 2020
Upgrading the GCS SDK to the most recent version.
Adjusting (i.e. improving) the REST mock accordingly.
This should significantly boost performance by pulling in
googleapis/java-core#86 in some cases.
@original-brownbear original-brownbear restored the upgrade-gcs-dep branch August 6, 2020 18:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Distributed/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs >upgrade v7.7.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants