[CURATOR-477] added support for turning off zk watches#278
[CURATOR-477] added support for turning off zk watches#278asfgit merged 7 commits intoapache:masterfrom
Conversation
| client.getData().decompressed().usingWatcher(this).inBackground(this).forPath(path); | ||
| } else { | ||
| client.getData().decompressed().inBackground(this).forPath(path); | ||
| } |
There was a problem hiding this comment.
feel like this could all be a little cleaner with a maybeWatch() helper in the class, e.g.
maybeWatch(client.getData().decompressed()).inBackground(this).forPath(path)
and so one throughout. You could also put the inBackground(this) into the helper too
| private ExecutorService executorService = null; | ||
| private int maxDepth = Integer.MAX_VALUE; | ||
| private boolean createParentNodes = false; | ||
| private boolean createZkWatches = true; |
There was a problem hiding this comment.
I'd probably reverse this as "disableZkWatch" since that's the unusual case.
| { | ||
| this.createZkWatches = createZkWatches; | ||
| return this; | ||
| } |
There was a problem hiding this comment.
and e.g. disableZkWatches(boolean)
|
@dragonsinth Thanks for your review. addressed the feedback. PTAL |
| return dataBuilder.usingWatcher(this).inBackground(this); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
private void doRefreshChildren() throws Exception
{
if ( treeState.get() == TreeState.STARTED )
{
maybeWatch(client.getChildren()).forPath(path);
}
}
private void doRefreshData() throws Exception
{
if ( treeState.get() == TreeState.STARTED )
{
if ( dataIsCompressed )
{
maybeWatch(client.getData().decompressed()).forPath(path);
}
else
{
maybeWatch(client.getData()).forPath(path);
}
}
}
private <T, P extends Watchable<BackgroundPathable<T>> & BackgroundPathable<T>> Pathable<T> maybeWatch(P dataBuilder)
{
if ( disableZkWatches )
{
return dataBuilder.inBackground(this);
}
else
{
return dataBuilder.usingWatcher(this).inBackground(this);
}
}
|
TestTreeCache is getting stuck on your branch. Haven't figured out why yet. Wanna take a look? |
|
Sorry. My bad. Problem with my change. Fixed. PTAL |
|
aha that would do it! I do think you should update the |
|
Ahh. Sorry. I missed that comment. I have changed it now. PTAL. |
|
@Randgalt LGTM, although I haven't rerun tests yet. |
|
@dragonsinth sorry. Added one more commit for exists check also. can you PTAL? |
|
@Randgalt just following-up on this - Any thing else pending on this? |
|
I will merge this shortly, thanks @ramaraochavali and @dragonsinth |
4 participants
In our use case, we use
TreeCacheto get Zk Data periodically. We startTreeCacheread data and close it. In this use case, TheZkWatchManagerofZooKeeperclass keeps growing for every TreeCache operation because newTreeNodeobjects are created and added there leading to a memory leak. Also since we do not want the Watcher to periodically watch, this creates unnecessary background operations.In this PR, Made the createZkWatches configurable in the Builder, so that use cases like ours can set it to false. The default is true, retaining the current behaviour.
Fixes the jira issue https://issues.apache.org/jira/browse/CURATOR-477