Skip to content

lazy snapshot repository initialization #31606

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

Merged
merged 21 commits into from
Jul 13, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
3dd9d22
lazy snapshot repository initialization
Jun 20, 2018
16d9b40
Merge remote-tracking branch 'remotes/origin/master' into lazy_repo_init
Jun 27, 2018
848f346
Merge remote-tracking branch 'remotes/origin/master' into lazy_repo_init
Jun 27, 2018
79ab701
Merge remote-tracking branch 'remotes/origin/master' into lazy_repo_init
Jun 28, 2018
2eaed57
drop unused import
Jun 28, 2018
3b03345
Merge remote-tracking branch 'remotes/origin/master' into lazy_repo_init
Jun 29, 2018
4b4b837
Merge remote-tracking branch 'remotes/origin/master' into lazy_repo_init
Jun 30, 2018
c89a103
move checks to ctor; rename methods for consistency
Jul 2, 2018
89e2310
Merge remote-tracking branch 'remotes/origin/master' into lazy_repo_init
Jul 2, 2018
3529a7e
fix URLRepositoryTests
Jul 3, 2018
608f644
Merge remote-tracking branch 'remotes/origin/master' into lazy_repo_init
Jul 3, 2018
bc60b22
changes due to Yannick's review - test code clean up; blob store para…
Jul 9, 2018
3a322aa
extend comment for r/o repo verification
Jul 9, 2018
f84ec61
added missed start
Jul 9, 2018
da5c07f
added missed start
Jul 10, 2018
d25b248
fix test; close blobStore if a concurrent blobStore initialization is…
Jul 10, 2018
b2ac9d8
fix thread check for unit test
Jul 11, 2018
c6b836a
fix thread check for unit test
Jul 11, 2018
d8c9593
Merge remote-tracking branch 'remotes/origin/master' into lazy_repo_init
Jul 11, 2018
8dfa60d
enforcing access to blobStore / blobContainer only to snapshot and ge…
Jul 12, 2018
ae3ee24
verifyBlobInnersAccess renamed to assertSnapshotOrGenericThread; adde…
Jul 13, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
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;
Expand All @@ -31,7 +32,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;
Expand Down Expand Up @@ -71,33 +71,44 @@ public class URLRepository extends BlobStoreRepository {

private final Environment environment;

private final URLBlobStore blobStore;

private final BlobPath basePath;

private final URL url;

/**
* 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;
basePath = BlobPath.cleanPath();
url = URL_SETTING.exists(metadata.settings())
? URL_SETTING.get(metadata.settings()) : REPOSITORIES_URL_SETTING.get(settings);
}

URL url = URL_SETTING.exists(metadata.settings()) ? URL_SETTING.get(metadata.settings()) : REPOSITORIES_URL_SETTING.get(settings);
@Override
protected BlobStore createBlobStore() {
URL normalizedURL = checkURL(url);
blobStore = new URLBlobStore(settings, normalizedURL);
basePath = BlobPath.cleanPath();
return new URLBlobStore(settings, normalizedURL);
}

// only use for testing
@Override
protected BlobContainer blobContainer() {
return super.blobContainer();
}

// only use for testing
@Override
protected BlobStore blobStore() {
return blobStore;
protected BlobStore getBlobStore() {
return super.getBlobStore();
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,22 @@
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 {

private URLRepository createRepository(Settings baseSettings, RepositoryMetaData repositoryMetaData) {
return new URLRepository(repositoryMetaData, TestEnvironment.newEnvironment(baseSettings),
new NamedXContentRegistry(Collections.emptyList())) {
@Override
protected void assertSnapshotOrGenericThread() {
// eliminate thread name check as we create repo manually on test/main threads
}
};
}

public void testWhiteListingRepoURL() throws IOException {
String repoPath = createTempDir().resolve("repository").toUri().toURL().toString();
Settings baseSettings = Settings.builder()
Expand All @@ -41,8 +55,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),
new NamedXContentRegistry(Collections.emptyList()));
final URLRepository repository = createRepository(baseSettings, repositoryMetaData);
repository.start();

assertThat("blob store has to be lazy initialized", repository.getBlobStore(), is(nullValue()));
repository.blobContainer();
assertThat("blobContainer has to initialize blob store", repository.getBlobStore(), not(nullValue()));
}

public void testIfNotWhiteListedMustSetRepoURL() throws IOException {
Expand All @@ -52,9 +70,10 @@ public void testIfNotWhiteListedMustSetRepoURL() throws IOException {
.put(URLRepository.REPOSITORIES_URL_SETTING.getKey(), repoPath)
.build();
RepositoryMetaData repositoryMetaData = new RepositoryMetaData("url", URLRepository.TYPE, baseSettings);
final URLRepository repository = createRepository(baseSettings, repositoryMetaData);
repository.start();
try {
new URLRepository(repositoryMetaData, TestEnvironment.newEnvironment(baseSettings),
new NamedXContentRegistry(Collections.emptyList()));
repository.blobContainer();
fail("RepositoryException should have been thrown.");
} catch (RepositoryException e) {
String msg = "[url] file url [" + repoPath
Expand All @@ -73,13 +92,33 @@ public void testMustBeSupportedProtocol() throws IOException {
.put(URLRepository.SUPPORTED_PROTOCOLS_SETTING.getKey(), "http,https")
.build();
RepositoryMetaData repositoryMetaData = new RepositoryMetaData("url", URLRepository.TYPE, baseSettings);
final URLRepository repository = createRepository(baseSettings, repositoryMetaData);
repository.start();
try {
new URLRepository(repositoryMetaData, TestEnvironment.newEnvironment(baseSettings),
new NamedXContentRegistry(Collections.emptyList()));
repository.blobContainer();
fail("RepositoryException should have been thrown.");
} catch (RepositoryException e) {
assertEquals("[url] unsupported url protocol [file] from URL [" + repoPath +"]", e.getMessage());
}
}

public void testNonNormalizedUrl() throws IOException {
Settings baseSettings = Settings.builder()
.put(Environment.PATH_HOME_SETTING.getKey(), createTempDir().toString())
.put(URLRepository.ALLOWED_URLS_SETTING.getKey(), "file:/tmp/")
.put(URLRepository.REPOSITORIES_URL_SETTING.getKey(), "file:/var/" )
.build();
RepositoryMetaData repositoryMetaData = new RepositoryMetaData("url", URLRepository.TYPE, baseSettings);
final URLRepository repository = createRepository(baseSettings, repositoryMetaData);
repository.start();
try {
repository.blobContainer();
fail("RepositoryException should have been thrown.");
} catch (RepositoryException e) {
assertEquals("[url] file url [file:/var/] doesn't match any of the locations "
+ "specified by path.repo or repositories.url.allowed_urls",
e.getMessage());
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,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;
Expand Down Expand Up @@ -78,25 +77,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 Environment environment;
private final AzureStorageService storageService;
private final 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
Expand All @@ -108,15 +103,33 @@ 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));

// 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
final LocationMode locationMode = Repository.LOCATION_MODE_SETTING.get(metadata.settings());
if (Repository.READONLY_SETTING.exists(metadata.settings())) {
this.readonly = Repository.READONLY_SETTING.get(metadata.settings());
} else {
this.readonly = locationMode == LocationMode.SECONDARY_ONLY;
}
}

// only use for testing
@Override
protected BlobStore getBlobStore() {
Copy link
Contributor

Choose a reason for hiding this comment

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

can this be removed now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nope, it is in use in AzureRepositorySettingsTests

return super.getBlobStore();
}

/**
* {@inheritDoc}
*/
@Override
protected BlobStore blobStore() {
protected AzureBlobStore createBlobStore() throws URISyntaxException, StorageException {
final AzureBlobStore blobStore = new AzureBlobStore(metadata, environment.settings(), storageService);

logger.debug((org.apache.logging.log4j.util.Supplier<?>) () -> new ParameterizedMessage(
"using container [{}], chunk_size [{}], compress [{}], base_path [{}]",
blobStore, chunkSize, compress, basePath));
return blobStore;
}

Expand Down Expand Up @@ -144,6 +157,7 @@ protected ByteSizeValue chunkSize() {
@Override
public void initializeSnapshot(SnapshotId snapshotId, List<IndexId> indices, MetaData clusterMetadata) {
try {
final AzureBlobStore blobStore = (AzureBlobStore) 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.");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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.getBlobStore(), 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());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,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.unit.ByteSizeUnit;
import org.elasticsearch.common.unit.ByteSizeValue;
Expand Down Expand Up @@ -56,18 +55,19 @@ class GoogleCloudStorageRepository extends BlobStoreRepository {
byteSizeSetting("chunk_size", MAX_CHUNK_SIZE, MIN_CHUNK_SIZE, MAX_CHUNK_SIZE, Property.NodeScope, Property.Dynamic);
static final Setting<String> CLIENT_NAME = new Setting<>("client", "default", Function.identity());

private final ByteSizeValue chunkSize;
private final boolean compress;
private final GoogleCloudStorageService storageService;
private final BlobPath basePath;
private final GoogleCloudStorageBlobStore blobStore;
private final boolean compress;
private final ByteSizeValue chunkSize;
private final String bucket;
private final String clientName;

GoogleCloudStorageRepository(RepositoryMetaData metadata, Environment environment,
NamedXContentRegistry namedXContentRegistry,
GoogleCloudStorageService storageService) throws Exception {
GoogleCloudStorageService storageService) {
super(metadata, environment.settings(), namedXContentRegistry);
this.storageService = storageService;

String bucket = getSetting(BUCKET, metadata);
String clientName = CLIENT_NAME.get(metadata.settings());
String basePath = BASE_PATH.get(metadata.settings());
if (Strings.hasLength(basePath)) {
BlobPath path = new BlobPath();
Expand All @@ -81,16 +81,14 @@ class GoogleCloudStorageRepository extends BlobStoreRepository {

this.compress = getSetting(COMPRESS, metadata);
this.chunkSize = getSetting(CHUNK_SIZE, metadata);

this.bucket = getSetting(BUCKET, metadata);
this.clientName = CLIENT_NAME.get(metadata.settings());
logger.debug("using bucket [{}], base_path [{}], chunk_size [{}], compress [{}]", bucket, basePath, chunkSize, compress);

this.blobStore = new GoogleCloudStorageBlobStore(settings, bucket, clientName, storageService);
}


@Override
protected BlobStore blobStore() {
return blobStore;
protected GoogleCloudStorageBlobStore createBlobStore() {
return new GoogleCloudStorageBlobStore(settings, bucket, clientName, storageService);
}

@Override
Expand Down
Loading