Skip to content

Commit faede0a

Browse files
rjernstrjernst
authored
Make NodeEnvironment.availableShardPaths singular (elastic#72441)
This commit renames the availableShardPaths method to be singular and return a single Path instead of an array. relates elastic#71205
1 parent 2a446ad commit faede0a

19 files changed

Lines changed: 83 additions & 112 deletions

File tree

‎server/src/internalClusterTest/java/org/elasticsearch/cluster/allocation/ClusterRerouteIT.java‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@
4848
import org.elasticsearch.test.InternalTestCluster;
4949
import org.elasticsearch.test.MockLogAppender;
5050

51+
import java.nio.file.Files;
5152
import java.nio.file.Path;
5253
import java.util.Arrays;
5354
import java.util.List;
@@ -266,12 +267,12 @@ private void rerouteWithAllocateLocalGateway(Settings commonSettings) throws Exc
266267
final Index index = resolveIndex("test");
267268

268269
logger.info("--> closing all nodes");
269-
Path[] shardLocation = internalCluster().getInstance(NodeEnvironment.class, node_1).availableShardPaths(new ShardId(index, 0));
270+
Path shardLocation = internalCluster().getInstance(NodeEnvironment.class, node_1).availableShardPath(new ShardId(index, 0));
270271
assertThat(FileSystemUtils.exists(shardLocation), equalTo(true)); // make sure the data is there!
271272
internalCluster().closeNonSharedNodes(false); // don't wipe data directories the index needs to be there!
272273

273-
logger.info("--> deleting the shard data [{}] ", Arrays.toString(shardLocation));
274-
assertThat(FileSystemUtils.exists(shardLocation), equalTo(true)); // verify again after cluster was shut down
274+
logger.info("--> deleting the shard data [{}] ", shardLocation);
275+
assertThat(Files.exists(shardLocation), equalTo(true)); // verify again after cluster was shut down
275276
IOUtils.rm(shardLocation);
276277

277278
logger.info("--> starting nodes back, will not allocate the shard since it has no data, but the index will be there");

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

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -547,19 +547,17 @@ public Settings onNodeStopped(String nodeName) throws Exception {
547547
});
548548

