Add support for range reads and retries to URL repositories #69521

fcofdez · 2021-02-24T09:59:50Z

This PR adds support for range reads to URL based repositories and improves the resiliency for the http based URLs. Since the URL http client is quite limited I've decided to use the apache http client, since we already use it in other plugins (s3). There's quite a bit of boilerplate for license headers, etc. The scope of the change is quite limited.

As a follow up, I'll add integration tests for searchable snapshots using an URL repository.

elasticmachine · 2021-02-24T11:58:00Z

Pinging @elastic/es-distributed (Team:Distributed)

ywelsch

Thank you for working on this @fcofdez. I've done a first pass and left some comments. I'm also wondering how the change to the HTTP client will affect https with custom certificates. Is the keystore used still the same?

...epository-url/src/main/java/org/elasticsearch/common/blobstore/url/HttpURLBlobContainer.java

ywelsch · 2021-02-24T13:09:57Z

...es/repository-url/src/main/java/org/elasticsearch/common/blobstore/url/URLBlobContainer.java

        }
    }

    @Override
    public InputStream readBlob(String blobName, long position, long length) throws IOException {
-        throw new UnsupportedOperationException();
+        final InputStream inputStream = getInputStream(new URL(path, blobName));


I would prefer to keep throwing UnsupportedOperationException here, as we should not support something with horrible performance characteristics.

In a separate PR, I think we should also make sure that users are using "fs" repository types when accessing shared filesystems instead of URL repositories (by changing docs).

In yet a separate PR, we can remove support for "file" in URL repositories in ES 8.0.

...epository-url/src/main/java/org/elasticsearch/plugin/repository/url/URLRepositoryPlugin.java

modules/repository-url/src/main/java/org/elasticsearch/common/blobstore/url/URLBlobStore.java

fcofdez · 2021-02-25T10:33:48Z

I'm also wondering how the change to the HTTP client will affect https with custom certificates. Is the keystore used still the same?

By default it uses:

    public static SSLContext createDefault() throws SSLInitializationException {
        try {
            final SSLContext sslContext = SSLContext.getInstance(SSLContextBuilder.TLS);
            sslContext.init(null, null, null);
            return sslContext;
        } catch (final NoSuchAlgorithmException ex) {
            throw new SSLInitializationException(ex.getMessage(), ex);
        } catch (final KeyManagementException ex) {
            throw new SSLInitializationException(ex.getMessage(), ex);
        }
    }

To be honest I'm not sure if this behaves the same way as it does today.

ywelsch

I've left more comments. One point I want to emphasize is that I think the exception handling in RetryingInputStream needs to be reworked.

To be honest I'm not sure if this behaves the same way as it does today.

can you follow-up with the es-security team on that?

...epository-url/src/main/java/org/elasticsearch/common/blobstore/url/FileURLBlobContainer.java

...epository-url/src/main/java/org/elasticsearch/common/blobstore/url/HttpURLBlobContainer.java

...sitory-url/src/main/java/org/elasticsearch/common/blobstore/url/RetryingHttpInputStream.java

docs/reference/snapshot-restore/register-repository.asciidoc

ywelsch · 2021-02-25T12:57:22Z

...sitory-url/src/main/java/org/elasticsearch/common/blobstore/url/RetryingHttpInputStream.java

+                throw new NoSuchFileException("blob object [" + blobName + "] not found");
+            }
+
+            if (response.getStatusCode() > 299) {


I think we should be more restrictive here, and only allow 200 (OK) and 206 (Partial Content).

...sitory-url/src/main/java/org/elasticsearch/common/blobstore/url/RetryingHttpInputStream.java

modules/repository-url/src/main/plugin-metadata/plugin-security.policy

ywelsch

LGTM. AFAICS it will use the default keystore so no change in behavior

fcofdez · 2021-03-07T16:29:30Z

@elasticmachine update branch

fcofdez · 2021-03-08T10:10:32Z

@elasticmachine update branch

Backport of elastic#69521

Backport of #69521

) Relates #69521

Relates elastic#69521 Backport of elastic#70709

Relates #69521 Backport of #70709

Today we document that you can use URL repositories with searchable snapshots, but in fact it only works for HTTP(S) repositories. This commit adjusts the docs to clarify. Relates elastic#69521

Today we document that you can use URL repositories with searchable snapshots, but in fact it only works for HTTP(S) repositories. This commit adjusts the docs to clarify. Relates #69521

