apache
Curator 94
https://issues.apache.org/jira/browse/CURATOR-94
Added some docs (based on my understanding) for ServiceType
Updated ServiceDiscoveryImpl.registerService so that PERMANENT registrations are not stored within the service.
This is to prevent two side effects:
When the ServiceDiscoveryImpl class is closed all PERMANENT registrations are deleted. This is not desireable since callers who create PERMANENT registations should expect them to survive until explicitly deleted.
PERMANENT registrations should be stateless with respect to a particular service discovery client (assume a cluster of rest services). It should be possible to use any of them to register/unregister. It is not desirable to keep re-registering them whenever the client losses/reconnects to Zookeeper.
First of all thank you for this PR, I was looking into some other issues with permanent service types and this would certainly have been a big problem as I did expect permanent service types to last beyond the lifetime of Curator's service discovery.
A slightly related question (I hope), looking at your diff you don't seem to be doing anything for the unregistration of these services. Looking at https://github.com/apache/curator/blob/master/curator-x-discovery/src/main/java/org/apache/curator/x/discovery/details/ServiceDiscoveryImpl.java#L241 as a result of your change of not putting them in the services map wouldn't that mean they cannot be unregistered at all? Also the current implementation of unregistration means that services cannot be unregistered using a different servicediscovery instance than they were registered with, which would be very useful with permanent service types.
Note: As a user, I think not being able to update permanent servicetypes could easily be worked around by unregistering and reregistering them with the new information.
I am afraid that this patch is more than a year old and the underlying code has changed.
I believe the original code with the patch applied looked like this:
https://github.com/dtrott/curator/blob/CURATOR-94/curator-x-discovery/src/main/java/org/apache/curator/x/discovery/details/ServiceDiscoveryImpl.java
Hence your suggestion of unregistered and re-registering would have worked, however as you have pointed out, this approach will not work any more as unregistration now requires that the service be in the map.
I have not looked at this issue since the patch was submitted (it worked for me ;-) however a very cursory scan of the current code base didn't indicate the the core issue had been fixed (PERMANENT services are not permanent).
I don't really like my original suggestion of loading PERMANENT registrations into all clients, as it would require a lot more overhead to monitor (watch) and load registrations in other clients, a simpler solution would be to add special cases to the update and unregister methods so that they load the PERMANENT record from zookeper instead of using the map, however I suspect that this might introduce some nasty race conditions - frankly I would be fine with this and resolve the issue with documentation ... "PERMANENT registrations require external synchronization", however I can see why others may feel that isn't clean.
I would also recommend confirming if the bug still exists and if so, removing PERMANENT until such time as someone creates a workable patch... otherwise the code base is just leaving a landmine for someone else to step on ....
I'm having the same problem. Tried with the old 2.10.0 and the latest 4.0.0, but I'm still losing the PERMANENT service registration when I call ServiceDiscovery.close() method.
Are there any news?
Thanks
Curator is now at Apache. Please open an issue on Curator's issue tracker: http://issues.apache.org/jira/browse/CURATOR
Closed as stale. There're many changes since this patch was made in the first place. Feel free to resubmit it if it's still relevant.
