Skip to content

Commit 8502e5b

Browse files
authored
Use InMemoryNoOpCommitDirectory for archives indices only (#121210)
Since #118606 searchable snapshots shards are not expected to write files on disk, with the exception of archives indices mounted as searchable snapshots which require to rewrite the segment infos file in a newer version. Ideally we should be able to remove the usage of the InMemoryNoOpCommitDirectory for non-archives searchable snapshots indices and only rely on SearchableSnapshotDirectory that throws on write operations. Similarly, starting 9.0 searchable snapshots shards do not write files on disk and therefore should be able to use a Directory implementation that forbids writes. Searchable snapshots shards for indices created before 9.0 require a mutable directory for peer-recoveries. In this change, we only allow writes for archives indices and searchable snapshots created before 9.0. Relates ES-10438
1 parent 1378b59 commit 8502e5b

File tree

3 files changed

+50
-53
lines changed

3 files changed

+50
-53
lines changed

server/src/main/java/org/elasticsearch/index/shard/IndexShard.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3209,7 +3209,12 @@ void checkIndex() throws IOException {
32093209
try {
32103210
doCheckIndex();
32113211
} catch (IOException e) {
3212-
store.markStoreCorrupted(e);
3212+
if (ExceptionsHelper.unwrap(e, AlreadyClosedException.class) != null) {
3213+
// Cache-based read operations on Lucene files can throw an AlreadyClosedException wrapped into an IOException in case
3214+
// of evictions. We don't want to mark the store as corrupted for this.
3215+
} else {
3216+
store.markStoreCorrupted(e);
3217+
}
32133218
throw e;
32143219
} finally {
32153220
store.decRef();

x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/FrozenSearchableSnapshotsIntegTests.java

Lines changed: 1 addition & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,6 @@
88
package org.elasticsearch.xpack.searchablesnapshots;
99

1010
import org.apache.lucene.search.TotalHits;
11-
import org.apache.lucene.store.ByteBuffersDirectory;
12-
import org.apache.lucene.store.Directory;
13-
import org.apache.lucene.store.FilterDirectory;
1411
import org.elasticsearch.ResourceNotFoundException;
1512
import org.elasticsearch.action.admin.cluster.allocation.ClusterAllocationExplainRequest;
1613
import org.elasticsearch.action.admin.cluster.allocation.TransportClusterAllocationExplainAction;
@@ -72,14 +69,11 @@
7269
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
7370
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertNoFailuresAndResponse;
7471
import static org.elasticsearch.xpack.searchablesnapshots.SearchableSnapshots.SNAPSHOT_RECOVERY_STATE_FACTORY_KEY;
75-
import static org.hamcrest.Matchers.arrayWithSize;
7672
import static org.hamcrest.Matchers.containsString;
7773
import static org.hamcrest.Matchers.equalTo;
7874
import static org.hamcrest.Matchers.greaterThan;
79-
import static org.hamcrest.Matchers.instanceOf;
8075
import static org.hamcrest.Matchers.is;
8176
import static org.hamcrest.Matchers.not;
82-
import static org.hamcrest.Matchers.notNullValue;
8377
import static org.hamcrest.Matchers.nullValue;
8478
import static org.hamcrest.Matchers.oneOf;
8579
import static org.hamcrest.Matchers.sameInstance;
@@ -258,22 +252,8 @@ public void testCreateAndRestorePartialSearchableSnapshot() throws Exception {
258252

259253
// the original shard size from the snapshot
260254
final long originalSize = snapshotShards.get(shardRouting.getId()).getStats().getTotalSize();
261-
totalExpectedSize += originalSize;
262-
263-
final Directory unwrappedDir = FilterDirectory.unwrap(
264-
internalCluster().getInstance(IndicesService.class, getDiscoveryNodes().resolveNode(shardRouting.currentNodeId()).getName())
265-
.indexServiceSafe(shardRouting.index())
266-
.getShard(shardRouting.getId())
267-
.store()
268-
.directory()
269-
);
270-
assertThat(shardRouting.toString(), unwrappedDir, notNullValue());
271-
assertThat(shardRouting.toString(), unwrappedDir, instanceOf(ByteBuffersDirectory.class));
272-
273-
final ByteBuffersDirectory inMemoryDir = (ByteBuffersDirectory) unwrappedDir;
274-
assertThat(inMemoryDir.listAll(), arrayWithSize(0));
275-
276255
assertThat(shardRouting.toString(), store.totalDataSetSizeInBytes(), equalTo(originalSize));
256+
totalExpectedSize += originalSize;
277257
}
278258

279259
final StoreStats store = indicesStatsResponse.getTotal().getStore();

x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/store/SearchableSnapshotDirectory.java

Lines changed: 43 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import org.elasticsearch.blobcache.shared.SharedBlobCacheService;
2424
import org.elasticsearch.cluster.metadata.IndexMetadata;
2525
import org.elasticsearch.cluster.routing.RecoverySource;
26+
import org.elasticsearch.common.Strings;
2627
import org.elasticsearch.common.blobstore.BlobContainer;
2728
import org.elasticsearch.common.bytes.BytesReference;
2829
import org.elasticsearch.common.lucene.store.ByteArrayIndexInput;
@@ -34,6 +35,7 @@
3435
import org.elasticsearch.core.IOUtils;
3536
import org.elasticsearch.core.Nullable;
3637
import org.elasticsearch.index.IndexSettings;
38+
import org.elasticsearch.index.IndexVersions;
3739
import org.elasticsearch.index.shard.ShardId;
3840
import org.elasticsearch.index.shard.ShardPath;
3941
import org.elasticsearch.index.snapshots.blobstore.BlobStoreIndexShardSnapshot;
@@ -277,6 +279,10 @@ private BlobStoreIndexShardSnapshot.FileInfo fileInfo(final String name) throws
277279
@Override
278280
public final String[] listAll() {
279281
ensureOpen();
282+
return listAllFiles();
283+
}
284+
285+
private String[] listAllFiles() {
280286
return files().stream().map(BlobStoreIndexShardSnapshot.FileInfo::physicalName).sorted(String::compareTo).toArray(String[]::new);
281287
}
282288

@@ -288,42 +294,39 @@ public final long fileLength(final String name) throws IOException {
288294

289295
@Override
290296
public Set<String> getPendingDeletions() {
291-
throw unsupportedException();
297+
throw unsupportedException("getPendingDeletions");
292298
}
293299

294300
@Override
295-
public void sync(Collection<String> names) {
296-
throw unsupportedException();
297-
}
301+
public void sync(Collection<String> names) {}
298302

299303
@Override
300-
public void syncMetaData() {
301-
throw unsupportedException();
302-
}
304+
public void syncMetaData() {}
303305

304306
@Override
305307
public void deleteFile(String name) {
306-
throw unsupportedException();
308+
throw unsupportedException("deleteFile(" + name + ')');
307309
}
308310

309311
@Override
310312
public IndexOutput createOutput(String name, IOContext context) {
311-
throw unsupportedException();
313+
throw unsupportedException("createOutput(" + name + ", " + context + ')');
312314
}
313315

314316
@Override
315317
public IndexOutput createTempOutput(String prefix, String suffix, IOContext context) {
316-
throw unsupportedException();
318+
throw unsupportedException("createTempOutput(" + prefix + ", " + suffix + ", " + context + ')');
317319
}
318320

319321
@Override
320322
public void rename(String source, String dest) {
321-
throw unsupportedException();
323+
throw unsupportedException("rename(" + source + ", " + dest + ')');
322324
}
323325

324-
private static UnsupportedOperationException unsupportedException() {
325-
assert false : "this operation is not supported and should have not be called";
326-
return new UnsupportedOperationException("Searchable snapshot directory does not support this operation");
326+
private UnsupportedOperationException unsupportedException(String description) {
327+
var message = "Searchable snapshot directory does not support the operation [" + description + ']';
328+
assert false : message + ", current directory files: " + Strings.arrayToCommaDelimitedString(this.listAllFiles());
329+
return new UnsupportedOperationException(message);
327330
}
328331

329332
@Override
@@ -612,24 +615,33 @@ public static Directory create(
612615
final Path cacheDir = CacheService.getShardCachePath(shardPath).resolve(snapshotId.getUUID());
613616
Files.createDirectories(cacheDir);
614617

615-
return new InMemoryNoOpCommitDirectory(
616-
new SearchableSnapshotDirectory(
617-
blobContainerSupplier,
618-
lazySnapshot::getOrCompute,
619-
blobStoreCacheService,
620-
initialRepository.getMetadata().name(),
621-
snapshotId,
622-
indexId,
623-
shardPath.getShardId(),
624-
indexSettings.getSettings(),
625-
currentTimeNanosSupplier,
626-
cache,
627-
cacheDir,
628-
shardPath,
629-
threadPool,
630-
sharedBlobCacheService
631-
)
618+
final var dir = new SearchableSnapshotDirectory(
619+
blobContainerSupplier,
620+
lazySnapshot::getOrCompute,
621+
blobStoreCacheService,
622+
initialRepository.getMetadata().name(),
623+
snapshotId,
624+
indexId,
625+
shardPath.getShardId(),
626+
indexSettings.getSettings(),
627+
currentTimeNanosSupplier,
628+
cache,
629+
cacheDir,
630+
shardPath,
631+
threadPool,
632+
sharedBlobCacheService
632633
);
634+
635+
// Archives indices mounted as searchable snapshots always require a writeable Lucene directory in order to rewrite the segments
636+
// infos file to the latest Lucene version. Similarly, searchable snapshot indices created before 9.0.0 also require a writeable
637+
// directory because in previous versions commits were executed during recovery (to associate translogs with Lucene indices),
638+
// creating additional files that need to be sent and written to replicas during peer-recoveries. From 9.0.0 we merged a change to
639+
// skip commits creation during recovery for searchable snapshots (see https://github.com/elastic/elasticsearch/pull/118606).
640+
var version = IndexMetadata.SETTING_INDEX_VERSION_COMPATIBILITY.get(indexSettings.getSettings());
641+
if (version.before(IndexVersions.UPGRADE_TO_LUCENE_10_0_0) || indexSettings.getIndexVersionCreated().isLegacyIndexVersion()) {
642+
return new InMemoryNoOpCommitDirectory(dir);
643+
}
644+
return dir;
633645
}
634646

635647
public static SearchableSnapshotDirectory unwrapDirectory(Directory dir) {

0 commit comments

Comments
 (0)