Today we document that you can use URL repositories with searchable snapshots, but in fact it only works for HTTP(S) repositories. This commit adjusts the docs to clarify. Relates elastic#69521

Today we document that you can use URL repositories with searchable snapshots, but in fact it only works for HTTP(S) repositories. This commit adjusts the docs to clarify. Relates #69521

fcofdez added 3 commits February 24, 2021 10:57

Add support for range reads and retries to URL repositories

c34ce5d

Merge remote-tracking branch 'origin/master' into url-repo-range-reads

fe15b12

Close delegate InputStream

be9ac38

fcofdez force-pushed the url-repo-range-reads branch from dfe16f1 to be9ac38 Compare February 24, 2021 10:24

fcofdez added 2 commits February 24, 2021 11:29

Change notice

965ee88

Use proper mock library

61fb82f

fcofdez marked this pull request as ready for review February 24, 2021 11:57

fcofdez added the Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination. label Feb 24, 2021

fcofdez added :Distributed Coordination/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs v8.0.0 v7.13.0 >enhancement labels Feb 24, 2021

fcofdez requested a review from ywelsch February 24, 2021 11:59

ywelsch reviewed Feb 24, 2021

View reviewed changes

fcofdez added 3 commits February 24, 2021 19:44

Address review comments and tackle connection abortion

a78eb96

Fix test

6e41e92

Add docs

9e99fac

fcofdez requested a review from ywelsch February 25, 2021 10:27

fcofdez added 2 commits February 25, 2021 11:36

Add system default SSLContext

da8888a

Remove unused import

b91aa5c

ywelsch reviewed Feb 25, 2021

View reviewed changes

fcofdez added 7 commits February 25, 2021 20:52

Address review comments

8b799f1

Re-organize error handling

feaaaff

Add unit tests

761ea9d

Merge remote-tracking branch 'origin/master' into url-repo-range-reads

07b6141

Add extra test

1c7eecc

Improve docs

0783ea9

Add shared connection pool flag

6c0cdf9

ywelsch approved these changes Mar 3, 2021

View reviewed changes

Merge branch 'master' into url-repo-range-reads

413f420

Merge branch 'master' into url-repo-range-reads

e86ea5a

fcofdez merged commit ae5308c into elastic:master Mar 8, 2021

fcofdez added the backport pending label Mar 8, 2021

fcofdez added a commit to fcofdez/elasticsearch that referenced this pull request Mar 9, 2021

Add support for range reads and retries to URL repositories

e920c25

Backport of elastic#69521

fcofdez mentioned this pull request Mar 9, 2021

[7.x] Add support for range reads and retries to URL repositories #70133

Merged

fcofdez removed the backport pending label Mar 9, 2021

fcofdez added a commit that referenced this pull request Mar 9, 2021

Add support for range reads and retries to URL repositories (#70133)

3afc38d

Backport of #69521

fcofdez added a commit that referenced this pull request Mar 26, 2021

Add searchable snapshots integration tests for URL repositories (#70709

3f8a925

) Relates #69521

fcofdez added a commit to fcofdez/elasticsearch that referenced this pull request Mar 26, 2021

Add searchable snapshots integration tests for URL repositories

1b0737f

Relates elastic#69521 Backport of elastic#70709

fcofdez mentioned this pull request Mar 26, 2021

[7.x] Add searchable snapshots integration tests for URL repositories #70917

Merged

fcofdez added a commit that referenced this pull request Mar 26, 2021

Add searchable snapshots integration tests for URL repositories (#70917)

308cdad

Relates #69521 Backport of #70709

jakelandis added v8.0.0-alpha1 and removed v8.0.0 labels Jul 26, 2021

This was referenced Sep 13, 2021

Fix URL repo docs for searchable snapshots #77624

Merged

Searchable snapshot support for http repos #68918

Closed

Add support for range reads and retries to URL repositories #69521

Add support for range reads and retries to URL repositories #69521

Uh oh!

Conversation

fcofdez commented Feb 24, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elasticmachine commented Feb 24, 2021

Uh oh!

ywelsch left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ywelsch Feb 24, 2021

Choose a reason for hiding this comment

Uh oh!

fcofdez Feb 25, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fcofdez commented Feb 25, 2021

Uh oh!

ywelsch left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ywelsch Feb 25, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ywelsch left a comment

Choose a reason for hiding this comment

Uh oh!

fcofdez commented Mar 7, 2021

Uh oh!

fcofdez commented Mar 8, 2021

Uh oh!

Uh oh!

fcofdez commented Feb 24, 2021 •

edited

Loading