curator-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From dtrott <>
Subject [GitHub] curator pull request: Curator 94
Date Tue, 25 Aug 2015 08:20:12 GMT
Github user dtrott commented on the pull request:
    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:
    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 PERMANENT the 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 ....

If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at or file a JIRA ticket
with INFRA.

View raw message