549549
if (corrupt) {
550-
for (Path path : internalCluster().getInstance(NodeEnvironment.class, nodeName).availableShardPaths(shardId)) {
551-
final Path indexPath = path.resolve(ShardPath.INDEX_FOLDER_NAME);
552-
if (Files.exists(indexPath)) { // multi data path might only have one path in use
553-
try (DirectoryStream<Path> stream = Files.newDirectoryStream(indexPath)) {
554-
for (Path item : stream) {
555-
if (item.getFileName().toString().startsWith("segments_")) {
556-
logger.debug("--> deleting [{}]", item);
557-
Files.delete(item);
558-
}
550+
Path path = internalCluster().getInstance(NodeEnvironment.class, nodeName).availableShardPath(shardId);
551+
final Path indexPath = path.resolve(ShardPath.INDEX_FOLDER_NAME);
552+
if (Files.exists(indexPath)) { // multi data path might only have one path in use
553+
try (DirectoryStream<Path> stream = Files.newDirectoryStream(indexPath)) {
554+
for (Path item : stream) {
555+
if (item.getFileName().toString().startsWith("segments_")) {
556+
logger.debug("--> deleting [{}]", item);
557+
Files.delete(item);
559558
}
560559
}
561560
}
562-
563561
}
564562
}
565563

‎server/src/internalClusterTest/java/org/elasticsearch/index/shard/IndexShardIT.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -130,11 +130,11 @@ public void testLockTryingToDelete() throws Exception {
130130

131131
ClusterService cs = getInstanceFromNode(ClusterService.class);
132132
final Index index = cs.state().metadata().index("test").getIndex();
133-
Path[] shardPaths = env.availableShardPaths(new ShardId(index, 0));
134-
logger.info("--> paths: [{}]", (Object)shardPaths);
133+
Path shardPath = env.availableShardPath(new ShardId(index, 0));
134+
logger.info("--> path: [{}]", shardPath);
135135
// Should not be able to acquire the lock because it's already open
136136
try {
137-
NodeEnvironment.acquireFSLockForPaths(IndexSettingsModule.newIndexSettings("test", Settings.EMPTY), shardPaths);
137+
NodeEnvironment.acquireFSLockForPaths(IndexSettingsModule.newIndexSettings("test", Settings.EMPTY), shardPath);
138138
fail("should not have been able to acquire the lock");
139139
} catch (LockObtainFailedException e) {
140140
assertTrue("msg: " + e.getMessage(), e.getMessage().contains("unable to acquire write.lock"));

‎server/src/internalClusterTest/java/org/elasticsearch/index/store/CorruptedFileIT.java‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -607,14 +607,12 @@ private int numShards(String... index) {
607607

608608
private List<Path> findFilesToCorruptOnNode(final String nodeName, final ShardId shardId) throws IOException {
609609
List<Path> files = new ArrayList<>();
610-
for (Path path : internalCluster().getInstance(NodeEnvironment.class, nodeName).availableShardPaths(shardId)) {
611-
path = path.resolve("index");
612-
if (Files.exists(path)) { // multi data path might only have one path in use
613-
try (DirectoryStream<Path> stream = Files.newDirectoryStream(path)) {
614-
for (Path item : stream) {
615-
if (item.getFileName().toString().startsWith("segments_")) {
616-
files.add(item);
617-
}
610+
Path path = internalCluster().getInstance(NodeEnvironment.class, nodeName).availableShardPath(shardId).resolve("index");
611+
if (Files.exists(path)) { // multi data path might only have one path in use
612+
try (DirectoryStream<Path> stream = Files.newDirectoryStream(path)) {
613+
for (Path item : stream) {
614+
if (item.getFileName().toString().startsWith("segments_")) {
615+
files.add(item);
618616
}
619617
}
620618
}

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

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -457,9 +457,7 @@ private Path indexDirectory(String server, Index index) {
457457

458458
private Path shardDirectory(String server, Index index, int shard) {
459459
NodeEnvironment env = internalCluster().getInstance(NodeEnvironment.class, server);
460-
final Path[] paths = env.availableShardPaths(new ShardId(index, shard));
461-
assert paths.length == 1;
462-
return paths[0];
460+
return env.availableShardPath(new ShardId(index, shard));
463461
}
464462

465463
private void assertShardDeleted(final String server, final Index index, final int shard) throws Exception {

‎server/src/internalClusterTest/java/org/elasticsearch/recovery/RelocationIT.java‎

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -424,18 +424,17 @@ public void testCancellationCleansTempFiles() throws Exception {
424424
logger.info("--> verifying no temporary recoveries are left");
425425
for (String node : internalCluster().getNodeNames()) {
426426
NodeEnvironment nodeEnvironment = internalCluster().getInstance(NodeEnvironment.class, node);
427-
for (final Path shardLoc : nodeEnvironment.availableShardPaths(new ShardId(indexName, "_na_", 0))) {
428-
if (Files.exists(shardLoc)) {
429-
assertBusy(() -> {
430-
try {
431-
forEachFileRecursively(shardLoc,
432-
(file, attrs) -> assertThat("found a temporary recovery file: " + file, file.getFileName().toString(),
433-
not(startsWith("recovery."))));
434-
} catch (IOException e) {
435-
throw new AssertionError("failed to walk file tree starting at [" + shardLoc + "]", e);
436-
}
437-
});
438-
}
427+
final Path shardLoc = nodeEnvironment.availableShardPath(new ShardId(indexName, "_na_", 0));
428+
if (Files.exists(shardLoc)) {
429+
assertBusy(() -> {
430+
try {
431+
forEachFileRecursively(shardLoc,
432+
(file, attrs) -> assertThat("found a temporary recovery file: " + file, file.getFileName().toString(),
433+
not(startsWith("recovery."))));
434+
} catch (IOException e) {
435+
throw new AssertionError("failed to walk file tree starting at [" + shardLoc + "]", e);
436+
}
437+
});
439438
}
440439
}
441440
}

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

Lines changed: 12 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -549,8 +549,8 @@ public void deleteShardDirectorySafe(
549549
IndexSettings indexSettings,
550550
Consumer<Path[]> listener
551551
) throws IOException, ShardLockObtainFailedException {
552-
final Path[] paths = availableShardPaths(shardId);
553-
logger.trace("deleting shard {} directory, paths: [{}]", shardId, paths);
552+
final Path path = availableShardPath(shardId);
553+
logger.trace("deleting shard {} directory, path: [{}]", shardId, path);
554554
try (ShardLock lock = shardLock(shardId, "shard deletion under lock")) {
555555
deleteShardDirectoryUnderLock(lock, indexSettings, listener);
556556
}
@@ -604,11 +604,11 @@ public void deleteShardDirectoryUnderLock(
604604
) throws IOException {
605605
final ShardId shardId = lock.getShardId();
606606
assert isShardLocked(shardId) : "shard " + shardId + " is not locked";
607-
final Path[] paths = availableShardPaths(shardId);
608-
logger.trace("acquiring locks for {}, paths: [{}]", shardId, paths);
609-
acquireFSLockForPaths(indexSettings, paths);
610-
listener.accept(paths);
611-
IOUtils.rm(paths);
607+
final Path path = availableShardPath(shardId);
608+
logger.trace("acquiring locks for {}, path: [{}]", shardId, path);
609+
acquireFSLockForPaths(indexSettings, path);
610+
listener.accept(new Path[] { path });
611+
IOUtils.rm(path);
612612
if (indexSettings.hasCustomDataPath()) {
613613
Path customLocation = resolveCustomLocation(indexSettings.customDataPath(), shardId);
614614
logger.trace("acquiring lock for {}, custom path: [{}]", shardId, customLocation);
@@ -617,11 +617,11 @@ public void deleteShardDirectoryUnderLock(
617617
listener.accept(new Path[]{customLocation});
618618
IOUtils.rm(customLocation);
619619
}
620-
logger.trace("deleted shard {} directory, paths: [{}]", shardId, paths);
621-
assert assertPathsDoNotExist(paths);
620+
logger.trace("deleted shard {} directory, path: [{}]", shardId, path);
621+
assert assertPathsDoNotExist(path);
622622
}
623623

624-
private static boolean assertPathsDoNotExist(final Path[] paths) {
624+
private static boolean assertPathsDoNotExist(final Path... paths) {
625625
Set<Path> existingPaths = Stream.of(paths)
626626
.filter(FileSystemUtils::exists)
627627
.filter(leftOver -> {
@@ -950,14 +950,9 @@ public Path indexPath(Index index) {
950950
* @see #resolveCustomLocation(String, ShardId)
951951
*
952952
*/
953-
public Path[] availableShardPaths(ShardId shardId) {
953+
public Path availableShardPath(ShardId shardId) {
954954
assertEnvIsLocked();
955-
final NodePath[] nodePaths = nodePaths();
956-
final Path[] shardLocations = new Path[nodePaths.length];
957-
for (int i = 0; i < nodePaths.length; i++) {
958-
shardLocations[i] = nodePaths[i].resolve(shardId);
959-
}
960-
return shardLocations;
955+
return nodePaths[0].resolve(shardId);
961956
}
962957

963958
/**

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ protected NodeGatewayStartedShards nodeOperation(NodeRequest request, Task task)
9898
final ShardId shardId = request.getShardId();
9999
logger.trace("{} loading local shard state info", shardId);
100100
ShardStateMetadata shardStateMetadata = ShardStateMetadata.FORMAT.loadLatestState(logger, namedXContentRegistry,
101-
nodeEnv.availableShardPaths(request.shardId));
101+
nodeEnv.availableShardPath(request.shardId));
102102
if (shardStateMetadata != null) {
103103
if (indicesService.getShardOrNull(shardId) == null) {
104104
final String customDataPath;

‎server/src/main/java/org/elasticsearch/index/shard/ShardPath.java‎

Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -109,9 +109,9 @@ public boolean isCustomDataPath() {
109109
*/
110110
public static ShardPath loadShardPath(Logger logger, NodeEnvironment env,
111111
ShardId shardId, String customDataPath) throws IOException {
112-
final Path[] paths = env.availableShardPaths(shardId);
112+
final Path shardPath = env.availableShardPath(shardId);
113113
final Path sharedDataPath = env.sharedDataPath();
114-
return loadShardPath(logger, shardId, customDataPath, paths, sharedDataPath);
114+
return loadShardPath(logger, shardId, customDataPath, new Path[] { shardPath }, sharedDataPath);
115115
}
116116

117117
/**
@@ -170,18 +170,16 @@ public static void deleteLeftoverShardDirectory(
170170
final Consumer<Path[]> listener
171171
) throws IOException {
172172
final String indexUUID = indexSettings.getUUID();
173-
final Path[] paths = env.availableShardPaths(lock.getShardId());
174-
for (Path path : paths) {
175-
// EMPTY is safe here because we never call namedObject
176-
ShardStateMetadata load = ShardStateMetadata.FORMAT.loadLatestState(logger, NamedXContentRegistry.EMPTY, path);
177-
if (load != null) {
178-
if (load.indexUUID.equals(indexUUID) == false && IndexMetadata.INDEX_UUID_NA_VALUE.equals(load.indexUUID) == false) {
179-
logger.warn("{} deleting leftover shard on path: [{}] with a different index UUID", lock.getShardId(), path);
180-
assert Files.isDirectory(path) : path + " is not a directory";
181-
NodeEnvironment.acquireFSLockForPaths(indexSettings, path);
182-
listener.accept(new Path[]{path});
183-
IOUtils.rm(path);
184-
}
173+
final Path path = env.availableShardPath(lock.getShardId());
174+
// EMPTY is safe here because we never call namedObject
175+
ShardStateMetadata load = ShardStateMetadata.FORMAT.loadLatestState(logger, NamedXContentRegistry.EMPTY, path);
176+
if (load != null) {
177+
if (load.indexUUID.equals(indexUUID) == false && IndexMetadata.INDEX_UUID_NA_VALUE.equals(load.indexUUID) == false) {
178+
logger.warn("{} deleting leftover shard on path: [{}] with a different index UUID", lock.getShardId(), path);
179+
assert Files.isDirectory(path) : path + " is not a directory";
180+
NodeEnvironment.acquireFSLockForPaths(indexSettings, path);
181+
listener.accept(new Path[]{path});
182+
IOUtils.rm(path);
185183
}
186184
}
187185
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1083,7 +1083,7 @@ public ShardDeletionCheckResult canDeleteShardContent(ShardId shardId, IndexSett
10831083
} else {
10841084
// lets see if it's path is available (return false if the shard doesn't exist)
10851085
// we don't need to delete anything that is not there
1086-
return FileSystemUtils.exists(nodeEnv.availableShardPaths(shardId)) ?
1086+
return Files.exists(nodeEnv.availableShardPath(shardId)) ?
10871087
ShardDeletionCheckResult.FOLDER_FOUND_CAN_DELETE :
10881088
ShardDeletionCheckResult.NO_FOLDER_FOUND;
10891089
}

0 commit comments

Comments
 (0)