diff --git a/docs/changelog/128653.yaml b/docs/changelog/128653.yaml new file mode 100644 index 0000000000000..6ecf29a22fbc2 --- /dev/null +++ b/docs/changelog/128653.yaml @@ -0,0 +1,5 @@ +pr: 128653 +summary: Add retry for `AccessDeniedException` in `AbstractFileWatchingService` +area: Infra/Settings +type: bug +issues: [] diff --git a/muted-tests.yml b/muted-tests.yml index eaf8d03a4434d..50719d926ab72 100644 --- a/muted-tests.yml +++ b/muted-tests.yml @@ -486,9 +486,6 @@ tests: - class: org.elasticsearch.packaging.test.DockerTests method: test124CanRestartContainerWithStackLoggingConfig issue: https://github.com/elastic/elasticsearch/issues/128121 -- class: org.elasticsearch.reservedstate.service.FileSettingsServiceIT - method: testSymlinkUpdateTriggerReload - issue: https://github.com/elastic/elasticsearch/issues/128619 - class: org.elasticsearch.packaging.test.DockerTests method: test085EnvironmentVariablesAreRespectedUnderDockerExec issue: https://github.com/elastic/elasticsearch/issues/128115 diff --git a/server/src/main/java/org/elasticsearch/common/file/AbstractFileWatchingService.java b/server/src/main/java/org/elasticsearch/common/file/AbstractFileWatchingService.java index 243126964529b..20362780e1ebe 100644 --- a/server/src/main/java/org/elasticsearch/common/file/AbstractFileWatchingService.java +++ b/server/src/main/java/org/elasticsearch/common/file/AbstractFileWatchingService.java @@ -17,6 +17,7 @@ import java.io.IOException; import java.io.InputStream; +import java.nio.file.AccessDeniedException; import java.nio.file.ClosedWatchServiceException; import java.nio.file.NoSuchFileException; import java.nio.file.Path; @@ -56,6 +57,7 @@ public abstract class AbstractFileWatchingService extends AbstractLifecycleCompo private static final Logger logger = LogManager.getLogger(AbstractFileWatchingService.class); private static final int REGISTER_RETRY_COUNT = 5; + private static final int ACCESS_DENIED_RETRY_COUNT = 5; private final Path settingsDir; private final Map fileUpdateState = new HashMap<>(); private WatchService watchService; // null; @@ -115,20 +117,33 @@ public final boolean watching() { return watcherThread != null; } - private FileUpdateState readFileUpdateState(Path path) throws IOException { - try { - BasicFileAttributes attr = filesReadAttributes(path, BasicFileAttributes.class); - return new FileUpdateState(attr.lastModifiedTime().toMillis(), path.toRealPath().toString(), attr.fileKey()); - } catch (NoSuchFileException e) { - // file doesn't exist anymore - return null; - } + // package private for testing + FileUpdateState readFileUpdateState(Path path) throws IOException, InterruptedException { + int retryCount = 0; + do { + try { + BasicFileAttributes attr = filesReadAttributes(path, BasicFileAttributes.class); + return new FileUpdateState(attr.lastModifiedTime().toMillis(), path.toRealPath().toString(), attr.fileKey()); + } catch (NoSuchFileException e) { + // file doesn't exist anymore + return null; + } catch (AccessDeniedException e) { + // This can happen on Windows when a symlink is deleted for a path while path.toRealPath() is called. In most cases the + // symlink is recreated, so retry + if (retryCount == ACCESS_DENIED_RETRY_COUNT - 1) { + throw e; + } + logger.debug("Could not read file state [{}] attempt [{}]", path, retryCount); + Thread.sleep(retryDelayMillis(retryCount)); + retryCount++; + } + } while (true); } // platform independent way to tell if a file changed // we compare the file modified timestamp, the absolute path (symlinks), and file id on the system @FixForMultiProject // what do we do when a file is removed? - final boolean fileChanged(Path path) throws IOException { + final boolean fileChanged(Path path) throws IOException, InterruptedException { FileUpdateState newFileState = readFileUpdateState(path); if (newFileState == null) { fileUpdateState.remove(path); diff --git a/server/src/test/java/org/elasticsearch/common/file/AbstractFileWatchingServiceTests.java b/server/src/test/java/org/elasticsearch/common/file/AbstractFileWatchingServiceTests.java index 5a2c06dd56eb6..052e3fead9844 100644 --- a/server/src/test/java/org/elasticsearch/common/file/AbstractFileWatchingServiceTests.java +++ b/server/src/test/java/org/elasticsearch/common/file/AbstractFileWatchingServiceTests.java @@ -25,6 +25,7 @@ import java.io.IOException; import java.io.InputStream; +import java.nio.file.AccessDeniedException; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.StandardCopyOption; @@ -238,6 +239,23 @@ public void testRegisterWatchKeyRetry() throws IOException, InterruptedException verify(service, times(2)).retryDelayMillis(anyInt()); } + public void testAccessDeniedRetry() throws IOException, InterruptedException { + var service = spy(fileWatchingService); + doAnswer(i -> 0L).when(service).retryDelayMillis(anyInt()); + + Files.createDirectories(service.watchedFileDir()); + var mockedPath = spy(service.watchedFileDir()); + + doThrow(new AccessDeniedException("can't read state")).doThrow(new AccessDeniedException("can't read state - attempt 2")) + .doAnswer(i -> mockedPath) + .when(mockedPath) + .toRealPath(); + + var result = service.readFileUpdateState(mockedPath); + assertNotNull(result); + verify(service, times(2)).retryDelayMillis(anyInt()); + } + // helpers private static void writeTestFile(Path path, String contents) throws IOException { Path tempFilePath = createTempFile();