(celeborn) branch main updated: [CELEBORN-1406] Use Files. getLastModifiedTime to find last modified time instead of file.lastModified

[mridulm80]

This is an automated email from the ASF dual-hosted git repository.

mridulm80 pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/celeborn.git


The following commit(s) were added to refs/heads/main by this push:
     new 5b8773c39 [CELEBORN-1406] Use Files. getLastModifiedTime to find last 
modified time instead of file.lastModified
5b8773c39 is described below

commit 5b8773c39fe70587ea055822a474539ca033262c
Author: Mridul Muralidharan <mridulatgmail.com>
AuthorDate: Mon May 6 23:15:02 2024 -0500

    [CELEBORN-1406] Use Files. getLastModifiedTime to find last modified time 
instead of file.lastModified
    
    ### What changes were proposed in this pull request?
    
    Use `Files.readAttributes` instead of `File.lastModified`.
    `File.lastModified` has been observed to have issues in some of our build 
boxes - which got addressed when moving to `Files.readAttributes`
    
    ### Why are the changes needed?
    
    Make reloading more reliable and fix flakey tests
    
    ### Does this PR introduce _any_ user-facing change?
    No
    
    ### How was this patch tested?
    Existing unit tests.
    
    Closes #2485 from mridulm/CELEBORN-1406.
    
    Authored-by: Mridul Muralidharan <mridulatgmail.com>
    Signed-off-by: Mridul Muralidharan <mridul<at>gmail.com>
---
 .../common/network/ssl/ReloadingX509TrustManager.java     | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git 
a/common/src/main/java/org/apache/celeborn/common/network/ssl/ReloadingX509TrustManager.java
 
b/common/src/main/java/org/apache/celeborn/common/network/ssl/ReloadingX509TrustManager.java
index 82f69f4f7..950b5408d 100644
--- 
a/common/src/main/java/org/apache/celeborn/common/network/ssl/ReloadingX509TrustManager.java
+++ 
b/common/src/main/java/org/apache/celeborn/common/network/ssl/ReloadingX509TrustManager.java
@@ -20,6 +20,7 @@ package org.apache.celeborn.common.network.ssl;
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.IOException;
+import java.nio.file.Files;
 import java.security.GeneralSecurityException;
 import java.security.KeyStore;
 import java.security.cert.CertificateException;
@@ -156,7 +157,7 @@ public class ReloadingX509TrustManager implements 
X509TrustManager, Runnable {
       // `file` can be a symbolic link. We need to reload if it points to 
another file,
       // or if the file has been modified
       if (latestCanonicalFile.getPath().equals(canonicalPath)
-          && latestCanonicalFile.lastModified() == lastLoaded) {
+          && getFileLastModified(latestCanonicalFile) == lastLoaded) {
         reload = false;
       }
     } else {
@@ -170,7 +171,7 @@ public class ReloadingX509TrustManager implements 
X509TrustManager, Runnable {
     KeyStore ks = KeyStore.getInstance(type);
     File latestCanonicalFile = file.getCanonicalFile();
     canonicalPath = latestCanonicalFile.getPath();
-    lastLoaded = latestCanonicalFile.lastModified();
+    lastLoaded = getFileLastModified(latestCanonicalFile);
     try (FileInputStream in = new FileInputStream(latestCanonicalFile)) {
       char[] passwordCharacters = password != null ? password.toCharArray() : 
null;
       ks.load(in, passwordCharacters);
@@ -215,4 +216,14 @@ public class ReloadingX509TrustManager implements 
X509TrustManager, Runnable {
       needsReloadCheckCounts++;
     }
   }
+
+  private static long getFileLastModified(File file) {
+    try {
+      return Files.getLastModifiedTime(file.toPath()).toMillis();
+    } catch (IOException ioEx) {
+      // fallback
+      logger.info("Unable to read attributes for {}", file, ioEx);
+      return file.lastModified();
+    }
+  }
 }