[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_r386560053
 
 

 ##########
 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:
   Looking into this a little bit more, it seems this utility is only ever used by the X509Util class to monitor changes to keyStore and trustStore files on disk, presumably for some sort of automatic reconfiguration.
   
   The modifier seems to be an attempt to work around the slow polling implementation of the watch service on Mac OS X (https://bugs.openjdk.java.net/browse/JDK-7133447), which doesn't have a native implementation on that platform.
   
   I can't imagine those files changing frequently enough for this to matter. But, even if they did change, I can't imagine that the optimization to automatically reconfigure a few seconds sooner matters either.
   
   However, if automatic TLS reconfiguration was a critical need for somebody... there are lots of alternative watch service implementations out there to work around this issue. One is https://github.com/takari/directory-watcher ; I'm not advocating for using any of these... I don't know enough about them, but if it was important for somebody, they could consider contributing a change that makes use of one of those, instead of relying on the com.sun modifier to the built-in watch service that this PR removes.

----------------------------------------------------------------
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