Skip to content

Commit 2a8c113

Browse files
jozalajozala
authored
[Tests] Fix copying files for test cluster (#124628) (#124661)
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. (cherry picked from commit 4ff1aad) # Conflicts: # build-tools/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchNode.java # test/test-clusters/build.gradle
1 parent dc3d475 commit 2a8c113

File tree

4 files changed

+155
-56
lines changed

4 files changed

+155
-56
lines changed

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

Lines changed: 40 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -60,11 +60,14 @@
6060
import java.io.LineNumberReader;
6161
import java.io.UncheckedIOException;
6262
import java.nio.charset.StandardCharsets;
63+
import java.nio.file.FileVisitResult;
6364
import java.nio.file.Files;
6465
import java.nio.file.NoSuchFileException;
6566
import java.nio.file.Path;
67+
import java.nio.file.SimpleFileVisitor;
6668
import java.nio.file.StandardCopyOption;
6769
import java.nio.file.StandardOpenOption;
70+
import java.nio.file.attribute.BasicFileAttributes;
6871
import java.time.Instant;
6972
import java.util.ArrayList;
7073
import java.util.Arrays;
@@ -1210,41 +1213,48 @@ private void syncWithCopy(Path sourceRoot, Path destinationRoot) {
12101213

12111214
private void sync(Path sourceRoot, Path destinationRoot, BiConsumer<Path, Path> syncMethod) {
12121215
assert Files.exists(destinationRoot) == false;
1213-
try (Stream<Path> stream = Files.walk(sourceRoot)) {
1214-
stream.forEach(source -> {
1215-
Path relativeDestination = sourceRoot.relativize(source);
1216-
if (relativeDestination.getNameCount() <= 1) {
1217-
return;
1218-
}
1219-
// Throw away the first name as the archives have everything in a single top level folder we are not interested in
1220-
relativeDestination = relativeDestination.subpath(1, relativeDestination.getNameCount());
1221-
1222-
Path destination = destinationRoot.resolve(relativeDestination);
1223-
if (Files.isDirectory(source)) {
1224-
try {
1225-
Files.createDirectories(destination);
1226-
} catch (IOException e) {
1227-
throw new UncheckedIOException("Can't create directory " + destination.getParent(), e);
1216+
try {
1217+
Files.walkFileTree(sourceRoot, new SimpleFileVisitor<Path>() {
1218+
@Override
1219+
public FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs) throws IOException {
1220+
Path relativeDestination = sourceRoot.relativize(dir);
1221+
if (relativeDestination.getNameCount() <= 1) {
1222+
return FileVisitResult.CONTINUE;
12281223
}
1229-
} else {
1230-
try {
1231-
Files.createDirectories(destination.getParent());
1232-
} catch (IOException e) {
1233-
throw new UncheckedIOException("Can't create directory " + destination.getParent(), e);
1224+
// Throw away the first name as the archives have everything in a single top level folder we are not interested in
1225+
relativeDestination = relativeDestination.subpath(1, relativeDestination.getNameCount());
1226+
Path destination = destinationRoot.resolve(relativeDestination);
1227+
Files.createDirectories(destination);
1228+
return FileVisitResult.CONTINUE;
1229+
}
1230+
1231+
@Override
1232+
public FileVisitResult visitFile(Path source, BasicFileAttributes attrs) throws IOException {
1233+
Path relativeDestination = sourceRoot.relativize(source);
1234+
if (relativeDestination.getNameCount() <= 1) {
1235+
return FileVisitResult.CONTINUE;
12341236
}
1237+
// Throw away the first name as the archives have everything in a single top level folder we are not interested in
1238+
relativeDestination = relativeDestination.subpath(1, relativeDestination.getNameCount());
1239+
Path destination = destinationRoot.resolve(relativeDestination);
1240+
Files.createDirectories(destination.getParent());
12351241
syncMethod.accept(destination, source);
1242+
return FileVisitResult.CONTINUE;
12361243
}
1237-
});
1238-
} catch (UncheckedIOException e) {
1239-
if (e.getCause() instanceof NoSuchFileException) {
1240-
NoSuchFileException cause = (NoSuchFileException) e.getCause();
1241-
// Ignore these files that are sometimes left behind by the JVM
1242-
if (cause.getFile() == null || cause.getFile().contains(".attach_pid") == false) {
1243-
throw new UncheckedIOException(cause);
1244+
1245+
@Override
1246+
public FileVisitResult visitFileFailed(Path file, IOException exc) throws IOException {
1247+
if (exc instanceof NoSuchFileException) {
1248+
NoSuchFileException noFileException = (NoSuchFileException) exc;
1249+
// Ignore these files that are sometimes left behind by the JVM
1250+
if (noFileException.getFile() != null && noFileException.getFile().contains(".attach_pid")) {
1251+
LOGGER.info("Ignoring file left behind by JVM: {}", noFileException.getFile());
1252+
return FileVisitResult.CONTINUE;
1253+
}
1254+
}
1255+
throw exc;
12441256
}
1245-
} else {
1246-
throw e;
1247-
}
1257+
});
12481258
} catch (IOException e) {
12491259
throw new UncheckedIOException("Can't walk source " + sourceRoot, e);
12501260
}

‎test/test-clusters/build.gradle‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,10 @@ dependencies {
88
shadow "org.apache.logging.log4j:log4j-api:${versions.log4j}"
99

1010
implementation files("$rootDir/build-tools/reaper/build/libs/reaper.jar")
11+
12+
testImplementation "junit:junit:${versions.junit}"
13+
testImplementation "org.hamcrest:hamcrest:${versions.hamcrest}"
14+
testImplementation "org.apache.logging.log4j:log4j-core:${versions.log4j}"
1115
}
1216

1317
// We use updated APIs here and since this project is only used for REST testing it's ok to run with > Java 8

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

Lines changed: 36 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -8,17 +8,25 @@
88

99
package org.elasticsearch.test.cluster.util;
1010

11+
import org.apache.logging.log4j.LogManager;
12+
import org.apache.logging.log4j.Logger;
13+
1114
import java.io.File;
1215
import java.io.IOException;
1316
import java.io.UncheckedIOException;
17+
import java.nio.file.FileVisitResult;
1418
import java.nio.file.Files;
1519
import java.nio.file.NoSuchFileException;
1620
import java.nio.file.Path;
21+
import java.nio.file.SimpleFileVisitor;
22+
import java.nio.file.attribute.BasicFileAttributes;
1723
import java.util.Comparator;
1824
import java.util.function.BiConsumer;
1925
import java.util.stream.Stream;
2026

2127
public final class IOUtils {
28+
private static final Logger LOGGER = LogManager.getLogger(IOUtils.class);
29+
2230
private static final int RETRY_DELETE_MILLIS = OS.current() == OS.WINDOWS ? 500 : 0;
2331
private static final int MAX_RETRY_DELETE_TIMES = OS.current() == OS.WINDOWS ? 15 : 0;
2432

@@ -89,35 +97,37 @@ public static void syncWithCopy(Path sourceRoot, Path destinationRoot) {
8997

9098
private static void sync(Path sourceRoot, Path destinationRoot, BiConsumer<Path, Path> syncMethod) {
9199
assert Files.exists(destinationRoot) == false;
92-
try (Stream<Path> stream = Files.walk(sourceRoot)) {
93-
stream.forEach(source -> {
94-
Path relativeDestination = sourceRoot.relativize(source);
95-
96-
Path destination = destinationRoot.resolve(relativeDestination);
97-
if (Files.isDirectory(source)) {
98-
try {
99-
Files.createDirectories(destination);
100-
} catch (IOException e) {
101-
throw new UncheckedIOException("Can't create directory " + destination.getParent(), e);
102-
}
103-
} else {
104-
try {
105-
Files.createDirectories(destination.getParent());
106-
} catch (IOException e) {
107-
throw new UncheckedIOException("Can't create directory " + destination.getParent(), e);
108-
}
100+
try {
101+
Files.walkFileTree(sourceRoot, new SimpleFileVisitor<Path>() {
102+
@Override
103+
public FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs) throws IOException {
104+
Path relativeDestination = sourceRoot.relativize(dir);
105+
Path destination = destinationRoot.resolve(relativeDestination);
106+
Files.createDirectories(destination);
107+
return FileVisitResult.CONTINUE;
108+
}
109+
110+
@Override
111+
public FileVisitResult visitFile(Path source, BasicFileAttributes attrs) throws IOException {
112+
Path relativeDestination = sourceRoot.relativize(source);
113+
Path destination = destinationRoot.resolve(relativeDestination);
114+
Files.createDirectories(destination.getParent());
109115
syncMethod.accept(destination, source);
116+
return FileVisitResult.CONTINUE;
110117
}
111-
});
112-
} catch (UncheckedIOException e) {
113-
if (e.getCause() instanceof NoSuchFileException cause) {
114-
// Ignore these files that are sometimes left behind by the JVM
115-
if (cause.getFile() == null || cause.getFile().contains(".attach_pid") == false) {
116-
throw new UncheckedIOException(cause);
118+
119+
@Override
120+
public FileVisitResult visitFileFailed(Path file, IOException exc) throws IOException {
121+
if (exc instanceof NoSuchFileException noFileException) {
122+
// Ignore these files that are sometimes left behind by the JVM
123+
if (noFileException.getFile() != null && noFileException.getFile().contains(".attach_pid")) {
124+
LOGGER.info("Ignoring file left behind by JVM: {}", noFileException.getFile());
125+
return FileVisitResult.CONTINUE;
126+
}
127+
}
128+
throw exc;
117129
}
118-
} else {
119-
throw e;
120-
}
130+
});
121131
} catch (IOException e) {
122132
throw new UncheckedIOException("Can't walk source " + sourceRoot, e);
123133
}

‎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)