RFR: 8297275: WatchService delay with PollingWatchService causes test failures

[Tyler Steele]

On Tue, 22 Nov 2022 19:28:32 GMT, Alan Bateman <alanb at openjdk.org> wrote:

> I think the issue is that the test registers a directory and immediately modifies a file in that directory. To an observer using the PollingWatchService, the file last modification time has not changed so the watch key is not queued... What is the precision of the time of last modification on AIX?

A reasonable explanation, but I disagree with the conclusion. The implementation of getLastModifiedTime comes from UnixFileSystem code which is similar between the Unixes [2]. To your question, AIX uses nanosecond information to compute the mtime as well. In addition, PollingWatchService explicitly converts the modification time to milliseconds [1].

> In passing, I would like to see POLLING_INIT_DELAY (added by JDK-8285956) be removed as it's inconsistent to start polling after 1s when the polling rate is 2s. The original code was correct.

Does this mean that you'd prefer the polling rate match always match the initial delay? I can see an argument for this. But in that case, I would advocate for the addition of a delay like the one proposed by this PR. Once the call to register a path returns, I think it's reasonable to assume that any change to the registered Watchable would be noticed by the WatchService. If the activation window that I describe is present, any changes made during that time can not be noticed because the polling thread is not running yet [3].

---

It may be worth clarifying that my changes were not intended just to make the test pass. It's  my belief that there is a real issue with the presence of this activation window, which PollingWatchService would benefit from removing. I am happy to discuss the best method for doing so.


Result of changing InitDelay and Interval. Changes are not reported until p_0.
scheduleAtFixedRate(thunk, InitDelay, Interval TimeUnit.SECONDS);

# Current
| InitDelay | Interval | ... | Interval |
           p_0        p_1   p_n       p_n+1
|<--------->| activation window

# Pre-8285956
| Interval | Interval | ... | Interval |
          p_0        p_1   p_n       p_n+1
|<-------->| activation window

# Proposed Change
| InitDelay | ... | Interval |
p_0        p_1   p_n       p_n+1

Legend:
InitDelay: is the POLLING_INIT_DELAY
Interval: is the DEFAULT_POLLING_INTERVAL
p_0: is the first poll run by the polling thread
p_n: is the (n+1)th poll by the polling thread


[1] https://github.com/openjdk/jdk/blob/2afb4c3327b6830a009ee1ab8a1eb7803ef53007/src/java.base/share/classes/sun/nio/fs/PollingWatchService.java#L367
[2] https://github.com/openjdk/jdk/blob/2afb4c3327b6830a009ee1ab8a1eb7803ef53007/src/java.base/unix/native/libjava/UnixFileSystem_md.c#L226-L247
[3] https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/util/concurrent/ScheduledExecutorService.html#scheduleAtFixedRate(java.lang.Runnable,long,long,java.util.concurrent.TimeUnit)

-------------

PR: https://git.openjdk.org/jdk/pull/10824