(celeborn) branch main updated: [CELEBORN-2496][TEST] Fix mtime granularity race in ReloadingX509TrustManagerSuiteJ.testReloadSymlink

[zaynt]

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

zaynt4606 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 e39aff5a40 [CELEBORN-2496][TEST] Fix mtime granularity race in 
ReloadingX509TrustManagerSuiteJ.testReloadSymlink
e39aff5a40 is described below

commit e39aff5a402eefd6eeb17d376a66a272355ff3b4
Author: yew1eb <yew...@gmail.com>
AuthorDate: Thu Oct 22 13:50:55 2026 +0800

    [CELEBORN-2496][TEST] Fix mtime granularity race in 
ReloadingX509TrustManagerSuiteJ.testReloadSymlink
    
    ### What changes were proposed in this pull request?
    
    Backdate `trustStore2` by 5 seconds immediately after creating it in 
`ReloadingX509TrustManagerSuiteJ.testReloadSymlink`, before the trust manager 
starts and before the symlink points to that file:
    
    ```java
    createTrustStore(trustStore2, "password", certs);
    trustStore2.setLastModified(trustStore2.lastModified() - 5000);
    ```
    
    The symlink target change triggers the first reload and records the 
backdated modification time. Adding the third certificate then rewrites the 
trust store with a distinguishable modification time, even with second-level 
timestamp granularity.
    
    Setting the timestamp before the first load avoids an additional 
modification event after the rewrite. The exact reload-count checks and the 
final assertion that all three certificates are loaded remain in place.
    
    ### Why are the changes needed?
    
    The test rewrites `trustStore2` shortly after its first reload. 
`ReloadingX509TrustManager.needsReload()` compares file modification times 
using `Files.getLastModifiedTime().toMillis()`. In the reported JDK 8/macOS 
environment, timestamps are truncated to whole seconds, so the rewrite can 
retain the recorded timestamp and remain undetected until the test times out:
    
    ```text
    java.lang.IllegalStateException: Trust store not reloaded after 100 
attempts!
    ```
    
    Backdating the file before loading it makes the later rewrite detectable 
without adding a sleep or a second timestamp update after the rewrite.
    
    JIRA: https://issues.apache.org/jira/browse/CELEBORN-2496
    
    ### Does this PR resolve a correctness bug?
    
    - [ ] Yes
    
    ### Does this PR introduce _any_ user-facing change?
    
    - [ ] Yes
    
    No, this is a test-only change.
    
    ### How was this patch tested?
    
    Validation of commit `76d737ea60eb6b18ce27c656f5b0bc00e436c30b`:
    
    - The author reported five consecutive runs of 
`ReloadingX509TrustManagerSuiteJ`, with all six tests passing in each run, plus 
Spotless.
    - Local review validation on Linux/JDK 17: all six tests passed, with no 
failures, errors, or skipped tests:
    
      ```shell
      build/mvn -B --no-transfer-progress -pl common -am test \
        -Dtest=ReloadingX509TrustManagerSuiteJ \
        -Dsurefire.failIfNoSpecifiedTests=false -DfailIfNoTests=false \
        -DwildcardSuites=none -DtestJarPhase=process-test-classes
      ```
    
    - The [Maven service CI job on JDK 
8](https://github.com/apache/celeborn/actions/runs/39716382757/job/119619384216)
 passed, including all six tests in this suite.
    
    Closes #3888 from yew1eb/CELEBORN-2496.
    
    Authored-by: yew1eb <yew...@gmail.com>
    Signed-off-by: zhengtao <shuaizhentao....@alibaba-inc.com>
    
    Co-Authored-By: Codex <nore...@openai.com>
    AI-Model: gpt-6
    AI-Contributed/Feature: 0/0
    AI-Contributed/UT: 0/6
---
 .../common/network/ssl/ReloadingX509TrustManagerSuiteJ.java         | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git 
a/common/src/test/java/org/apache/celeborn/common/network/ssl/ReloadingX509TrustManagerSuiteJ.java
 
b/common/src/test/java/org/apache/celeborn/common/network/ssl/ReloadingX509TrustManagerSuiteJ.java
index 98762a5b3a..04ca6ca9e3 100644
--- 
a/common/src/test/java/org/apache/celeborn/common/network/ssl/ReloadingX509TrustManagerSuiteJ.java
+++ 
b/common/src/test/java/org/apache/celeborn/common/network/ssl/ReloadingX509TrustManagerSuiteJ.java
@@ -256,6 +256,12 @@ public class ReloadingX509TrustManagerSuiteJ {
     certs.put("cert1", cert1);
     certs.put("cert2", cert2);
     createTrustStore(trustStore2, "password", certs);
+    // The reload check compares modification times, so a rewrite landing 
within the same
+    // timestamp tick as the last load is indistinguishable from no change 
(e.g. JDK 8 on
+    // macOS truncates mtime to whole seconds). Backdate trustStore2 before it 
is first
+    // loaded, so the cert3 rewrite below always carries a newer timestamp and 
no post-write
+    // mtime nudge (which could trigger an extra, racy reload) is needed.
+    trustStore2.setLastModified(trustStore2.lastModified() - 5000);
 
     File trustStoreSymlink = File.createTempFile("testreloadsymlink", "jks");
     trustStoreSymlink.delete();