Skip to content

Commit c12d232

Browse files
s1monws1monw
authored
Pass Directory instead of DirectoryService to Store (#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 79cd638 commit c12d232

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

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

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

Lines changed: 5 additions & 8 deletions
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

Lines changed: 1 addition & 8 deletions
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

Lines changed: 20 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -104,12 +104,10 @@ public class StoreTests extends ESTestCase {
104104
private static final Version MIN_SUPPORTED_LUCENE_VERSION = org.elasticsearch.Version.CURRENT
105105
.minimumIndexCompatibilityVersion().luceneVersion;
106106

107-
public void testRefCount() throws IOException {
107+
public void testRefCount() {
108108
final ShardId shardId = new ShardId("index", "_na_", 1);
109-
DirectoryService directoryService = new LuceneManagedDirectoryService(random());
110109
IndexSettings indexSettings = INDEX_SETTINGS;
111-
112-
Store store = new Store(shardId, indexSettings, directoryService, new DummyShardLock(shardId));
110+
Store store = new Store(shardId, indexSettings, StoreTests.newDirectory(random()), new DummyShardLock(shardId));
113111
int incs = randomIntBetween(1, 100);
114112
for (int i = 0; i < incs; i++) {
115113
if (randomBoolean()) {
@@ -296,8 +294,7 @@ public void testVerifyingIndexOutputWithBogusInput() throws IOException {
296294

297295
public void testNewChecksums() throws IOException {
298296
final ShardId shardId = new ShardId("index", "_na_", 1);
299-
DirectoryService directoryService = new LuceneManagedDirectoryService(random());
300-
Store store = new Store(shardId, INDEX_SETTINGS, directoryService, new DummyShardLock(shardId));
297+
Store store = new Store(shardId, INDEX_SETTINGS, StoreTests.newDirectory(random()), new DummyShardLock(shardId));
301298
// set default codec - all segments need checksums
302299
IndexWriter writer = new IndexWriter(store.directory(), newIndexWriterConfig(random(), new MockAnalyzer(random())).setCodec(TestUtil.getDefaultCodec()));
303300
int docs = 1 + random().nextInt(100);
@@ -347,7 +344,7 @@ public void testNewChecksums() throws IOException {
347344
assertConsistent(store, metadata);
348345

349346
TestUtil.checkIndex(store.directory());
350-
assertDeleteContent(store, directoryService);
347+
assertDeleteContent(store, store.directory());
351348
IOUtils.close(store);
352349
}
353350

@@ -455,32 +452,11 @@ private void corruptFile(Directory dir, String fileIn, String fileOut) throws IO
455452

456453
}
457454

458-
public void assertDeleteContent(Store store, DirectoryService service) throws IOException {
455+
public void assertDeleteContent(Store store, Directory dir) throws IOException {
459456
deleteContent(store.directory());
460457
assertThat(Arrays.toString(store.directory().listAll()), store.directory().listAll().length, equalTo(0));
461458
assertThat(store.stats().sizeInBytes(), equalTo(0L));
462-
assertThat(service.newDirectory().listAll().length, equalTo(0));
463-
}
464-
465-
private static final class LuceneManagedDirectoryService extends DirectoryService {
466-
private final Directory dir;
467-
private final Random random;
468-
469-
LuceneManagedDirectoryService(Random random) {
470-
this(random, true);
471-
}
472-
473-
LuceneManagedDirectoryService(Random random, boolean preventDoubleWrite) {
474-
super(new ShardId(INDEX_SETTINGS.getIndex(), 1), INDEX_SETTINGS);
475-
dir = StoreTests.newDirectory(random);
476-
this.random = random;
477-
}
478-
479-
@Override
480-
public Directory newDirectory() throws IOException {
481-
return dir;
482-
}
483-
459+
assertThat(dir.listAll().length, equalTo(0));
484460
}
485461

486462
public static void assertConsistent(Store store, Store.MetadataSnapshot metadata) throws IOException {
@@ -511,8 +487,7 @@ public void testRecoveryDiff() throws IOException, InterruptedException {
511487
iwc.setMergePolicy(NoMergePolicy.INSTANCE);
512488
iwc.setUseCompoundFile(random.nextBoolean());
513489
final ShardId shardId = new ShardId("index", "_na_", 1);
514-
DirectoryService directoryService = new LuceneManagedDirectoryService(random);
515-
Store store = new Store(shardId, INDEX_SETTINGS, directoryService, new DummyShardLock(shardId));
490+
Store store = new Store(shardId, INDEX_SETTINGS, StoreTests.newDirectory(random()), new DummyShardLock(shardId));
516491
IndexWriter writer = new IndexWriter(store.directory(), iwc);
517492
final boolean lotsOfSegments = rarely(random);
518493
for (Document d : docs) {
@@ -526,7 +501,7 @@ public void testRecoveryDiff() throws IOException, InterruptedException {
526501
writer.commit();
527502
writer.close();
528503
first = store.getMetadata(null);
529-
assertDeleteContent(store, directoryService);
504+
assertDeleteContent(store, store.directory());
530505
store.close();
531506
}
532507
long time = new Date().getTime();
@@ -541,8 +516,7 @@ public void testRecoveryDiff() throws IOException, InterruptedException {
541516
iwc.setMergePolicy(NoMergePolicy.INSTANCE);
542517
iwc.setUseCompoundFile(random.nextBoolean());
543518
final ShardId shardId = new ShardId("index", "_na_", 1);
544-
DirectoryService directoryService = new LuceneManagedDirectoryService(random);
545-
store = new Store(shardId, INDEX_SETTINGS, directoryService, new DummyShardLock(shardId));
519+
store = new Store(shardId, INDEX_SETTINGS, StoreTests.newDirectory(random()), new DummyShardLock(shardId));
546520
IndexWriter writer = new IndexWriter(store.directory(), iwc);
547521
final boolean lotsOfSegments = rarely(random);
548522
for (Document d : docs) {
@@ -639,8 +613,7 @@ public void testRecoveryDiff() throws IOException, InterruptedException {
639613

640614
public void testCleanupFromSnapshot() throws IOException {
641615
final ShardId shardId = new ShardId("index", "_na_", 1);
642-
DirectoryService directoryService = new LuceneManagedDirectoryService(random());
643-
Store store = new Store(shardId, INDEX_SETTINGS, directoryService, new DummyShardLock(shardId));
616+
Store store = new Store(shardId, INDEX_SETTINGS, StoreTests.newDirectory(random()), new DummyShardLock(shardId));
644617
// this time random codec....
645618
IndexWriterConfig indexWriterConfig = newIndexWriterConfig(random(), new MockAnalyzer(random())).setCodec(TestUtil.getDefaultCodec());
646619
// we keep all commits and that allows us clean based on multiple snapshots
@@ -727,11 +700,10 @@ public void testCleanupFromSnapshot() throws IOException {
727700

728701
public void testOnCloseCallback() throws IOException {
729702
final ShardId shardId = new ShardId(new Index(randomRealisticUnicodeOfCodepointLengthBetween(1, 10), "_na_"), randomIntBetween(0, 100));
730-
DirectoryService directoryService = new LuceneManagedDirectoryService(random());
731703
final AtomicInteger count = new AtomicInteger(0);
732704
final ShardLock lock = new DummyShardLock(shardId);
733705

734-
Store store = new Store(shardId, INDEX_SETTINGS, directoryService, lock, theLock -> {
706+
Store store = new Store(shardId, INDEX_SETTINGS, StoreTests.newDirectory(random()), lock, theLock -> {
735707
assertEquals(shardId, theLock.getShardId());
736708
assertEquals(lock, theLock);
737709
count.incrementAndGet();
@@ -748,11 +720,10 @@ public void testOnCloseCallback() throws IOException {
748720

749721
public void testStoreStats() throws IOException {
750722
final ShardId shardId = new ShardId("index", "_na_", 1);
751-
DirectoryService directoryService = new LuceneManagedDirectoryService(random());
752723
Settings settings = Settings.builder()
753724
.put(IndexMetaData.SETTING_VERSION_CREATED, org.elasticsearch.Version.CURRENT)
754725
.put(Store.INDEX_STORE_STATS_REFRESH_INTERVAL_SETTING.getKey(), TimeValue.timeValueMinutes(0)).build();
755-
Store store = new Store(shardId, IndexSettingsModule.newIndexSettings("index", settings), directoryService,
726+
Store store = new Store(shardId, IndexSettingsModule.newIndexSettings("index", settings), StoreTests.newDirectory(random()),
756727
new DummyShardLock(shardId));
757728
long initialStoreSize = 0;
758729
for (String extraFiles : store.directory().listAll()) {
@@ -843,8 +814,7 @@ protected Store.MetadataSnapshot createMetaDataSnapshot() {
843814

844815
public void testUserDataRead() throws IOException {
845816
final ShardId shardId = new ShardId("index", "_na_", 1);
846-
DirectoryService directoryService = new LuceneManagedDirectoryService(random());
847-
Store store = new Store(shardId, INDEX_SETTINGS, directoryService, new DummyShardLock(shardId));
817+
Store store = new Store(shardId, INDEX_SETTINGS, StoreTests.newDirectory(random()), new DummyShardLock(shardId));
848818
IndexWriterConfig config = newIndexWriterConfig(random(), new MockAnalyzer(random())).setCodec(TestUtil.getDefaultCodec());
849819
SnapshotDeletionPolicy deletionPolicy = new SnapshotDeletionPolicy(new KeepOnlyLastCommitDeletionPolicy());
850820
config.setIndexDeletionPolicy(deletionPolicy);
@@ -867,7 +837,7 @@ public void testUserDataRead() throws IOException {
867837
assertThat(metadata.getCommitUserData().get(Engine.SYNC_COMMIT_ID), equalTo(syncId));
868838
assertThat(metadata.getCommitUserData().get(Translog.TRANSLOG_GENERATION_KEY), equalTo(translogId));
869839
TestUtil.checkIndex(store.directory());
870-
assertDeleteContent(store, directoryService);
840+
assertDeleteContent(store, store.directory());
871841
IOUtils.close(store);
872842
}
873843

@@ -893,8 +863,7 @@ public void testStreamStoreFilesMetaData() throws Exception {
893863
public void testMarkCorruptedOnTruncatedSegmentsFile() throws IOException {
894864
IndexWriterConfig iwc = newIndexWriterConfig();
895865
final ShardId shardId = new ShardId("index", "_na_", 1);
896-
DirectoryService directoryService = new LuceneManagedDirectoryService(random());
897-
Store store = new Store(shardId, INDEX_SETTINGS, directoryService, new DummyShardLock(shardId));
866+
Store store = new Store(shardId, INDEX_SETTINGS, StoreTests.newDirectory(random()), new DummyShardLock(shardId));
898867
IndexWriter writer = new IndexWriter(store.directory(), iwc);
899868

900869
int numDocs = 1 + random().nextInt(10);
@@ -945,15 +914,7 @@ public void testCanOpenIndex() throws IOException {
945914
writer.commit();
946915
writer.close();
947916
assertTrue(Store.canOpenIndex(logger, tempDir, shardId, (id, l) -> new DummyShardLock(id)));
948-
949-
DirectoryService directoryService = new DirectoryService(shardId, INDEX_SETTINGS) {
950-
951-
@Override
952-
public Directory newDirectory() throws IOException {
953-
return dir;
954-
}
955-
};
956-
Store store = new Store(shardId, INDEX_SETTINGS, directoryService, new DummyShardLock(shardId));
917+
Store store = new Store(shardId, INDEX_SETTINGS, dir, new DummyShardLock(shardId));
957918
store.markStoreCorrupted(new CorruptIndexException("foo", "bar"));
958919
assertFalse(Store.canOpenIndex(logger, tempDir, shardId, (id, l) -> new DummyShardLock(id)));
959920
store.close();
@@ -962,14 +923,7 @@ public Directory newDirectory() throws IOException {
962923
public void testDeserializeCorruptionException() throws IOException {
963924
final ShardId shardId = new ShardId("index", "_na_", 1);
964925
final Directory dir = new RAMDirectory(); // I use ram dir to prevent that virusscanner being a PITA
965-
DirectoryService directoryService = new DirectoryService(shardId, INDEX_SETTINGS) {
966-
967-
@Override
968-
public Directory newDirectory() throws IOException {
969-
return dir;
970-
}
971-
};
972-
Store store = new Store(shardId, INDEX_SETTINGS, directoryService, new DummyShardLock(shardId));
926+
Store store = new Store(shardId, INDEX_SETTINGS, dir, new DummyShardLock(shardId));
973927
CorruptIndexException ex = new CorruptIndexException("foo", "bar");
974928
store.markStoreCorrupted(ex);
975929
try {
@@ -998,14 +952,7 @@ public Directory newDirectory() throws IOException {
998952
public void testCanReadOldCorruptionMarker() throws IOException {
999953
final ShardId shardId = new ShardId("index", "_na_", 1);
1000954
final Directory dir = new RAMDirectory(); // I use ram dir to prevent that virusscanner being a PITA
1001-
DirectoryService directoryService = new DirectoryService(shardId, INDEX_SETTINGS) {
1002-
1003-
@Override
1004-
public Directory newDirectory() throws IOException {
1005-
return dir;
1006-
}
1007-
};
1008-
Store store = new Store(shardId, INDEX_SETTINGS, directoryService, new DummyShardLock(shardId));
955+
Store store = new Store(shardId, INDEX_SETTINGS, dir, new DummyShardLock(shardId));
1009956

1010957
CorruptIndexException exception = new CorruptIndexException("foo", "bar");
1011958
String uuid = Store.CORRUPTED + UUIDs.randomBase64UUID();
@@ -1065,8 +1012,7 @@ public Directory newDirectory() throws IOException {
10651012

10661013
public void testEnsureIndexHasHistoryUUID() throws IOException {
10671014
final ShardId shardId = new ShardId("index", "_na_", 1);
1068-
DirectoryService directoryService = new LuceneManagedDirectoryService(random());
1069-
try (Store store = new Store(shardId, INDEX_SETTINGS, directoryService, new DummyShardLock(shardId))) {
1015+
try (Store store = new Store(shardId, INDEX_SETTINGS, StoreTests.newDirectory(random()), new DummyShardLock(shardId))) {
10701016

10711017
store.createEmpty();
10721018

@@ -1098,8 +1044,7 @@ public void testEnsureIndexHasHistoryUUID() throws IOException {
10981044

10991045
public void testHistoryUUIDCanBeForced() throws IOException {
11001046
final ShardId shardId = new ShardId("index", "_na_", 1);
1101-
DirectoryService directoryService = new LuceneManagedDirectoryService(random());
1102-
try (Store store = new Store(shardId, INDEX_SETTINGS, directoryService, new DummyShardLock(shardId))) {
1047+
try (Store store = new Store(shardId, INDEX_SETTINGS, StoreTests.newDirectory(random()), new DummyShardLock(shardId))) {
11031048

11041049
store.createEmpty();
11051050

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

Lines changed: 5 additions & 13 deletions
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)