Repository navigation
Update Curator to 5.3.0 - #12939
Update Curator to 5.3.0#12939
Conversation
|
Tagged "Incompatible" since a feature (Exhibitor integration) is being removed. Given the state of Exhibitor — it seems to be mostly maintainerless for the last few years — I don't think this is a problem. But it should still be noted. |
|
One reason this upgrade is especially interesting is that it claims to fix https://issues.apache.org/jira/browse/CURATOR-638, a bug that people run into when ZK servers change IPs. This is relatively common in virtualized and containerized environments. |
|
I am not sure if https://issues.apache.org/jira/browse/CURATOR-538 is fixed in this version as it says it's targeting 5.4.0 which is still unreleased. There are a couple other bugs people might be hitting in containerized environments as well such as:
Though making this update will get us other fixes and should make it easier to keep up to date with the 5.x releases. |
|
Ah, nevermind! Well, it won't actually fix CURATOR-638 then. But, as you point out, it'll make it easier for us to get the fix when Curator 5.4 is out. Which is still good. |
|
Sorry, the failure is due to a recent Travis change that introduced a "silent" merge conflict. Due to that change, please rebase this PR on the latest master to pick up the newest Travis file. |
| // This close() call actually calls shutdownNow() on the executor registered with the Cache object, it | ||
| // better have its own executor or ignore shutdownNow() calls... | ||
| log.debug("Closing inventory cache for %s. Also removing listeners.", containerKey); | ||
| removed.getCache().getListenable().clear(); |
There was a problem hiding this comment.
i guess this wasn't really necessary to be there in previous version either, since it looks like the listeners got cleared there too https://github.com/apache/curator/blob/apache-curator-4.3.0/curator-recipes/src/main/java/org/apache/curator/framework/recipes/cache/PathChildrenCache.java#L378
| @Override | ||
| public CountDownLatch startImmediate() | ||
| { | ||
| return null; |
There was a problem hiding this comment.
nit: I looked around and don't think anything would be calling this, but maybe it would be safer to return a CountDownLatch with 0 as the count just in case anything does?
Follow up to #12939. As noted in that PR there are a few fixes in 5.4.0 that should make running on Kubernetes more reliable. Notably: - https://issues.apache.org/jira/browse/CURATOR-538 - https://issues.apache.org/jira/browse/CURATOR-649
* Update Curator to 5.3.0 * Update licenses.yaml * Fix inspections + add tests. * Fix checkstyle * Another intellij inspection fix * Update curator exclusions * Cleanup new exhibitor references * Remove unused dep and checkstyle fix
Follow up to apache#12939. As noted in that PR there are a few fixes in 5.4.0 that should make running on Kubernetes more reliable. Notably: - https://issues.apache.org/jira/browse/CURATOR-538 - https://issues.apache.org/jira/browse/CURATOR-649
5 participants
Description
Update Apache Curator to the latest version, 5.3.0. This version drops support for ZooKeeper 3.4 but Druid has already officially dropped support in 0.22. Further Curator has removed support for Exhibitor so all related configurations and tests have been removed in this PR as well. This should make is simple to keep up to date with bug fixes and patches released by the Curator team.
Key changed/added classes in this PR
server/src/main/java/org/apache/druid/curator/inventory/CuratorInventoryManager.javaRemoved call to
clear()as this is no longed exposed and is handled inside theclose()below.This PR has: