Skip to content

Commit 003d9f0

Browse files
jozalajozala
authored
[Tests] Fix copying files for test cluster (elastic#124628) (elastic#124659)
In case when file with `.attach_pid` in name was stored in distribution and then deleted, the exception could stop copying/linking files without any sign of issue. The files were then missing in the cluster used in the test causing them sometimes to fail (depending on which files haven't been copied). When using `Files.walk` it is impossible to catch the IOException and continue walking through files conditionally. It has been replaced with FileVisitor implementation to be able to continue if the exception is caused by files left temporarily by JVM but no longer available.
1 parent 1a8825e commit 003d9f0

File tree

4 files changed

+149
-55
lines changed

4 files changed

+149
-55
lines changed

‎build-tools/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchNode.java

Lines changed: 39 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -61,11 +61,14 @@
6161
import java.io.UncheckedIOException;
6262
import java.net.URL;
6363
import java.nio.charset.StandardCharsets;
64+
import java.nio.file.FileVisitResult;
6465
import java.nio.file.Files;
6566
import java.nio.file.NoSuchFileException;
6667
import java.nio.file.Path;
68+
import java.nio.file.SimpleFileVisitor;
6769
import java.nio.file.StandardCopyOption;
6870
import java.nio.file.StandardOpenOption;
71+
import java.nio.file.attribute.BasicFileAttributes;
6972
import java.time.Instant;
7073
import java.util.ArrayList;
7174
import java.util.Arrays;
@@ -1297,40 +1300,47 @@ private void syncWithCopy(Path sourceRoot, Path destinationRoot) {
12971300

12981301
private void sync(Path sourceRoot, Path destinationRoot, BiConsumer<Path, Path> syncMethod) {
12991302
assert Files.exists(destinationRoot) == false;
1300-
try (Stream<Path> stream = Files.walk(sourceRoot)) {
1301-
stream.forEach(source -> {
1302-
Path relativeDestination = sourceRoot.relativize(source);
1303-
if (relativeDestination.getNameCount() <= 1) {
1304-
return;
1305-
}
1306-
// Throw away the first name as the archives have everything in a single top level folder we are not interested in
1307-
relativeDestination = relativeDestination.subpath(1, relativeDestination.getNameCount());
1308-
1309-
Path destination = destinationRoot.resolve(relativeDestination);
1310-
if (Files.isDirectory(source)) {
1311-
try {
1312-
Files.createDirectories(destination);
1313-
} catch (IOException e) {
1314-
throw new UncheckedIOException("Can't create directory " + destination.getParent(), e);
1303+
try {
1304+
Files.walkFileTree(sourceRoot, new SimpleFileVisitor<>() {
1305+
@Override
1306+
public FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs) throws IOException {
1307+
Path relativeDestination = sourceRoot.relativize(dir);
1308+
if (relativeDestination.getNameCount() <= 1) {
1309+
return FileVisitResult.CONTINUE;
13151310
}
1316-
} else {
1317-
try {
1318-
Files.createDirectories(destination.getParent());
1319-
} catch (IOException e) {
1320-
throw new UncheckedIOException("Can't create directory " + destination.getParent(), e);
1311+
// Throw away the first name as the archives have everything in a single top level folder we are not interested in
1312+
relativeDestination = relativeDestination.subpath(1, relativeDestination.getNameCount());
1313+
Path destination = destinationRoot.resolve(relativeDestination);
1314+
Files.createDirectories(destination);
1315+
return FileVisitResult.CONTINUE;
1316+
}
1317+
1318+
@Override
1319+
public FileVisitResult visitFile(Path source, BasicFileAttributes attrs) throws IOException {
1320+
Path relativeDestination = sourceRoot.relativize(source);
1321+
if (relativeDestination.getNameCount() <= 1) {
1322+
return FileVisitResult.CONTINUE;
13211323
}
1324+
// Throw away the first name as the archives have everything in a single top level folder we are not interested in
1325+
relativeDestination = relativeDestination.subpath(1, relativeDestination.getNameCount());
1326+
Path destination = destinationRoot.resolve(relativeDestination);
1327+
Files.createDirectories(destination.getParent());
13221328
syncMethod.accept(destination, source);
1329+
return FileVisitResult.CONTINUE;
13231330
}
1324-
});
1325-
} catch (UncheckedIOException e) {
1326-
if (e.getCause() instanceof NoSuchFileException cause) {
1327-
// Ignore these files that are sometimes left behind by the JVM
1328-
if (cause.getFile() == null || cause.getFile().contains(".attach_pid") == false) {
1329-
throw new UncheckedIOException(cause);
1331+
1332+
@Override
1333+
public FileVisitResult visitFileFailed(Path file, IOException exc) throws IOException {
1334+
if (exc instanceof NoSuchFileException noFileException) {
1335+
// Ignore these files that are sometimes left behind by the JVM
1336+
if (noFileException.getFile() != null && noFileException.getFile().contains(".attach_pid")) {
1337+
LOGGER.info("Ignoring file left behind by JVM: {}", noFileException.getFile());
1338+
return FileVisitResult.CONTINUE;
1339+
}
1340+
}
1341+
throw exc;
13301342
}
1331-
} else {
1332-
throw e;
1333-
}
1343+
});
13341344
} catch (IOException e) {
13351345
throw new UncheckedIOException("Can't walk source " + sourceRoot, e);
13361346
}

‎test/test-clusters/build.gradle

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,10 @@ dependencies {
99
implementation "com.fasterxml.jackson.core:jackson-annotations:${versions.jackson}"
1010
implementation "com.fasterxml.jackson.core:jackson-databind:${versions.jackson}"
1111
implementation "org.elasticsearch.gradle:reaper"
12+
13+
testImplementation "junit:junit:${versions.junit}"
14+
testImplementation "org.hamcrest:hamcrest:${versions.hamcrest}"
15+
testImplementation "org.apache.logging.log4j:log4j-core:${versions.log4j}"
1216
}
1317

1418
tasks.named("processResources").configure {

‎test/test-clusters/src/main/java/org/elasticsearch/test/cluster/util/IOUtils.java

Lines changed: 31 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,12 @@
1515
import java.io.File;
1616
import java.io.IOException;
1717
import java.io.UncheckedIOException;
18+
import java.nio.file.FileVisitResult;
1819
import java.nio.file.Files;
1920
import java.nio.file.NoSuchFileException;
2021
import java.nio.file.Path;
22+
import java.nio.file.SimpleFileVisitor;
23+
import java.nio.file.attribute.BasicFileAttributes;
2124
import java.util.Comparator;
2225
import java.util.function.BiConsumer;
2326
import java.util.stream.Stream;
@@ -118,35 +121,37 @@ public static void syncWithCopy(Path sourceRoot, Path destinationRoot) {
118121

119122
private static void sync(Path sourceRoot, Path destinationRoot, BiConsumer<Path, Path> syncMethod) {
120123
assert Files.exists(destinationRoot) == false;
121-
try (Stream<Path> stream = Files.walk(sourceRoot)) {
122-
stream.forEach(source -> {
123-
Path relativeDestination = sourceRoot.relativize(source);
124-
125-
Path destination = destinationRoot.resolve(relativeDestination);
126-
if (Files.isDirectory(source)) {
127-
try {
128-
Files.createDirectories(destination);
129-
} catch (IOException e) {
130-
throw new UncheckedIOException("Can't create directory " + destination.getParent(), e);
131-
}
132-
} else {
133-
try {
134-
Files.createDirectories(destination.getParent());
135-
} catch (IOException e) {
136-
throw new UncheckedIOException("Can't create directory " + destination.getParent(), e);
137-
}
124+
try {
125+
Files.walkFileTree(sourceRoot, new SimpleFileVisitor<>() {
126+
@Override
127+
public FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs) throws IOException {
128+
Path relativeDestination = sourceRoot.relativize(dir);
129+
Path destination = destinationRoot.resolve(relativeDestination);
130+
Files.createDirectories(destination);
131+
return FileVisitResult.CONTINUE;
132+
}
133+
134+
@Override
135+
public FileVisitResult visitFile(Path source, BasicFileAttributes attrs) throws IOException {
136+
Path relativeDestination = sourceRoot.relativize(source);
137+
Path destination = destinationRoot.resolve(relativeDestination);
138+
Files.createDirectories(destination.getParent());
138139
syncMethod.accept(destination, source);
140+
return FileVisitResult.CONTINUE;
139141
}
140-
});
141-
} catch (UncheckedIOException e) {
142-
if (e.getCause() instanceof NoSuchFileException cause) {
143-
// Ignore these files that are sometimes left behind by the JVM
144-
if (cause.getFile() == null || cause.getFile().contains(".attach_pid") == false) {
145-
throw new UncheckedIOException(cause);
142+
143+
@Override
144+
public FileVisitResult visitFileFailed(Path file, IOException exc) throws IOException {
145+
if (exc instanceof NoSuchFileException noFileException) {
146+
// Ignore these files that are sometimes left behind by the JVM
147+
if (noFileException.getFile() != null && noFileException.getFile().contains(".attach_pid")) {
148+
LOGGER.info("Ignoring file left behind by JVM: {}", noFileException.getFile());
149+
return FileVisitResult.CONTINUE;
150+
}
151+
}
152+
throw exc;
146153
}
147-
} else {
148-
throw e;
149-
}
154+
});
150155
} catch (IOException e) {
151156
throw new UncheckedIOException("Can't walk source " + sourceRoot, e);
152157
}

‎test/test-clusters/src/test/java/org/elasticsearch/test/cluster/util/IOUtilsTests.java

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the "Elastic License
4+
* 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
5+
* Public License v 1"; you may not use this file except in compliance with, at
6+
* your election, the "Elastic License 2.0", the "GNU Affero General Public
7+
* License v3.0 only", or the "Server Side Public License, v 1".
8+
*/
9+
10+
package org.elasticsearch.test.cluster.util;
11+
12+
import org.junit.Test;
13+
14+
import java.io.IOException;
15+
import java.io.UncheckedIOException;
16+
import java.nio.file.Files;
17+
import java.nio.file.Path;
18+
19+
import static org.hamcrest.CoreMatchers.containsString;
20+
import static org.hamcrest.MatcherAssert.assertThat;
21+
import static org.hamcrest.core.Is.is;
22+
import static org.hamcrest.core.Is.isA;
23+
import static org.junit.Assert.assertThrows;
24+
25+
public class IOUtilsTests {
26+
27+
@Test
28+
public void testSyncWithLinks() throws IOException {
29+
// given
30+
Path sourceDir = Files.createTempDirectory("sourceDir");
31+
Files.createFile(sourceDir.resolve("file1.txt"));
32+
Files.createFile(sourceDir.resolve("file2.txt"));
33+
Files.createDirectory(sourceDir.resolve("nestedDir"));
34+
Files.createFile(sourceDir.resolve("nestedDir").resolve("file3.txt"));
35+
36+
Path baseDestinationDir = Files.createTempDirectory("baseDestinationDir");
37+
Path destinationDir = baseDestinationDir.resolve("destinationDir");
38+
39+
// when
40+
IOUtils.syncWithLinks(sourceDir, destinationDir);
41+
42+
// then
43+
assertFileExists(destinationDir.resolve("file1.txt"));
44+
assertFileExists(destinationDir.resolve("file2.txt"));
45+
assertFileExists(destinationDir.resolve("nestedDir").resolve("file3.txt"));
46+
}
47+
48+
private void assertFileExists(Path path) throws IOException {
49+
assertThat("File " + path + " doesn't exist", Files.exists(path), is(true));
50+
assertThat("File " + path + " is not a regular file", Files.isRegularFile(path), is(true));
51+
assertThat("File " + path + " is not readable", Files.isReadable(path), is(true));
52+
if (OS.current() != OS.WINDOWS) {
53+
assertThat("Expected 2 hard links", Files.getAttribute(path, "unix:nlink"), is(2));
54+
}
55+
}
56+
57+
@Test
58+
public void testSyncWithLinksThrowExceptionWhenDestinationIsNotWritable() throws IOException {
59+
// given
60+
Path sourceDir = Files.createTempDirectory("sourceDir");
61+
Files.createFile(sourceDir.resolve("file1.txt"));
62+
63+
Path baseDestinationDir = Files.createTempDirectory("baseDestinationDir");
64+
Path destinationDir = baseDestinationDir.resolve("destinationDir");
65+
66+
baseDestinationDir.toFile().setWritable(false);
67+
68+
// when
69+
UncheckedIOException ex = assertThrows(UncheckedIOException.class, () -> IOUtils.syncWithLinks(sourceDir, destinationDir));
70+
71+
// then
72+
assertThat(ex.getCause(), isA(IOException.class));
73+
assertThat(ex.getCause().getMessage(), containsString("destinationDir"));
74+
}
75+
}

0 commit comments

Comments
 (0)