lazy snapshot repository initialization

modules/repository-url/src/main/java/org/elasticsearch/repositories/url/URLRepository.java

@@ -20,8 +20,8 @@
 package org.elasticsearch.repositories.url;
 
 import org.elasticsearch.cluster.metadata.RepositoryMetaData;
+import org.elasticsearch.common.blobstore.BlobContainer;
 import org.elasticsearch.common.blobstore.BlobPath;
-import org.elasticsearch.common.blobstore.BlobStore;
 import org.elasticsearch.common.blobstore.url.URLBlobStore;
 import org.elasticsearch.common.settings.Setting;
 import org.elasticsearch.common.settings.Setting.Property;
@@ -31,7 +31,6 @@
 import org.elasticsearch.repositories.RepositoryException;
 import org.elasticsearch.repositories.blobstore.BlobStoreRepository;
 
-import java.io.IOException;
 import java.net.MalformedURLException;
 import java.net.URISyntaxException;
 import java.net.URL;
@@ -49,7 +48,7 @@
  * <dt>{@code concurrent_streams}</dt><dd>Number of concurrent read/write stream (per repository on each node). Defaults to 5.</dd>
  * </dl>
  */
-public class URLRepository extends BlobStoreRepository {
+public class URLRepository extends BlobStoreRepository<URLBlobStore> {
 
     public static final String TYPE = "url";
 
@@ -71,33 +70,46 @@ public class URLRepository extends BlobStoreRepository {
 
     private final Environment environment;
 
-    private final URLBlobStore blobStore;
-
     private final BlobPath basePath;
 
     /**
      * Constructs a read-only URL-based repository
      */
     public URLRepository(RepositoryMetaData metadata, Environment environment,
-                         NamedXContentRegistry namedXContentRegistry) throws IOException {
+                         NamedXContentRegistry namedXContentRegistry) {
         super(metadata, environment.settings(), namedXContentRegistry);
 
         if (URL_SETTING.exists(metadata.settings()) == false && REPOSITORIES_URL_SETTING.exists(settings) ==  false) {
             throw new RepositoryException(metadata.name(), "missing url");
         }
+        this.environment = environment;
         supportedProtocols = SUPPORTED_PROTOCOLS_SETTING.get(settings);
         urlWhiteList = ALLOWED_URLS_SETTING.get(settings).toArray(new URIPattern[]{});
-        this.environment = environment;
-
-        URL url = URL_SETTING.exists(metadata.settings()) ? URL_SETTING.get(metadata.settings()) : REPOSITORIES_URL_SETTING.get(settings);
-        URL normalizedURL = checkURL(url);
-        blobStore = new URLBlobStore(settings, normalizedURL);
         basePath = BlobPath.cleanPath();
     }
 
+    private URL getUrl() {
+        URL url = URL_SETTING.exists(metadata.settings())
+            ? URL_SETTING.get(metadata.settings()) : REPOSITORIES_URL_SETTING.get(settings);
+        return checkURL(url);
+    }
+
+    @Override
+    protected URLBlobStore createBlobStore() {
+        URL normalizedURL = getUrl();
+        return new URLBlobStore(settings, normalizedURL);
+    }
+
+    // only use for testing
+    @Override
+    protected BlobContainer snapshotsBlobContainer() {
+        return super.snapshotsBlobContainer();
+    }
+
+    // only use for testing
     @Override
-    protected BlobStore blobStore() {
-        return blobStore;
+    protected URLBlobStore innerBlobStore() {
+        return super.innerBlobStore();
     }
 
     @Override

modules/repository-url/src/test/java/org/elasticsearch/repositories/url/URLRepositoryTests.java

@@ -31,6 +31,10 @@
 import java.nio.file.Path;
 import java.util.Collections;
 
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.CoreMatchers.not;
+import static org.hamcrest.CoreMatchers.nullValue;
+
 public class URLRepositoryTests extends ESTestCase {
 
     public void testWhiteListingRepoURL() throws IOException {
@@ -41,8 +45,12 @@ public void testWhiteListingRepoURL() throws IOException {
             .put(URLRepository.REPOSITORIES_URL_SETTING.getKey(), repoPath)
             .build();
         RepositoryMetaData repositoryMetaData = new RepositoryMetaData("url", URLRepository.TYPE, baseSettings);
-        new URLRepository(repositoryMetaData, TestEnvironment.newEnvironment(baseSettings),
+        final URLRepository repository = new URLRepository(repositoryMetaData, TestEnvironment.newEnvironment(baseSettings),
             new NamedXContentRegistry(Collections.emptyList()));
+
+        assertThat("blob store has to be lazy initialized", repository.innerBlobStore(), is(nullValue()));
+        repository.snapshotsBlobContainer();
+        assertThat("snapshotsBlobContainer has to initialize blob store", repository.innerBlobStore(), not(nullValue()));
     }
 
     public void testIfNotWhiteListedMustSetRepoURL() throws IOException {
@@ -52,9 +60,11 @@ public void testIfNotWhiteListedMustSetRepoURL() throws IOException {
             .put(URLRepository.REPOSITORIES_URL_SETTING.getKey(), repoPath)
             .build();
         RepositoryMetaData repositoryMetaData = new RepositoryMetaData("url", URLRepository.TYPE, baseSettings);
+        URLRepository repository = new URLRepository(repositoryMetaData, TestEnvironment.newEnvironment(baseSettings),
+            new NamedXContentRegistry(Collections.emptyList()));
+        assertThat(repository.innerBlobStore(), is(nullValue()));
         try {
-            new URLRepository(repositoryMetaData, TestEnvironment.newEnvironment(baseSettings),
-                new NamedXContentRegistry(Collections.emptyList()));
+            repository.snapshotsBlobContainer();
             fail("RepositoryException should have been thrown.");
         } catch (RepositoryException e) {
             String msg = "[url] file url [" + repoPath
@@ -73,9 +83,11 @@ public void testMustBeSupportedProtocol() throws IOException {
             .put(URLRepository.SUPPORTED_PROTOCOLS_SETTING.getKey(), "http,https")
             .build();
         RepositoryMetaData repositoryMetaData = new RepositoryMetaData("url", URLRepository.TYPE, baseSettings);
+        URLRepository repository = new URLRepository(repositoryMetaData, TestEnvironment.newEnvironment(baseSettings),
+            new NamedXContentRegistry(Collections.emptyList()));
+        assertThat(repository.innerBlobStore(), is(nullValue()));
         try {
-            new URLRepository(repositoryMetaData, TestEnvironment.newEnvironment(baseSettings),
-                new NamedXContentRegistry(Collections.emptyList()));
+            repository.snapshotsBlobContainer();
             fail("RepositoryException should have been thrown.");
         } catch (RepositoryException e) {
             assertEquals("[url] unsupported url protocol [file] from URL [" + repoPath +"]", e.getMessage());

plugins/repository-azure/src/main/java/org/elasticsearch/repositories/azure/AzureRepository.java

@@ -27,7 +27,6 @@
 import org.elasticsearch.cluster.metadata.RepositoryMetaData;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.blobstore.BlobPath;
-import org.elasticsearch.common.blobstore.BlobStore;
 import org.elasticsearch.common.settings.Setting;
 import org.elasticsearch.common.settings.Setting.Property;
 import org.elasticsearch.common.unit.ByteSizeValue;
@@ -38,7 +37,6 @@
 import org.elasticsearch.snapshots.SnapshotCreationException;
 import org.elasticsearch.snapshots.SnapshotId;
 
-import java.io.IOException;
 import java.net.URISyntaxException;
 import java.util.List;
 import java.util.Locale;
@@ -58,7 +56,7 @@
  * <dt>{@code compress}</dt><dd>If set to true metadata files will be stored compressed. Defaults to false.</dd>
  * </dl>
  */
-public class AzureRepository extends BlobStoreRepository {
+public class AzureRepository extends BlobStoreRepository<AzureBlobStore> {
 
     public static final String TYPE = "azure";
 
@@ -78,25 +76,21 @@ public static final class Repository {
         public static final Setting<Boolean> READONLY_SETTING = Setting.boolSetting("readonly", false, Property.NodeScope);
     }
 
-    private final AzureBlobStore blobStore;
     private final BlobPath basePath;
     private final ByteSizeValue chunkSize;
     private final boolean compress;
-    private final boolean readonly;
+    private final Environment environment;
+    private final AzureStorageService storageService;
+    private volatile boolean readonly;
 
     public AzureRepository(RepositoryMetaData metadata, Environment environment, NamedXContentRegistry namedXContentRegistry,
-            AzureStorageService storageService) throws IOException, URISyntaxException, StorageException {
+            AzureStorageService storageService) {
         super(metadata, environment.settings(), namedXContentRegistry);
-        this.blobStore = new AzureBlobStore(metadata, environment.settings(), storageService);
         this.chunkSize = Repository.CHUNK_SIZE_SETTING.get(metadata.settings());
         this.compress = Repository.COMPRESS_SETTING.get(metadata.settings());
-        // If the user explicitly did not define a readonly value, we set it by ourselves depending on the location mode setting.
-        // For secondary_only setting, the repository should be read only
-        if (Repository.READONLY_SETTING.exists(metadata.settings())) {
-            this.readonly = Repository.READONLY_SETTING.get(metadata.settings());
-        } else {
-            this.readonly = this.blobStore.getLocationMode() == LocationMode.SECONDARY_ONLY;
-        }
+        this.environment = environment;
+        this.storageService = storageService;
+
         final String basePath = Strings.trimLeadingCharacter(Repository.BASE_PATH_SETTING.get(metadata.settings()), '/');
         if (Strings.hasLength(basePath)) {
             // Remove starting / if any
@@ -108,15 +102,40 @@ public AzureRepository(RepositoryMetaData metadata, Environment environment, Nam
         } else {
             this.basePath = BlobPath.cleanPath();
         }
-        logger.debug((org.apache.logging.log4j.util.Supplier<?>) () -> new ParameterizedMessage(
-                "using container [{}], chunk_size [{}], compress [{}], base_path [{}]", blobStore, chunkSize, compress, basePath));
+    }
+
+    // only use for testing
+    @Override
+    protected AzureBlobStore blobStore() {
+        return super.blobStore();
+    }
+
+    // only use for testing
+    @Override
+    protected AzureBlobStore innerBlobStore() {
+        return super.innerBlobStore();
     }
 
     /**
      * {@inheritDoc}
      */
     @Override
-    protected BlobStore blobStore() {
+    protected AzureBlobStore createBlobStore() throws URISyntaxException, StorageException {
+        final AzureBlobStore blobStore = new AzureBlobStore(metadata, environment.settings(), storageService);
+
+        // If the user explicitly did not define a readonly value, we set it by ourselves depending on the location mode setting.
+        // For secondary_only setting, the repository should be read only
+
+        if (Repository.READONLY_SETTING.exists(metadata.settings())) {
+            this.readonly = Repository.READONLY_SETTING.get(metadata.settings());
+        } else {
+            this.readonly = blobStore.getLocationMode() == LocationMode.SECONDARY_ONLY;
+        }
+
+        logger.debug((org.apache.logging.log4j.util.Supplier<?>) () -> new ParameterizedMessage(
+            "using container [{}], chunk_size [{}], compress [{}], base_path [{}]",
+            blobStore, chunkSize, compress, basePath));
+
         return blobStore;
     }
 
@@ -144,6 +163,7 @@ protected ByteSizeValue chunkSize() {
     @Override
     public void initializeSnapshot(SnapshotId snapshotId, List<IndexId> indices, MetaData clusterMetadata) {
         try {
+            final AzureBlobStore blobStore = blobStore();
             if (blobStore.containerExist() == false) {
                 throw new IllegalArgumentException("The bucket [" + blobStore + "] does not exist. Please create it before "
                         + " creating an azure snapshot repository backed by it.");
@@ -156,6 +176,14 @@ public void initializeSnapshot(SnapshotId snapshotId, List<IndexId> indices, Met
 
     @Override
     public boolean isReadOnly() {
+        // If the user explicitly did not define a readonly value, we set it by ourselves depending on the location mode setting.
+        // For secondary_only setting, the repository should be read only
+        if (Repository.READONLY_SETTING.exists(metadata.settings())) {
+            this.readonly = Repository.READONLY_SETTING.get(metadata.settings());
+        } else {
+            // otherwise it's possible obtain actual value only if blobStore is created
+            blobStore();
+        }
         return readonly;
     }
 }

plugins/repository-azure/src/test/java/org/elasticsearch/repositories/azure/AzureRepositorySettingsTests.java

@@ -20,7 +20,6 @@
 package org.elasticsearch.repositories.azure;
 
 import com.microsoft.azure.storage.LocationMode;
-import com.microsoft.azure.storage.StorageException;
 import org.elasticsearch.cluster.metadata.RepositoryMetaData;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.unit.ByteSizeUnit;
@@ -30,76 +29,76 @@
 import org.elasticsearch.env.TestEnvironment;
 import org.elasticsearch.test.ESTestCase;
 
-import java.io.IOException;
-import java.net.URISyntaxException;
-
 import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.nullValue;
 import static org.mockito.Mockito.mock;
 
 public class AzureRepositorySettingsTests extends ESTestCase {
 
-    private AzureRepository azureRepository(Settings settings) throws StorageException, IOException, URISyntaxException {
+    private AzureRepository azureRepository(Settings settings) {
         Settings internalSettings = Settings.builder()
             .put(Environment.PATH_HOME_SETTING.getKey(), createTempDir().toAbsolutePath())
             .putList(Environment.PATH_DATA_SETTING.getKey(), tmpPaths())
             .put(settings)
             .build();
-        return new AzureRepository(new RepositoryMetaData("foo", "azure", internalSettings),
+        final AzureRepository azureRepository = new AzureRepository(new RepositoryMetaData("foo", "azure", internalSettings),
             TestEnvironment.newEnvironment(internalSettings), NamedXContentRegistry.EMPTY, mock(AzureStorageService.class));
+        assertThat(azureRepository.innerBlobStore(), is(nullValue()));
+        return azureRepository;
     }
 
-    public void testReadonlyDefault() throws StorageException, IOException, URISyntaxException {
+    public void testReadonlyDefault() {
         assertThat(azureRepository(Settings.EMPTY).isReadOnly(), is(false));
     }
 
-    public void testReadonlyDefaultAndReadonlyOn() throws StorageException, IOException, URISyntaxException {
+    public void testReadonlyDefaultAndReadonlyOn() {
         assertThat(azureRepository(Settings.builder()
             .put("readonly", true)
             .build()).isReadOnly(), is(true));
     }
 
-    public void testReadonlyWithPrimaryOnly() throws StorageException, IOException, URISyntaxException {
+    public void testReadonlyWithPrimaryOnly() {
         assertThat(azureRepository(Settings.builder()
             .put(AzureRepository.Repository.LOCATION_MODE_SETTING.getKey(), LocationMode.PRIMARY_ONLY.name())
             .build()).isReadOnly(), is(false));
     }
 
-    public void testReadonlyWithPrimaryOnlyAndReadonlyOn() throws StorageException, IOException, URISyntaxException {
+    public void testReadonlyWithPrimaryOnlyAndReadonlyOn() {
         assertThat(azureRepository(Settings.builder()
             .put(AzureRepository.Repository.LOCATION_MODE_SETTING.getKey(), LocationMode.PRIMARY_ONLY.name())
             .put("readonly", true)
             .build()).isReadOnly(), is(true));
     }
 
-    public void testReadonlyWithSecondaryOnlyAndReadonlyOn() throws StorageException, IOException, URISyntaxException {
+    public void testReadonlyWithSecondaryOnlyAndReadonlyOn() {
         assertThat(azureRepository(Settings.builder()
             .put(AzureRepository.Repository.LOCATION_MODE_SETTING.getKey(), LocationMode.SECONDARY_ONLY.name())
             .put("readonly", true)
             .build()).isReadOnly(), is(true));
     }
 
-    public void testReadonlyWithSecondaryOnlyAndReadonlyOff() throws StorageException, IOException, URISyntaxException {
+    public void testReadonlyWithSecondaryOnlyAndReadonlyOff() {
         assertThat(azureRepository(Settings.builder()
             .put(AzureRepository.Repository.LOCATION_MODE_SETTING.getKey(), LocationMode.SECONDARY_ONLY.name())
             .put("readonly", false)
             .build()).isReadOnly(), is(false));
     }
 
-    public void testReadonlyWithPrimaryAndSecondaryOnlyAndReadonlyOn() throws StorageException, IOException, URISyntaxException {
+    public void testReadonlyWithPrimaryAndSecondaryOnlyAndReadonlyOn() {
         assertThat(azureRepository(Settings.builder()
             .put(AzureRepository.Repository.LOCATION_MODE_SETTING.getKey(), LocationMode.PRIMARY_THEN_SECONDARY.name())
             .put("readonly", true)
             .build()).isReadOnly(), is(true));
     }
 
-    public void testReadonlyWithPrimaryAndSecondaryOnlyAndReadonlyOff() throws StorageException, IOException, URISyntaxException {
+    public void testReadonlyWithPrimaryAndSecondaryOnlyAndReadonlyOff() {
         assertThat(azureRepository(Settings.builder()
             .put(AzureRepository.Repository.LOCATION_MODE_SETTING.getKey(), LocationMode.PRIMARY_THEN_SECONDARY.name())
             .put("readonly", false)
             .build()).isReadOnly(), is(false));
     }
 
-    public void testChunkSize() throws StorageException, IOException, URISyntaxException {
+    public void testChunkSize() {
         // default chunk size
         AzureRepository azureRepository = azureRepository(Settings.EMPTY);
         assertEquals(AzureStorageService.MAX_CHUNK_SIZE, azureRepository.chunkSize());

plugins/repository-azure/src/test/java/org/elasticsearch/repositories/azure/AzureSnapshotRestoreTests.java

@@ -57,7 +57,11 @@
 import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.greaterThan;
+import static org.hamcrest.Matchers.instanceOf;
+import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.lessThanOrEqualTo;
+import static org.hamcrest.Matchers.notNullValue;
+import static org.hamcrest.Matchers.nullValue;
 
 /**
  * Those integration tests need an Azure access and must be run with
@@ -408,12 +412,24 @@ public void testGeoRedundantStorage() throws Exception {
     }
 
     @Override
-    protected void createTestRepository(String name) {
+    protected void createTestRepository(String name, boolean verify) {
         assertAcked(client().admin().cluster().preparePutRepository(name)
             .setType(AzureRepository.TYPE)
+            .setVerify(verify)
             .setSettings(Settings.builder()
                 .put(Repository.CONTAINER_SETTING.getKey(), getContainerName())
                 .put(Repository.BASE_PATH_SETTING.getKey(), getRepositoryPath())
                 .put(Repository.CHUNK_SIZE_SETTING.getKey(), randomIntBetween(100, 1000), ByteSizeUnit.BYTES)));
     }
+
+
+    @Override
+    protected void afterCreationCheck(org.elasticsearch.repositories.Repository repository, boolean verify) {
+        assertThat(repository, instanceOf(AzureRepository.class));
+
+        AzureRepository azureRepository = (AzureRepository) repository;
+
+        assertThat("azure blob store has to be lazy initialized",
+            azureRepository.innerBlobStore(), verify ? is(notNullValue()) : is(nullValue()));
+    }
 }