Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ResourceWatcher: Use native JDK7 APIs #11295

Closed
spinscale opened this issue May 22, 2015 · 7 comments
Closed

ResourceWatcher: Use native JDK7 APIs #11295

spinscale opened this issue May 22, 2015 · 7 comments
Labels
:Core/Infra/Core Core issues without another label >enhancement help wanted adoptme high hanging fruit Team:Core/Infra Meta label for core/infra team

Comments

@spinscale
Copy link
Contributor

The ResourceWatcherService supports watching files based on three different intervals. Since we require java 7 for Elasticsearch, we can use the WatchService for files, which - depending on the file system (osx and windows do not support this IIRC) - then is notified of immediate updates of files and does not require polling, but still falls back to polling in case this notification functionality is not available.

The JDK contains a SensitivityWatchEventModifier class, which could be used as default and we should be able to make this configurable as well, so we can still configure the different intervals.

@jpountz
Copy link
Contributor

jpountz commented May 22, 2015

+1

@colings86 colings86 added the :Core/Infra/Core Core issues without another label label Apr 24, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

@fabiodrg
Copy link

fabiodrg commented Nov 12, 2018

What is the current state for this issue? Should we start slowly using the WatchService? I would like to help, but there are many places where ResourceWatcherService is used, where to start.

@danielmitterdorfer
Copy link
Member

@fabiodrg666 thanks for your interest and your offer to help. The intention behind this change is that we keep ResourceWatcherService but only change its implementation so that it uses the JDK's WatchService internally, i.e. usages of the class ResourceWatcherService should not be replaced. If you want to work on this change, our contribution guidelines as well as our testing documentation should help you to get started.

@rjernst rjernst added the Team:Core/Infra Meta label for core/infra team label May 4, 2020
@rjernst rjernst added the needs:triage Requires assignment of a team area label label Dec 3, 2020
@jaymode jaymode added high hanging fruit and removed needs:triage Requires assignment of a team area label labels Dec 14, 2020
@jaymode
Copy link
Member

jaymode commented Dec 14, 2020

I've looked into this previously and one issue that prevented me from opening a PR is that the macOS implementation is based entirely on polling and does not have a native file change notification implementation in the JDK. I do not believe that this will be feasible to use unless we contribute a native macOS implementation to the JDK. JDK Issue: https://bugs.openjdk.java.net/browse/JDK-7133447

@rjernst
Copy link
Member

rjernst commented Dec 14, 2020

While this was an interesting idea, I remember that the jdk watcher also was not quite a direct replacement for the resource watcher in Elasticsearch. Given that, combined with the complications mentioned by @jaymode, I think after 5 years it is unlikely we will ever get around to trying this, and there is no clear incentive. Thus, I think we can close. WDYT @jaymode ?

@jaymode
Copy link
Member

jaymode commented Dec 15, 2020

I agree with closing. Let’s revisit if/when the JDK has a proper native implementation for our supported operating systems

@jaymode jaymode closed this as completed Dec 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Core/Infra/Core Core issues without another label >enhancement help wanted adoptme high hanging fruit Team:Core/Infra Meta label for core/infra team
Projects
None yet
Development

No branches or pull requests

9 participants