Skip to content

Commit 2a446ad

Browse files
rjernstrjernst
authored
Make NodeEnvironment.indexPaths singular (elastic#72438)
This commit renames the indexPaths method to be singular and return a single Path instead of an array. This is done in isolation from other NodeEnvironment methods to make it reviewable. relates elastic#71205
1 parent 4bcebbc commit 2a446ad

10 files changed

Lines changed: 60 additions & 89 deletions

File tree

‎server/src/internalClusterTest/java/org/elasticsearch/gateway/MetadataNodesIT.java‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@
2424
import org.elasticsearch.test.InternalTestCluster.RestartCallback;
2525

2626
import java.nio.file.Files;
27-
import java.nio.file.Path;
2827
import java.util.List;
2928
import java.util.Map;
3029

@@ -181,12 +180,7 @@ protected void assertIndexInMetaState(final String nodeName, final String indexN
181180

182181
private boolean indexDirectoryExists(String nodeName, Index index) {
183182
NodeEnvironment nodeEnv = ((InternalTestCluster) cluster()).getInstance(NodeEnvironment.class, nodeName);
184-
for (Path path : nodeEnv.indexPaths(index)) {
185-
if (Files.exists(path)) {
186-
return true;
187-
}
188-
}
189-
return false;
183+
return Files.exists(nodeEnv.indexPath(index));
190184
}
191185

192186
private ImmutableOpenMap<String, IndexMetadata> getIndicesMetadataOnNode(String nodeName) {

‎server/src/internalClusterTest/java/org/elasticsearch/indices/store/IndicesStoreIntegrationIT.java‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -452,9 +452,7 @@ public void onFailure(String source, Exception e) {
452452

453453
private Path indexDirectory(String server, Index index) {
454454
NodeEnvironment env = internalCluster().getInstance(NodeEnvironment.class, server);
455-
final Path[] paths = env.indexPaths(index);
456-
assert paths.length == 1;
457-
return paths[0];
455+
return env.indexPath(index);
458456
}
459457

460458
private Path shardDirectory(String server, Index index, int shard) {

‎server/src/main/java/org/elasticsearch/env/NodeEnvironment.java‎

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -688,10 +688,10 @@ public void deleteIndexDirectorySafe(
688688
* @param indexSettings settings for the index being deleted
689689
*/
690690
public void deleteIndexDirectoryUnderLock(Index index, IndexSettings indexSettings, Consumer<Path[]> listener) throws IOException {
691-
final Path[] indexPaths = indexPaths(index);
692-
logger.trace("deleting index {} directory, paths({}): [{}]", index, indexPaths.length, indexPaths);
693-
listener.accept(indexPaths);
694-
IOUtils.rm(indexPaths);
691+
final Path indexPath = indexPath(index);
692+
logger.trace("deleting index {} directory: [{}]", index, indexPath);
693+
listener.accept(new Path[] { indexPath });
694+
IOUtils.rm(indexPath);
695695
if (indexSettings.hasCustomDataPath()) {
696696
Path customLocation = resolveIndexCustomLocation(indexSettings.customDataPath(), index.getUUID());
697697
logger.trace("deleting custom index {} directory [{}]", index, customLocation);
@@ -935,13 +935,9 @@ public NodePath[] nodePaths() {
935935
/**
936936
* Returns all index paths.
937937
*/
938-
public Path[] indexPaths(Index index) {
938+
public Path indexPath(Index index) {
939939
assertEnvIsLocked();
940-
Path[] indexPaths = new Path[nodePaths.length];
941-
for (int i = 0; i < nodePaths.length; i++) {
942-
indexPaths[i] = nodePaths[i].resolve(index);
943-
}
944-
return indexPaths;
940+
return nodePaths[0].resolve(index);
945941
}
946942

947943

‎server/src/main/java/org/elasticsearch/gateway/MetaStateService.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ private Tuple<Manifest, Metadata> loadFullStateBWC() throws IOException {
142142
*/
143143
@Nullable
144144
public IndexMetadata loadIndexState(Index index) throws IOException {
145-
return INDEX_METADATA_FORMAT.loadLatestState(logger, namedXContentRegistry, nodeEnv.indexPaths(index));
145+
return INDEX_METADATA_FORMAT.loadLatestState(logger, namedXContentRegistry, nodeEnv.indexPath(index));
146146
}
147147

148148
/**
@@ -205,7 +205,7 @@ public long writeIndex(String reason, IndexMetadata indexMetadata) throws WriteS
205205
logger.trace("[{}] writing state, reason [{}]", index, reason);
206206
try {
207207
long generation = INDEX_METADATA_FORMAT.write(indexMetadata,
208-
nodeEnv.indexPaths(indexMetadata.getIndex()));
208+
nodeEnv.indexPath(indexMetadata.getIndex()));
209209
logger.trace("[{}] state written", index);
210210
return generation;
211211
} catch (WriteStateException ex) {
@@ -246,7 +246,7 @@ void cleanupGlobalState(long currentGeneration) {
246246
* @param currentGeneration current state generation to keep in the index directory.
247247
*/
248248
public void cleanupIndex(Index index, long currentGeneration) {
249-
INDEX_METADATA_FORMAT.cleanupOldFiles(currentGeneration, nodeEnv.indexPaths(index));
249+
INDEX_METADATA_FORMAT.cleanupOldFiles(currentGeneration, nodeEnv.indexPath(index));
250250
}
251251

252252
/**

‎server/src/main/java/org/elasticsearch/gateway/MetadataStateFormat.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -309,7 +309,7 @@ protected Directory newDirectory(Path dir) throws IOException {
309309
* @param currentGeneration state generation to keep.
310310
* @param locations state paths.
311311
*/
312-
public void cleanupOldFiles(final long currentGeneration, Path[] locations) {
312+
public void cleanupOldFiles(final long currentGeneration, Path... locations) {
313313
final String fileNameToKeep = getStateFileName(currentGeneration);
314314
for (Path location : locations) {
315315
logger.trace("cleanupOldFiles: cleaning up {}", location);

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -346,7 +346,7 @@ public synchronized void writeDanglingIndicesInfo() {
346346
return;
347347
}
348348
try {
349-
IndexMetadata.FORMAT.writeAndCleanup(getMetadata(), nodeEnv.indexPaths(index()));
349+
IndexMetadata.FORMAT.writeAndCleanup(getMetadata(), nodeEnv.indexPath(index()));
350350
} catch (WriteStateException e) {
351351
logger.warn(() -> new ParameterizedMessage("failed to write dangling indices state for index {}", index()), e);
352352
}
@@ -358,7 +358,7 @@ public synchronized void deleteDanglingIndicesInfo() {
358358
return;
359359
}
360360
try {
361-
MetadataStateFormat.deleteMetaState(nodeEnv.indexPaths(index()));
361+
MetadataStateFormat.deleteMetaState(nodeEnv.indexPath(index()));
362362
} catch (IOException e) {
363363
logger.warn(() -> new ParameterizedMessage("failed to delete dangling indices state for index {}", index()), e);
364364
}

‎server/src/main/java/org/elasticsearch/indices/IndicesService.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -935,7 +935,7 @@ private void deleteIndexStoreIfDeletionAllowed(final String reason, final Index
935935
}
936936
// this is a pure protection to make sure this index doesn't get re-imported as a dangling index.
937937
// we should in the future rather write a tombstone rather than wiping the metadata.
938-
MetadataStateFormat.deleteMetaState(nodeEnv.indexPaths(index));
938+
MetadataStateFormat.deleteMetaState(nodeEnv.indexPath(index));
939939
}
940940
}
941941

@@ -1028,7 +1028,7 @@ public IndexMetadata verifyIndexIsDeleted(final Index index, final ClusterState
10281028
if (clusterState.metadata().index(index) != null) {
10291029
throw new IllegalStateException("Cannot delete index [" + index + "], it is still part of the cluster state.");
10301030
}
1031-
if (nodeEnv.hasNodeFile() && FileSystemUtils.exists(nodeEnv.indexPaths(index))) {
1031+
if (nodeEnv.hasNodeFile() && Files.exists(nodeEnv.indexPath(index))) {
10321032
final IndexMetadata metadata;
10331033
try {
10341034
metadata = metaStateService.loadIndexState(index);

‎server/src/test/java/org/elasticsearch/env/NodeEnvironmentTests.java‎

Lines changed: 38 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -102,10 +102,9 @@ public void testShardLock() throws Exception {
102102
} catch (ShardLockObtainFailedException ex) {
103103
// expected
104104
}
105-
for (Path path : env.indexPaths(index)) {
106-
Files.createDirectories(path.resolve("0"));
107-
Files.createDirectories(path.resolve("1"));
108-
}
105+
Path path = env.indexPath(index);
106+
Files.createDirectories(path.resolve("0"));
107+
Files.createDirectories(path.resolve("1"));
109108
try {
110109
env.lockAllForIndex(index, idxSettings, "3", randomIntBetween(0, 10));
111110
fail("shard 0 is locked");
@@ -135,10 +134,9 @@ public void testAvailableIndexFolders() throws Exception {
135134
Set<String> actualPaths = new HashSet<>();
136135
for (int i = 0; i < numIndices; i++) {
137136
Index index = new Index("foo" + i, "fooUUID" + i);
138-
for (Path path : env.indexPaths(index)) {
139-
Files.createDirectories(path.resolve(MetadataStateFormat.STATE_DIR_NAME));
140-
actualPaths.add(path.getFileName().toString());
141-
}
137+
Path path = env.indexPath(index);
138+
Files.createDirectories(path.resolve(MetadataStateFormat.STATE_DIR_NAME));
139+
actualPaths.add(path.getFileName().toString());
142140
}
143141

144142
assertThat(actualPaths, equalTo(env.availableIndexFolders()));
@@ -153,12 +151,11 @@ public void testAvailableIndexFoldersWithExclusions() throws Exception {
153151
Set<String> actualPaths = new HashSet<>();
154152
for (int i = 0; i < numIndices; i++) {
155153
Index index = new Index("foo" + i, "fooUUID" + i);
156-
for (Path path : env.indexPaths(index)) {
157-
Files.createDirectories(path.resolve(MetadataStateFormat.STATE_DIR_NAME));
158-
actualPaths.add(path.getFileName().toString());
159-
}
154+
Path path = env.indexPath(index);
155+
Files.createDirectories(path.resolve(MetadataStateFormat.STATE_DIR_NAME));
156+
actualPaths.add(path.getFileName().toString());
160157
if (randomBoolean()) {
161-
excludedPaths.add(env.indexPaths(index)[0].getFileName().toString());
158+
excludedPaths.add(env.indexPath(index).getFileName().toString());
162159
}
163160
}
164161

@@ -173,17 +170,15 @@ public void testResolveIndexFolders() throws Exception {
173170
Map<String, List<Path>> actualIndexDataPaths = new HashMap<>();
174171
for (int i = 0; i < numIndices; i++) {
175172
Index index = new Index("foo" + i, "fooUUID" + i);
176-
Path[] indexPaths = env.indexPaths(index);
177-
for (Path path : indexPaths) {
178-
Files.createDirectories(path);
179-
String fileName = path.getFileName().toString();
180-
List<Path> paths = actualIndexDataPaths.get(fileName);
181-
if (paths == null) {
182-
paths = new ArrayList<>();
183-
}
184-
paths.add(path);
185-
actualIndexDataPaths.put(fileName, paths);
173+
Path path = env.indexPath(index);
174+
Files.createDirectories(path);
175+
String fileName = path.getFileName().toString();
176+
List<Path> paths = actualIndexDataPaths.get(fileName);
177+
if (paths == null) {
178+
paths = new ArrayList<>();
186179
}
180+
paths.add(path);
181+
actualIndexDataPaths.put(fileName, paths);
187182
}
188183
for (Map.Entry<String, List<Path>> actualIndexDataPathEntry : actualIndexDataPaths.entrySet()) {
189184
List<Path> actual = actualIndexDataPathEntry.getValue();
@@ -200,20 +195,18 @@ public void testDeleteSafe() throws Exception {
200195
final ShardLock fooLock = env.shardLock(new ShardId(index, 0), "1");
201196
assertEquals(new ShardId(index, 0), fooLock.getShardId());
202197

203-
for (Path path : env.indexPaths(index)) {
204-
Files.createDirectories(path.resolve("0"));
205-
Files.createDirectories(path.resolve("1"));
206-
}
198+
Path path = env.indexPath(index);
199+
Files.createDirectories(path.resolve("0"));
200+
Files.createDirectories(path.resolve("1"));
207201

208202
expectThrows(ShardLockObtainFailedException.class,
209203
() -> env.deleteShardDirectorySafe(new ShardId(index, 0), idxSettings, shardPaths -> {
210204
assert false : "should not be called " + shardPaths;
211205
}));
212206

213-
for (Path path : env.indexPaths(index)) {
214-
assertTrue(Files.exists(path.resolve("0")));
215-
assertTrue(Files.exists(path.resolve("1")));
216-
}
207+
path = env.indexPath(index);
208+
assertTrue(Files.exists(path.resolve("0")));
209+
assertTrue(Files.exists(path.resolve("1")));
217210

218211
{
219212
SetOnce<Path[]> listener = new SetOnce<>();
@@ -224,10 +217,9 @@ public void testDeleteSafe() throws Exception {
224217
}
225218
}
226219

227-
for (Path path : env.indexPaths(index)) {
228-
assertTrue(Files.exists(path.resolve("0")));
229-
assertFalse(Files.exists(path.resolve("1")));
230-
}
220+
path = env.indexPath(index);
221+
assertTrue(Files.exists(path.resolve("0")));
222+
assertFalse(Files.exists(path.resolve("1")));
231223

232224
expectThrows(ShardLockObtainFailedException.class,
233225
() -> env.deleteIndexDirectorySafe(index, randomIntBetween(0, 10), idxSettings, indexPaths -> {
@@ -236,9 +228,8 @@ public void testDeleteSafe() throws Exception {
236228

237229
fooLock.close();
238230

239-
for (Path path : env.indexPaths(index)) {
240-
assertTrue(Files.exists(path));
241-
}
231+
path = env.indexPath(index);
232+
assertTrue(Files.exists(path));
242233

243234
final AtomicReference<Throwable> threadException = new AtomicReference<>();
244235
final CountDownLatch latch = new CountDownLatch(1);
@@ -274,12 +265,11 @@ protected void doRun() throws Exception {
274265

275266
final SetOnce<Path[]> listener = new SetOnce<>();
276267
env.deleteIndexDirectorySafe(index, 5000, idxSettings, listener::set);
277-
assertArrayEquals(env.indexPaths(index), listener.get());
268+
assertThat(listener.get()[0], equalTo(env.indexPath(index)));
278269
assertNull(threadException.get());
279270

280-
for (Path path : env.indexPaths(index)) {
281-
assertFalse(Files.exists(path));
282-
}
271+
path = env.indexPath(index);
272+
assertFalse(Files.exists(path));
283273
latch.await();
284274
assertTrue("LockedShards: " + env.lockedShards(), env.lockedShards().isEmpty());
285275
env.close();
@@ -361,7 +351,7 @@ public void testCustomDataPaths() throws Exception {
361351
equalTo(dataPath.resolve("indices/" + index.getUUID() + "/0")));
362352

363353
assertThat("index paths uses the regular template",
364-
env.indexPaths(index)[0], equalTo(dataPath.resolve("indices/" + index.getUUID())));
354+
env.indexPath(index), equalTo(dataPath.resolve("indices/" + index.getUUID())));
365355

366356
assertThat(env.availableShardPaths(sid), equalTo(env.availableShardPaths(sid)));
367357
assertThat(env.resolveCustomLocation("/tmp/foo", sid).toAbsolutePath(),
@@ -372,7 +362,7 @@ public void testCustomDataPaths() throws Exception {
372362
equalTo(dataPath.resolve("indices/" + index.getUUID() + "/0")));
373363

374364
assertThat("index paths uses the regular template",
375-
env.indexPaths(index)[0], equalTo(dataPath.resolve("indices/" + index.getUUID())));
365+
env.indexPath(index), equalTo(dataPath.resolve("indices/" + index.getUUID())));
376366

377367
env.close();
378368
}
@@ -415,10 +405,8 @@ public void testEnsureNoShardDataOrIndexMetadata() throws IOException {
415405

416406
Path indexPath;
417407
try (NodeEnvironment env = newNodeEnvironment(settings)) {
418-
for (Path path : env.indexPaths(index)) {
419-
Files.createDirectories(path.resolve(MetadataStateFormat.STATE_DIR_NAME));
420-
}
421-
indexPath = env.indexPaths(index)[0];
408+
indexPath = env.indexPath(index);
409+
Files.createDirectories(indexPath.resolve(MetadataStateFormat.STATE_DIR_NAME));
422410
}
423411

424412
verifyFailsOnMetadata(noDataNoMasterSettings, indexPath);
@@ -430,9 +418,7 @@ public void testEnsureNoShardDataOrIndexMetadata() throws IOException {
430418

431419
// test that we can create data=false env with only meta information. Also create shard data for following asserts
432420
try (NodeEnvironment env = newNodeEnvironment(noDataSettings)) {
433-
for (Path path : env.indexPaths(index)) {
434-
Files.createDirectories(path.resolve(shardDataDirName));
435-
}
421+
Files.createDirectories(env.indexPath(index).resolve(shardDataDirName));
436422
}
437423

438424
verifyFailsOnShardData(noDataSettings, indexPath, shardDataDirName);
@@ -448,9 +434,7 @@ public void testEnsureNoShardDataOrIndexMetadata() throws IOException {
448434

449435
// test that we can create data=true, master=true env. Also remove state dir to leave only shard data for following asserts
450436
try (NodeEnvironment env = newNodeEnvironment(settings)) {
451-
for (Path path : env.indexPaths(index)) {
452-
Files.delete(path.resolve(MetadataStateFormat.STATE_DIR_NAME));
453-
}
437+
Files.delete(env.indexPath(index).resolve(MetadataStateFormat.STATE_DIR_NAME));
454438
}
455439

456440
// assert that we fail on shard data even without the metadata dir.

‎server/src/test/java/org/elasticsearch/env/NodeRepurposeCommandTests.java‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -227,11 +227,10 @@ private void createIndexDataFiles(Settings settings, int shardCount, boolean wri
227227
.build());
228228
}
229229
}
230-
for (Path path : env.indexPaths(INDEX)) {
231-
for (int i = 0; i < shardCount; ++i) {
232-
Files.createDirectories(path.resolve(Integer.toString(shardDataDirNumber)));
233-
shardDataDirNumber += randomIntBetween(1,10);
234-
}
230+
Path path = env.indexPath(INDEX);
231+
for (int i = 0; i < shardCount; ++i) {
232+
Files.createDirectories(path.resolve(Integer.toString(shardDataDirNumber)));
233+
shardDataDirNumber += randomIntBetween(1,10);
235234
}
236235
}
237236
}

‎server/src/test/java/org/elasticsearch/indices/IndicesServiceTests.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121
import org.elasticsearch.cluster.metadata.Metadata;
2222
import org.elasticsearch.cluster.service.ClusterService;
2323
import org.elasticsearch.common.UUIDs;
24-
import org.elasticsearch.common.io.FileSystemUtils;
2524
import org.elasticsearch.common.settings.Setting;
2625
import org.elasticsearch.common.settings.Settings;
2726
import org.elasticsearch.common.unit.TimeValue;
@@ -57,6 +56,7 @@
5756
import org.elasticsearch.test.hamcrest.RegexMatcher;
5857

5958
import java.io.IOException;
59+
import java.nio.file.Files;
6060
import java.util.ArrayList;
6161
import java.util.Arrays;
6262
import java.util.Collection;
@@ -336,7 +336,7 @@ public void testVerifyIfIndexContentDeleted() throws Exception {
336336
.metadata(Metadata.builder(csWithIndex.metadata()).remove(index.getName()))
337337
.build();
338338
indicesService.verifyIndexIsDeleted(index, withoutIndex);
339-
assertFalse("index files should be deleted", FileSystemUtils.exists(nodeEnv.indexPaths(index)));
339+
assertFalse("index files should be deleted", Files.exists(nodeEnv.indexPath(index)));
340340
}
341341

342342
public void testDanglingIndicesWithAliasConflict() throws Exception {

0 commit comments

Comments
 (0)