Skip to content

Commit 7c58371

Browse files
s1monws1monw
committed
Pass Directory instead of DirectoryService to Store (elastic#33466)
Instead of passing DirectoryService which causes yet another dependency on Store we can just pass in a Directory since we will just call `DirectoryService#newDirectory()` on it anyway.
1 parent 53f4f2f commit 7c58371

File tree

8 files changed

+39
-130
lines changed

8 files changed

+39
-130
lines changed

‎server/src/main/java/org/elasticsearch/index/IndexService.java

+4-1
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@
6363
import org.elasticsearch.index.shard.ShardNotFoundException;
6464
import org.elasticsearch.index.shard.ShardPath;
6565
import org.elasticsearch.index.similarity.SimilarityService;
66+
import org.elasticsearch.index.store.DirectoryService;
6667
import org.elasticsearch.index.store.IndexStore;
6768
import org.elasticsearch.index.store.Store;
6869
import org.elasticsearch.index.translog.Translog;
@@ -376,7 +377,9 @@ public synchronized IndexShard createShard(ShardRouting routing, Consumer<ShardI
376377
warmer.warm(searcher, shard, IndexService.this.indexSettings);
377378
}
378379
};
379-
store = new Store(shardId, this.indexSettings, indexStore.newDirectoryService(path), lock,
380+
// TODO we can remove either IndexStore or DirectoryService. All we need is a simple Supplier<Directory>
381+
DirectoryService directoryService = indexStore.newDirectoryService(path);
382+
store = new Store(shardId, this.indexSettings, directoryService.newDirectory(), lock,
380383
new StoreCloseListener(shardId, () -> eventListener.onStoreClosed(shardId)));
381384
indexShard = new IndexShard(routing, this.indexSettings, path, store, indexSortSupplier,
382385
indexCache, mapperService, similarityService, engineFactory,

‎server/src/main/java/org/elasticsearch/index/store/Store.java

+5-8
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,6 @@
6464
import org.elasticsearch.common.lucene.store.InputStreamIndexInput;
6565
import org.elasticsearch.common.settings.Setting;
6666
import org.elasticsearch.common.settings.Setting.Property;
67-
import org.elasticsearch.common.settings.Settings;
6867
import org.elasticsearch.common.unit.TimeValue;
6968
import org.elasticsearch.common.util.concurrent.AbstractRefCounted;
7069
import org.elasticsearch.common.util.concurrent.RefCounted;
@@ -153,18 +152,16 @@ protected void closeInternal() {
153152
}
154153
};
155154

156-
public Store(ShardId shardId, IndexSettings indexSettings, DirectoryService directoryService, ShardLock shardLock) throws IOException {
157-
this(shardId, indexSettings, directoryService, shardLock, OnClose.EMPTY);
155+
public Store(ShardId shardId, IndexSettings indexSettings, Directory directory, ShardLock shardLock) {
156+
this(shardId, indexSettings, directory, shardLock, OnClose.EMPTY);
158157
}
159158

160-
public Store(ShardId shardId, IndexSettings indexSettings, DirectoryService directoryService, ShardLock shardLock,
161-
OnClose onClose) throws IOException {
159+
public Store(ShardId shardId, IndexSettings indexSettings, Directory directory, ShardLock shardLock,
160+
OnClose onClose) {
162161
super(shardId, indexSettings);
163-
final Settings settings = indexSettings.getSettings();
164-
Directory dir = directoryService.newDirectory();
165162
final TimeValue refreshInterval = indexSettings.getValue(INDEX_STORE_STATS_REFRESH_INTERVAL_SETTING);
166163
logger.debug("store stats are refreshed with refresh_interval [{}]", refreshInterval);
167-
ByteSizeCachingDirectory sizeCachingDir = new ByteSizeCachingDirectory(dir, refreshInterval);
164+
ByteSizeCachingDirectory sizeCachingDir = new ByteSizeCachingDirectory(directory, refreshInterval);
168165
this.directory = new StoreDirectory(sizeCachingDir, Loggers.getLogger("index.store.deletes", shardId));
169166
this.shardLock = shardLock;
170167
this.onClose = onClose;

‎server/src/test/java/org/elasticsearch/index/shard/RefreshListenersTests.java

+1-8
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,6 @@
5050
import org.elasticsearch.index.mapper.ParsedDocument;
5151
import org.elasticsearch.index.mapper.SeqNoFieldMapper;
5252
import org.elasticsearch.index.seqno.SequenceNumbers;
53-
import org.elasticsearch.index.store.DirectoryService;
5453
import org.elasticsearch.index.store.Store;
5554
import org.elasticsearch.index.translog.Translog;
5655
import org.elasticsearch.index.translog.TranslogConfig;
@@ -106,13 +105,7 @@ public void setupListeners() throws Exception {
106105
ShardId shardId = new ShardId(new Index("index", "_na_"), 1);
107106
String allocationId = UUIDs.randomBase64UUID(random());
108107
Directory directory = newDirectory();
109-
DirectoryService directoryService = new DirectoryService(shardId, indexSettings) {
110-
@Override
111-
public Directory newDirectory() throws IOException {
112-
return directory;
113-
}
114-
};
115-
store = new Store(shardId, indexSettings, directoryService, new DummyShardLock(shardId));
108+
store = new Store(shardId, indexSettings, directory, new DummyShardLock(shardId));
116109
IndexWriterConfig iwc = newIndexWriterConfig();
117110
TranslogConfig translogConfig = new TranslogConfig(shardId, createTempDir("translog"), indexSettings,
118111
BigArrays.NON_RECYCLING_INSTANCE);

‎server/src/test/java/org/elasticsearch/index/store/StoreTests.java

+20-75
Original file line numberDiff line numberDiff line change
@@ -108,12 +108,10 @@ public class StoreTests extends ESTestCase {
108108
private static final Version MIN_SUPPORTED_LUCENE_VERSION = org.elasticsearch.Version.CURRENT
109109
.minimumIndexCompatibilityVersion().luceneVersion;
110110

111-
public void testRefCount() throws IOException {
111+
public void testRefCount() {
112112
final ShardId shardId = new ShardId("index", "_na_", 1);
113-
DirectoryService directoryService = new LuceneManagedDirectoryService(random());
114113
IndexSettings indexSettings = INDEX_SETTINGS;
115-
116-
Store store = new Store(shardId, indexSettings, directoryService, new DummyShardLock(shardId));
114+
Store store = new Store(shardId, indexSettings, StoreTests.newDirectory(random()), new DummyShardLock(shardId));
117115
int incs = randomIntBetween(1, 100);
118116
for (int i = 0; i < incs; i++) {
119117
if (randomBoolean()) {
@@ -310,8 +308,7 @@ public void testVerifyingIndexOutputWithBogusInput() throws IOException {
310308

311309
public void testNewChecksums() throws IOException {
312310
final ShardId shardId = new ShardId("index", "_na_", 1);
313-
DirectoryService directoryService = new LuceneManagedDirectoryService(random());
314-
Store store = new Store(shardId, INDEX_SETTINGS, directoryService, new DummyShardLock(shardId));
311+
Store store = new Store(shardId, INDEX_SETTINGS, StoreTests.newDirectory(random()), new DummyShardLock(shardId));
315312
// set default codec - all segments need checksums
316313
IndexWriter writer = new IndexWriter(store.directory(), newIndexWriterConfig(random(), new MockAnalyzer(random())).setCodec(TestUtil.getDefaultCodec()));
317314
int docs = 1 + random().nextInt(100);
@@ -361,7 +358,7 @@ public void testNewChecksums() throws IOException {
361358
assertConsistent(store, metadata);
362359

363360
TestUtil.checkIndex(store.directory());
364-
assertDeleteContent(store, directoryService);
361+
assertDeleteContent(store, store.directory());
365362
IOUtils.close(store);
366363
}
367364

@@ -469,32 +466,11 @@ private void corruptFile(Directory dir, String fileIn, String fileOut) throws IO
469466

470467
}
471468

472-
public void assertDeleteContent(Store store, DirectoryService service) throws IOException {
469+
public void assertDeleteContent(Store store, Directory dir) throws IOException {
473470
deleteContent(store.directory());
474471
assertThat(Arrays.toString(store.directory().listAll()), store.directory().listAll().length, equalTo(0));
475472
assertThat(store.stats().sizeInBytes(), equalTo(0L));
476-
assertThat(service.newDirectory().listAll().length, equalTo(0));
477-
}
478-
479-
private static final class LuceneManagedDirectoryService extends DirectoryService {
480-
private final Directory dir;
481-
private final Random random;
482-
483-
LuceneManagedDirectoryService(Random random) {
484-
this(random, true);
485-
}
486-
487-
LuceneManagedDirectoryService(Random random, boolean preventDoubleWrite) {
488-
super(new ShardId(INDEX_SETTINGS.getIndex(), 1), INDEX_SETTINGS);
489-
dir = StoreTests.newDirectory(random);
490-
this.random = random;
491-
}
492-
493-
@Override
494-
public Directory newDirectory() throws IOException {
495-
return dir;
496-
}
497-
473+
assertThat(dir.listAll().length, equalTo(0));
498474
}
499475

500476
public static void assertConsistent(Store store, Store.MetadataSnapshot metadata) throws IOException {
@@ -525,8 +501,7 @@ public void testRecoveryDiff() throws IOException, InterruptedException {
525501
iwc.setMergePolicy(NoMergePolicy.INSTANCE);
526502
iwc.setUseCompoundFile(random.nextBoolean());
527503
final ShardId shardId = new ShardId("index", "_na_", 1);
528-
DirectoryService directoryService = new LuceneManagedDirectoryService(random);
529-
Store store = new Store(shardId, INDEX_SETTINGS, directoryService, new DummyShardLock(shardId));
504+
Store store = new Store(shardId, INDEX_SETTINGS, StoreTests.newDirectory(random()), new DummyShardLock(shardId));
530505
IndexWriter writer = new IndexWriter(store.directory(), iwc);
531506
final boolean lotsOfSegments = rarely(random);
532507
for (Document d : docs) {
@@ -540,7 +515,7 @@ public void testRecoveryDiff() throws IOException, InterruptedException {
540515
writer.commit();
541516
writer.close();
542517
first = store.getMetadata(null);
543-
assertDeleteContent(store, directoryService);
518+
assertDeleteContent(store, store.directory());
544519
store.close();
545520
}
546521
long time = new Date().getTime();
@@ -555,8 +530,7 @@ public void testRecoveryDiff() throws IOException, InterruptedException {
555530
iwc.setMergePolicy(NoMergePolicy.INSTANCE);
556531
iwc.setUseCompoundFile(random.nextBoolean());
557532
final ShardId shardId = new ShardId("index", "_na_", 1);
558-
DirectoryService directoryService = new LuceneManagedDirectoryService(random);
559-
store = new Store(shardId, INDEX_SETTINGS, directoryService, new DummyShardLock(shardId));
533+
store = new Store(shardId, INDEX_SETTINGS, StoreTests.newDirectory(random()), new DummyShardLock(shardId));
560534
IndexWriter writer = new IndexWriter(store.directory(), iwc);
561535
final boolean lotsOfSegments = rarely(random);
562536
for (Document d : docs) {
@@ -653,8 +627,7 @@ public void testRecoveryDiff() throws IOException, InterruptedException {
653627

654628
public void testCleanupFromSnapshot() throws IOException {
655629
final ShardId shardId = new ShardId("index", "_na_", 1);
656-
DirectoryService directoryService = new LuceneManagedDirectoryService(random());
657-
Store store = new Store(shardId, INDEX_SETTINGS, directoryService, new DummyShardLock(shardId));
630+
Store store = new Store(shardId, INDEX_SETTINGS, StoreTests.newDirectory(random()), new DummyShardLock(shardId));
658631
// this time random codec....
659632
IndexWriterConfig indexWriterConfig = newIndexWriterConfig(random(), new MockAnalyzer(random())).setCodec(TestUtil.getDefaultCodec());
660633
// we keep all commits and that allows us clean based on multiple snapshots
@@ -741,11 +714,10 @@ public void testCleanupFromSnapshot() throws IOException {
741714

742715
public void testOnCloseCallback() throws IOException {
743716
final ShardId shardId = new ShardId(new Index(randomRealisticUnicodeOfCodepointLengthBetween(1, 10), "_na_"), randomIntBetween(0, 100));
744-
DirectoryService directoryService = new LuceneManagedDirectoryService(random());
745717
final AtomicInteger count = new AtomicInteger(0);
746718
final ShardLock lock = new DummyShardLock(shardId);
747719

748-
Store store = new Store(shardId, INDEX_SETTINGS, directoryService, lock, theLock -> {
720+
Store store = new Store(shardId, INDEX_SETTINGS, StoreTests.newDirectory(random()), lock, theLock -> {
749721
assertEquals(shardId, theLock.getShardId());
750722
assertEquals(lock, theLock);
751723
count.incrementAndGet();
@@ -762,11 +734,10 @@ public void testOnCloseCallback() throws IOException {
762734

763735
public void testStoreStats() throws IOException {
764736
final ShardId shardId = new ShardId("index", "_na_", 1);
765-
DirectoryService directoryService = new LuceneManagedDirectoryService(random());
766737
Settings settings = Settings.builder()
767738
.put(IndexMetaData.SETTING_VERSION_CREATED, org.elasticsearch.Version.CURRENT)
768739
.put(Store.INDEX_STORE_STATS_REFRESH_INTERVAL_SETTING.getKey(), TimeValue.timeValueMinutes(0)).build();
769-
Store store = new Store(shardId, IndexSettingsModule.newIndexSettings("index", settings), directoryService,
740+
Store store = new Store(shardId, IndexSettingsModule.newIndexSettings("index", settings), StoreTests.newDirectory(random()),
770741
new DummyShardLock(shardId));
771742
long initialStoreSize = 0;
772743
for (String extraFiles : store.directory().listAll()) {
@@ -857,8 +828,7 @@ protected Store.MetadataSnapshot createMetaDataSnapshot() {
857828

858829
public void testUserDataRead() throws IOException {
859830
final ShardId shardId = new ShardId("index", "_na_", 1);
860-
DirectoryService directoryService = new LuceneManagedDirectoryService(random());
861-
Store store = new Store(shardId, INDEX_SETTINGS, directoryService, new DummyShardLock(shardId));
831+
Store store = new Store(shardId, INDEX_SETTINGS, StoreTests.newDirectory(random()), new DummyShardLock(shardId));
862832
IndexWriterConfig config = newIndexWriterConfig(random(), new MockAnalyzer(random())).setCodec(TestUtil.getDefaultCodec());
863833
SnapshotDeletionPolicy deletionPolicy = new SnapshotDeletionPolicy(new KeepOnlyLastCommitDeletionPolicy());
864834
config.setIndexDeletionPolicy(deletionPolicy);
@@ -881,7 +851,7 @@ public void testUserDataRead() throws IOException {
881851
assertThat(metadata.getCommitUserData().get(Engine.SYNC_COMMIT_ID), equalTo(syncId));
882852
assertThat(metadata.getCommitUserData().get(Translog.TRANSLOG_GENERATION_KEY), equalTo(translogId));
883853
TestUtil.checkIndex(store.directory());
884-
assertDeleteContent(store, directoryService);
854+
assertDeleteContent(store, store.directory());
885855
IOUtils.close(store);
886856
}
887857

@@ -907,8 +877,7 @@ public void testStreamStoreFilesMetaData() throws Exception {
907877
public void testMarkCorruptedOnTruncatedSegmentsFile() throws IOException {
908878
IndexWriterConfig iwc = newIndexWriterConfig();
909879
final ShardId shardId = new ShardId("index", "_na_", 1);
910-
DirectoryService directoryService = new LuceneManagedDirectoryService(random());
911-
Store store = new Store(shardId, INDEX_SETTINGS, directoryService, new DummyShardLock(shardId));
880+
Store store = new Store(shardId, INDEX_SETTINGS, StoreTests.newDirectory(random()), new DummyShardLock(shardId));
912881
IndexWriter writer = new IndexWriter(store.directory(), iwc);
913882

914883
int numDocs = 1 + random().nextInt(10);
@@ -959,15 +928,7 @@ public void testCanOpenIndex() throws IOException {
959928
writer.commit();
960929
writer.close();
961930
assertTrue(Store.canOpenIndex(logger, tempDir, shardId, (id, l) -> new DummyShardLock(id)));
962-
963-
DirectoryService directoryService = new DirectoryService(shardId, INDEX_SETTINGS) {
964-
965-
@Override
966-
public Directory newDirectory() throws IOException {
967-
return dir;
968-
}
969-
};
970-
Store store = new Store(shardId, INDEX_SETTINGS, directoryService, new DummyShardLock(shardId));
931+
Store store = new Store(shardId, INDEX_SETTINGS, dir, new DummyShardLock(shardId));
971932
store.markStoreCorrupted(new CorruptIndexException("foo", "bar"));
972933
assertFalse(Store.canOpenIndex(logger, tempDir, shardId, (id, l) -> new DummyShardLock(id)));
973934
store.close();
@@ -976,14 +937,7 @@ public Directory newDirectory() throws IOException {
976937
public void testDeserializeCorruptionException() throws IOException {
977938
final ShardId shardId = new ShardId("index", "_na_", 1);
978939
final Directory dir = new RAMDirectory(); // I use ram dir to prevent that virusscanner being a PITA
979-
DirectoryService directoryService = new DirectoryService(shardId, INDEX_SETTINGS) {
980-
981-
@Override
982-
public Directory newDirectory() throws IOException {
983-
return dir;
984-
}
985-
};
986-
Store store = new Store(shardId, INDEX_SETTINGS, directoryService, new DummyShardLock(shardId));
940+
Store store = new Store(shardId, INDEX_SETTINGS, dir, new DummyShardLock(shardId));
987941
CorruptIndexException ex = new CorruptIndexException("foo", "bar");
988942
store.markStoreCorrupted(ex);
989943
try {
@@ -1012,14 +966,7 @@ public Directory newDirectory() throws IOException {
1012966
public void testCanReadOldCorruptionMarker() throws IOException {
1013967
final ShardId shardId = new ShardId("index", "_na_", 1);
1014968
final Directory dir = new RAMDirectory(); // I use ram dir to prevent that virusscanner being a PITA
1015-
DirectoryService directoryService = new DirectoryService(shardId, INDEX_SETTINGS) {
1016-
1017-
@Override
1018-
public Directory newDirectory() throws IOException {
1019-
return dir;
1020-
}
1021-
};
1022-
Store store = new Store(shardId, INDEX_SETTINGS, directoryService, new DummyShardLock(shardId));
969+
Store store = new Store(shardId, INDEX_SETTINGS, dir, new DummyShardLock(shardId));
1023970

1024971
CorruptIndexException exception = new CorruptIndexException("foo", "bar");
1025972
String uuid = Store.CORRUPTED + UUIDs.randomBase64UUID();
@@ -1079,8 +1026,7 @@ public Directory newDirectory() throws IOException {
10791026

10801027
public void testEnsureIndexHas6xCommitTags() throws IOException {
10811028
final ShardId shardId = new ShardId("index", "_na_", 1);
1082-
DirectoryService directoryService = new LuceneManagedDirectoryService(random());
1083-
try (Store store = new Store(shardId, INDEX_SETTINGS, directoryService, new DummyShardLock(shardId))) {
1029+
try (Store store = new Store(shardId, INDEX_SETTINGS, StoreTests.newDirectory(random()), new DummyShardLock(shardId))) {
10841030
store.createEmpty();
10851031
// remove the history uuid, seqno markers
10861032
try (IndexWriter writer = openWriter(store)) {
@@ -1145,8 +1091,7 @@ public void testEnsureIndexHas6xCommitTags() throws IOException {
11451091

11461092
public void testHistoryUUIDCanBeForced() throws IOException {
11471093
final ShardId shardId = new ShardId("index", "_na_", 1);
1148-
DirectoryService directoryService = new LuceneManagedDirectoryService(random());
1149-
try (Store store = new Store(shardId, INDEX_SETTINGS, directoryService, new DummyShardLock(shardId))) {
1094+
try (Store store = new Store(shardId, INDEX_SETTINGS, StoreTests.newDirectory(random()), new DummyShardLock(shardId))) {
11501095

11511096
store.createEmpty();
11521097

‎server/src/test/java/org/elasticsearch/indices/recovery/RecoverySourceHandlerTests.java

+5-13
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,6 @@
6363
import org.elasticsearch.index.shard.IndexShardRelocatedException;
6464
import org.elasticsearch.index.shard.IndexShardState;
6565
import org.elasticsearch.index.shard.ShardId;
66-
import org.elasticsearch.index.store.DirectoryService;
6766
import org.elasticsearch.index.store.Store;
6867
import org.elasticsearch.index.store.StoreFileMetaData;
6968
import org.elasticsearch.index.translog.Translog;
@@ -461,18 +460,11 @@ private Store newStore(Path path) throws IOException {
461460
return newStore(path, true);
462461
}
463462
private Store newStore(Path path, boolean checkIndex) throws IOException {
464-
DirectoryService directoryService = new DirectoryService(shardId, INDEX_SETTINGS) {
465-
466-
@Override
467-
public Directory newDirectory() throws IOException {
468-
BaseDirectoryWrapper baseDirectoryWrapper = RecoverySourceHandlerTests.newFSDirectory(path);
469-
if (checkIndex == false) {
470-
baseDirectoryWrapper.setCheckIndexOnClose(false); // don't run checkindex we might corrupt the index in these tests
471-
}
472-
return baseDirectoryWrapper;
473-
}
474-
};
475-
return new Store(shardId, INDEX_SETTINGS, directoryService, new DummyShardLock(shardId));
463+
BaseDirectoryWrapper baseDirectoryWrapper = RecoverySourceHandlerTests.newFSDirectory(path);
464+
if (checkIndex == false) {
465+
baseDirectoryWrapper.setCheckIndexOnClose(false); // don't run checkindex we might corrupt the index in these tests
466+
}
467+
return new Store(shardId, INDEX_SETTINGS, baseDirectoryWrapper, new DummyShardLock(shardId));
476468
}
477469

478470

0 commit comments

Comments
 (0)