Skip to content

Commit 0599822

Browse files
author
Ali Beyad
committed
Adding repository index generational files
Before, a repository would maintain an index file (named 'index') per repository, that contained the current snapshots in the repository. This file was not atomically written, so repositories had to depend on listing the blobs in the repository to determine what the current snapshots are, and only rely on the index file if the repository does not support the listBlobs operation. This could cause an incorrect view of the current snapshots in the repository if any prior snapshot delete operations failed to delete snapshot metadata files. This commit introduces the atomic writing of the index file, and because atomic writes are not guaranteed if the file already exists, we write to a generational index file (index-N, where N is the current generation). We also maintain an index-latest file that contains the current generation, for those repositories that cannot list blobs. Closes #19002 Relates #18156
1 parent 62397c0 commit 0599822

File tree

9 files changed

+261
-236
lines changed

9 files changed

+261
-236
lines changed

‎core/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -366,7 +366,6 @@ public void apply(Settings value, Settings current, Settings previous) {
366366
Node.NODE_INGEST_SETTING,
367367
Node.NODE_ATTRIBUTES,
368368
URLRepository.ALLOWED_URLS_SETTING,
369-
URLRepository.REPOSITORIES_LIST_DIRECTORIES_SETTING,
370369
URLRepository.REPOSITORIES_URL_SETTING,
371370
URLRepository.SUPPORTED_PROTOCOLS_SETTING,
372371
TransportMasterNodeReadAction.FORCE_LOCAL_SETTING,

‎core/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java‎

Lines changed: 182 additions & 143 deletions
Large diffs are not rendered by default.

‎core/src/main/java/org/elasticsearch/repositories/uri/URLRepository.java‎

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919

2020
package org.elasticsearch.repositories.uri;
2121

22-
import org.elasticsearch.snapshots.SnapshotId;
2322
import org.elasticsearch.common.blobstore.BlobPath;
2423
import org.elasticsearch.common.blobstore.BlobStore;
2524
import org.elasticsearch.common.blobstore.url.URLBlobStore;
@@ -42,7 +41,6 @@
4241
import java.util.Collections;
4342
import java.util.List;
4443
import java.util.function.Function;
45-
import java.util.function.Predicate;
4644

4745
/**
4846
* Read-only URL-based implementation of the BlobStoreRepository
@@ -69,11 +67,6 @@ public class URLRepository extends BlobStoreRepository {
6967
new Setting<>("repositories.url.url", (s) -> s.get("repositories.uri.url", "http:"), URLRepository::parseURL,
7068
Property.NodeScope);
7169

72-
public static final Setting<Boolean> LIST_DIRECTORIES_SETTING =
73-
Setting.boolSetting("list_directories", true, Property.NodeScope);
74-
public static final Setting<Boolean> REPOSITORIES_LIST_DIRECTORIES_SETTING =
75-
Setting.boolSetting("repositories.uri.list_directories", true, Property.NodeScope);
76-
7770
private final List<String> supportedProtocols;
7871

7972
private final URIPattern[] urlWhiteList;
@@ -84,8 +77,6 @@ public class URLRepository extends BlobStoreRepository {
8477

8578
private final BlobPath basePath;
8679

87-
private boolean listDirectories;
88-
8980
/**
9081
* Constructs new read-only URL-based repository
9182
*
@@ -103,7 +94,6 @@ public URLRepository(RepositoryName name, RepositorySettings repositorySettings,
10394
supportedProtocols = SUPPORTED_PROTOCOLS_SETTING.get(settings);
10495
urlWhiteList = ALLOWED_URLS_SETTING.get(settings).toArray(new URIPattern[]{});
10596
this.environment = environment;
106-
listDirectories = LIST_DIRECTORIES_SETTING.exists(repositorySettings.settings()) ? LIST_DIRECTORIES_SETTING.get(repositorySettings.settings()) : REPOSITORIES_LIST_DIRECTORIES_SETTING.get(settings);
10797

10898
URL url = URL_SETTING.exists(repositorySettings.settings()) ? URL_SETTING.get(repositorySettings.settings()) : REPOSITORIES_URL_SETTING.get(settings);
10999
URL normalizedURL = checkURL(url);
@@ -124,19 +114,6 @@ protected BlobPath basePath() {
124114
return basePath;
125115
}
126116

127-
@Override
128-
public List<SnapshotId> snapshots() {
129-
if (listDirectories) {
130-
return super.snapshots();
131-
} else {
132-
try {
133-
return readSnapshotList();
134-
} catch (IOException ex) {
135-
throw new RepositoryException(repositoryName, "failed to get snapshot list in repository", ex);
136-
}
137-
}
138-
}
139-
140117
/**
141118
* Makes sure that the url is white listed or if it points to the local file system it matches one on of the root path in path.repo
142119
*/

‎core/src/test/java/org/elasticsearch/repositories/blobstore/BlobStoreRepositoryTests.java‎

Lines changed: 59 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@
2424
import org.elasticsearch.client.Client;
2525
import org.elasticsearch.common.UUIDs;
2626
import org.elasticsearch.common.bytes.BytesReference;
27-
import org.elasticsearch.common.collect.Tuple;
2827
import org.elasticsearch.common.io.stream.BytesStreamOutput;
2928
import org.elasticsearch.common.io.stream.OutputStreamStreamOutput;
3029
import org.elasticsearch.common.io.stream.StreamOutput;
@@ -46,7 +45,6 @@
4645
import java.util.stream.Collectors;
4746

4847
import static org.elasticsearch.repositories.blobstore.BlobStoreRepository.blobId;
49-
import static org.elasticsearch.repositories.blobstore.BlobStoreRepository.parseNameUUIDFromBlobName;
5048
import static org.hamcrest.Matchers.equalTo;
5149

5250
/**
@@ -108,53 +106,57 @@ public void testRetrieveSnapshots() throws Exception {
108106
assertThat(snapshotIds, equalTo(originalSnapshots));
109107
}
110108

111-
public void testSnapshotIndexFile() throws Exception {
112-
final Client client = client();
113-
final Path location = ESIntegTestCase.randomRepoPath(node().settings());
114-
final String repositoryName = "test-repo";
115-
116-
PutRepositoryResponse putRepositoryResponse =
117-
client.admin().cluster().preparePutRepository(repositoryName)
118-
.setType("fs")
119-
.setSettings(Settings.builder().put(node().settings()).put("location", location))
120-
.get();
121-
assertThat(putRepositoryResponse.isAcknowledged(), equalTo(true));
122-
123-
final RepositoriesService repositoriesService = getInstanceFromNode(RepositoriesService.class);
124-
@SuppressWarnings("unchecked") final BlobStoreRepository repository =
125-
(BlobStoreRepository) repositoriesService.repository(repositoryName);
109+
public void testReadAndWriteSnapshotsThroughIndexFile() throws Exception {
110+
final BlobStoreRepository repository = setupRepo();
126111

127112
// write to and read from a snapshot file with no entries
128-
repository.writeSnapshotList(Collections.emptyList());
129-
List<SnapshotId> readSnapshotIds = repository.readSnapshotList();
130-
assertThat(readSnapshotIds.size(), equalTo(0));
113+
assertThat(repository.snapshots().size(), equalTo(0));
114+
repository.writeSnapshotsToIndexGen(Collections.emptyList());
115+
assertThat(repository.snapshots().size(), equalTo(0));
131116

132117
// write to and read from a snapshot file with a random number of entries
133118
final int numSnapshots = randomIntBetween(1, 1000);
134119
final List<SnapshotId> snapshotIds = new ArrayList<>(numSnapshots);
135120
for (int i = 0; i < numSnapshots; i++) {
136121
snapshotIds.add(new SnapshotId(randomAsciiOfLength(8), UUIDs.randomBase64UUID()));
137122
}
138-
repository.writeSnapshotList(snapshotIds);
139-
readSnapshotIds = repository.readSnapshotList();
140-
assertThat(readSnapshotIds, equalTo(snapshotIds));
123+
repository.writeSnapshotsToIndexGen(snapshotIds);
124+
assertThat(repository.snapshots(), equalTo(snapshotIds));
141125
}
142126

143-
public void testOldIndexFileFormat() throws Exception {
144-
final Client client = client();
145-
final Path location = ESIntegTestCase.randomRepoPath(node().settings());
146-
final String repositoryName = "test-repo";
127+
public void testIndexGenerationalFiles() throws Exception {
128+
final BlobStoreRepository repository = setupRepo();
147129

148-
PutRepositoryResponse putRepositoryResponse =
149-
client.admin().cluster().preparePutRepository(repositoryName)
150-
.setType("fs")
151-
.setSettings(Settings.builder().put(node().settings()).put("location", location))
152-
.get();
153-
assertThat(putRepositoryResponse.isAcknowledged(), equalTo(true));
130+
// write to index generational file
131+
final int numSnapshots = randomIntBetween(1, 1000);
132+
final List<SnapshotId> snapshotIds = new ArrayList<>(numSnapshots);
133+
for (int i = 0; i < numSnapshots; i++) {
134+
snapshotIds.add(new SnapshotId(randomAsciiOfLength(8), UUIDs.randomBase64UUID()));
135+
}
136+
repository.writeSnapshotsToIndexGen(snapshotIds);
137+
assertThat(Sets.newHashSet(repository.readSnapshotsFromIndex()), equalTo(Sets.newHashSet(snapshotIds)));
138+
assertThat(repository.latestIndexBlobId(), equalTo(0L));
139+
assertThat(repository.readSnapshotIndexLatestBlob(), equalTo(0L));
154140

155-
final RepositoriesService repositoriesService = getInstanceFromNode(RepositoriesService.class);
156-
@SuppressWarnings("unchecked") final BlobStoreRepository repository =
157-
(BlobStoreRepository) repositoriesService.repository(repositoryName);
141+
// adding more and writing to a new index generational file
142+
for (int i = 0; i < 10; i++) {
143+
snapshotIds.add(new SnapshotId(randomAsciiOfLength(8), UUIDs.randomBase64UUID()));
144+
}
145+
repository.writeSnapshotsToIndexGen(snapshotIds);
146+
assertThat(Sets.newHashSet(repository.readSnapshotsFromIndex()), equalTo(Sets.newHashSet(snapshotIds)));
147+
assertThat(repository.latestIndexBlobId(), equalTo(1L));
148+
assertThat(repository.readSnapshotIndexLatestBlob(), equalTo(1L));
149+
150+
// removing a snapshot adn writing to a new index generational file
151+
snapshotIds.remove(0);
152+
repository.writeSnapshotsToIndexGen(snapshotIds);
153+
assertThat(Sets.newHashSet(repository.readSnapshotsFromIndex()), equalTo(Sets.newHashSet(snapshotIds)));
154+
assertThat(repository.latestIndexBlobId(), equalTo(2L));
155+
assertThat(repository.readSnapshotIndexLatestBlob(), equalTo(2L));
156+
}
157+
158+
public void testOldIndexFileFormat() throws Exception {
159+
final BlobStoreRepository repository = setupRepo();
158160

159161
// write old index file format
160162
final int numOldSnapshots = randomIntBetween(1, 50);
@@ -163,36 +165,15 @@ public void testOldIndexFileFormat() throws Exception {
163165
snapshotIds.add(new SnapshotId(randomAsciiOfLength(8), SnapshotId.UNASSIGNED_UUID));
164166
}
165167
writeOldFormat(repository, snapshotIds.stream().map(SnapshotId::getName).collect(Collectors.toList()));
166-
List<SnapshotId> readSnapshotIds = repository.readSnapshotList();
167-
assertThat(Sets.newHashSet(readSnapshotIds), equalTo(Sets.newHashSet(snapshotIds)));
168+
assertThat(Sets.newHashSet(repository.snapshots()), equalTo(Sets.newHashSet(snapshotIds)));
168169

169170
// write to and read from a snapshot file with a random number of new entries added
170171
final int numSnapshots = randomIntBetween(1, 1000);
171172
for (int i = 0; i < numSnapshots; i++) {
172173
snapshotIds.add(new SnapshotId(randomAsciiOfLength(8), UUIDs.randomBase64UUID()));
173174
}
174-
repository.writeSnapshotList(snapshotIds);
175-
readSnapshotIds = repository.readSnapshotList();
176-
assertThat(Sets.newHashSet(readSnapshotIds), equalTo(Sets.newHashSet(snapshotIds)));
177-
}
178-
179-
public void testParseUUIDFromBlobName() {
180-
String blobStr = "abc123";
181-
Tuple<String, String> pair = parseNameUUIDFromBlobName(blobStr);
182-
assertThat(pair.v1(), equalTo(blobStr)); // snapshot name
183-
assertThat(pair.v2(), equalTo(SnapshotId.UNASSIGNED_UUID)); // snapshot uuid
184-
blobStr = "abcefghijklmnopqrstuvwxyz";
185-
pair = parseNameUUIDFromBlobName(blobStr);
186-
assertThat(pair.v1(), equalTo(blobStr));
187-
assertThat(pair.v2(), equalTo(SnapshotId.UNASSIGNED_UUID));
188-
blobStr = "abc123-xyz"; // not enough characters after '-' to have a uuid
189-
pair = parseNameUUIDFromBlobName(blobStr);
190-
assertThat(pair.v1(), equalTo(blobStr));
191-
assertThat(pair.v2(), equalTo(SnapshotId.UNASSIGNED_UUID));
192-
blobStr = "abc123-a1b2c3d4e5f6g7h8i9j0k1";
193-
pair = parseNameUUIDFromBlobName(blobStr);
194-
assertThat(pair.v1(), equalTo("abc123"));
195-
assertThat(pair.v2(), equalTo("a1b2c3d4e5f6g7h8i9j0k1"));
175+
repository.writeSnapshotsToIndexGen(snapshotIds);
176+
assertThat(Sets.newHashSet(repository.snapshots()), equalTo(Sets.newHashSet(snapshotIds)));
196177
}
197178

198179
public void testBlobId() {
@@ -208,6 +189,24 @@ public void testBlobId() {
208189
assertThat(blobId(snapshotId), equalTo("abc-123-" + uuid)); // snapshot name + '-' + uuid
209190
}
210191

192+
private BlobStoreRepository setupRepo() {
193+
final Client client = client();
194+
final Path location = ESIntegTestCase.randomRepoPath(node().settings());
195+
final String repositoryName = "test-repo";
196+
197+
PutRepositoryResponse putRepositoryResponse =
198+
client.admin().cluster().preparePutRepository(repositoryName)
199+
.setType("fs")
200+
.setSettings(Settings.builder().put(node().settings()).put("location", location))
201+
.get();
202+
assertThat(putRepositoryResponse.isAcknowledged(), equalTo(true));
203+
204+
final RepositoriesService repositoriesService = getInstanceFromNode(RepositoriesService.class);
205+
@SuppressWarnings("unchecked") final BlobStoreRepository repository =
206+
(BlobStoreRepository) repositoriesService.repository(repositoryName);
207+
return repository;
208+
}
209+
211210
private void writeOldFormat(final BlobStoreRepository repository, final List<String> snapshotNames) throws Exception {
212211
final BytesReference bRef;
213212
try (BytesStreamOutput bStream = new BytesStreamOutput()) {

‎core/src/test/java/org/elasticsearch/snapshots/DedicatedClusterSnapshotRestoreIT.java‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -284,8 +284,8 @@ public void clusterStateProcessed(String source, ClusterState oldState, ClusterS
284284
countDownLatch.await();
285285
}
286286

287-
private static interface ClusterStateUpdater {
288-
public ClusterState execute(ClusterState currentState) throws Exception;
287+
private interface ClusterStateUpdater {
288+
ClusterState execute(ClusterState currentState) throws Exception;
289289
}
290290

291291
public void testSnapshotDuringNodeShutdown() throws Exception {
@@ -392,8 +392,11 @@ public void testSnapshotWithStuckNode() throws Exception {
392392

393393
logger.info("--> making sure that snapshot no longer exists");
394394
assertThrows(client().admin().cluster().prepareGetSnapshots("test-repo").setSnapshots("test-snap").execute(), SnapshotMissingException.class);
395-
// Subtract index file from the count
396-
assertThat("not all files were deleted during snapshot cancellation", numberOfFilesBeforeSnapshot, equalTo(numberOfFiles(repo) - 1));
395+
// Subtract three files that will remain in the repository:
396+
// (1) index-1
397+
// (2) index-0 (because we keep the previous version) and
398+
// (3) index-latest
399+
assertThat("not all files were deleted during snapshot cancellation", numberOfFilesBeforeSnapshot, equalTo(numberOfFiles(repo) - 3));
397400
logger.info("--> done");
398401
}
399402

‎core/src/test/java/org/elasticsearch/snapshots/SharedClusterSnapshotRestoreIT.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -818,8 +818,9 @@ public void testDeleteSnapshot() throws Exception {
818818

819819
logger.info("--> delete the last snapshot");
820820
client.admin().cluster().prepareDeleteSnapshot("test-repo", lastSnapshot).get();
821-
logger.info("--> make sure that number of files is back to what it was when the first snapshot was made");
822-
assertThat(numberOfFiles(repo), equalTo(numberOfFiles[0]));
821+
logger.info("--> make sure that number of files is back to what it was when the first snapshot was made, " +
822+
"plus one because one backup index-N file should remain");
823+
assertThat(numberOfFiles(repo), equalTo(numberOfFiles[0] + 1));
823824
}
824825

825826
public void testDeleteSnapshotWithMissingIndexAndShardMetadata() throws Exception {

‎plugins/repository-azure/src/main/java/org/elasticsearch/cloud/azure/blobstore/AzureBlobStore.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import org.elasticsearch.repositories.RepositoryName;
3535
import org.elasticsearch.repositories.RepositorySettings;
3636

37+
import java.io.IOException;
3738
import java.io.InputStream;
3839
import java.io.OutputStream;
3940
import java.net.URISyntaxException;
@@ -127,7 +128,7 @@ public void deleteBlob(String container, String blob) throws URISyntaxException,
127128
this.client.deleteBlob(this.accountName, this.locMode, container, blob);
128129
}
129130

130-
public InputStream getInputStream(String container, String blob) throws URISyntaxException, StorageException
131+
public InputStream getInputStream(String container, String blob) throws URISyntaxException, StorageException, IOException
131132
{
132133
return this.client.getInputStream(this.accountName, this.locMode, container, blob);
133134
}

‎plugins/repository-azure/src/main/java/org/elasticsearch/cloud/azure/storage/AzureStorageService.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import org.elasticsearch.common.unit.ByteSizeValue;
3030
import org.elasticsearch.common.unit.TimeValue;
3131

32+
import java.io.IOException;
3233
import java.io.InputStream;
3334
import java.io.OutputStream;
3435
import java.net.URISyntaxException;
@@ -75,7 +76,7 @@ final class Storage {
7576
void deleteBlob(String account, LocationMode mode, String container, String blob) throws URISyntaxException, StorageException;
7677

7778
InputStream getInputStream(String account, LocationMode mode, String container, String blob)
78-
throws URISyntaxException, StorageException;
79+
throws URISyntaxException, StorageException, IOException;
7980

8081
OutputStream getOutputStream(String account, LocationMode mode, String container, String blob)
8182
throws URISyntaxException, StorageException;

‎plugins/repository-azure/src/test/java/org/elasticsearch/cloud/azure/storage/AzureStorageServiceMock.java‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@
3131

3232
import java.io.ByteArrayInputStream;
3333
import java.io.ByteArrayOutputStream;
34+
import java.io.FileNotFoundException;
35+
import java.io.IOException;
3436
import java.io.InputStream;
3537
import java.io.OutputStream;
3638
import java.net.URISyntaxException;
@@ -79,7 +81,10 @@ public void deleteBlob(String account, LocationMode mode, String container, Stri
7981
}
8082

8183
@Override
82-
public InputStream getInputStream(String account, LocationMode mode, String container, String blob) {
84+
public InputStream getInputStream(String account, LocationMode mode, String container, String blob) throws IOException {
85+
if (!blobExists(account, mode, container, blob)) {
86+
throw new FileNotFoundException("missing blob [" + blob + "]");
87+
}
8388
return new ByteArrayInputStream(blobs.get(blob).toByteArray());
8489
}
8590

0 commit comments

Comments
 (0)