[GitHub] [zookeeper] ctubbsii commented on a change in pull request #1269: ZOOKEEPER-3739: Remove unsupported com.sun API

[GitBox <gi...@apache.org>]

ctubbsii commented on a change in pull request #1269: ZOOKEEPER-3739: Remove unsupported com.sun API
URL: https://github.com/apache/zookeeper/pull/1269#discussion_r384163550
 
 

 ##########
 File path: zookeeper-server/src/main/java/org/apache/zookeeper/common/FileChangeWatcher.java
 ##########
 @@ -72,7 +71,7 @@ public FileChangeWatcher(Path dirPath, Consumer<WatchEvent<?>> callback) throws
 
         LOG.debug("Registering with watch service: {}", dirPath);
 
-        dirPath.register(watchService, new WatchEvent.Kind<?>[]{StandardWatchEventKinds.ENTRY_CREATE, StandardWatchEventKinds.ENTRY_DELETE, StandardWatchEventKinds.ENTRY_MODIFY, StandardWatchEventKinds.OVERFLOW}, SensitivityWatchEventModifier.HIGH);
 
 Review comment:
   > Can't we detect the presence of that option at runtime and use it with reflection?
   
   Code to do that would look similar to what is described in:
   https://github.com/HotswapProjects/HotswapAgent/issues/41#issue-39256602
   
   However, because of module access restrictions in future JVMs, there's a strong possibility that doing this would result in runtime exceptions in newer JVMs, unless users added the `jdk.unsupported` module to their module path at runtime... and even then, these unsupported APIs are supposed to be phased out entirely at some point... so there's a good chance this will add complexity which will never matter, because the complex code path will never be traversed (or will be traversed and cause an access restriction error at runtime).
   
   > What's the impact of this patch?
   
   That's the question I had, too (and the related: is it even worth it to try to keep the modifier?).
   
   From what I could tell, the behavior this modifies is not well defined anyway. On some systems, the modifier *could* make file changes observed more quickly... but there's no guarantee of that... and there's no guarantee of seeing *every* change event on the file being watched, no matter what modifier is on it. Even with the modifier, it's probably still going to be on the order of a few seconds to notice changes. In my opinion, it's probably not worth it, but I'm not an expert on NIO stuff by far.
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services