You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@curator.apache.org by "Ryan0751 (via GitHub)" <gi...@apache.org> on 2023/09/13 13:32:56 UTC
[GitHub] [curator] Ryan0751 opened a new pull request, #479: Curator 690. curator cache fails to load the cache if there are more than 64 k child z nodes
Ryan0751 opened a new pull request, #479:
URL: https://github.com/apache/curator/pull/479
This PR addresses the 64K child ZNode scaling limitation of CuratorCache.
This is done by utilizing tiered Phasers to handle monitoring of additional outstanding operations during the initialization phase of the cache.
As each child Phaser reaches the 0xffff capacity of a single Phaser, an additional child Phaser is created and registered with the root Phaser.
After initialization completes, the "currentChildPhaser" member variable is set to rootPhaser such that all child Phasers (which will never be used again) can be GC'd.
A unit test has been provided which fails without this fix, but succeeds with.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@curator.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [curator] Ryan0751 commented on a diff in pull request #479: Curator 690. curator cache fails to load the cache if there are more than 64 k child z nodes
Posted by "Ryan0751 (via GitHub)" <gi...@apache.org>.
Ryan0751 commented on code in PR #479:
URL: https://github.com/apache/curator/pull/479#discussion_r1324850074
##########
curator-recipes/src/main/java/org/apache/curator/framework/recipes/cache/CuratorCacheImpl.java:
##########
@@ -59,14 +59,29 @@ class CuratorCacheImpl implements CuratorCache, CuratorCacheBridge {
private final StandardListenerManager<CuratorCacheListener> listenerManager = StandardListenerManager.standard();
private final Consumer<Exception> exceptionHandler;
- private final Phaser outstandingOps = new Phaser() {
+ private final Phaser rootPhaser = new Phaser() {
@Override
protected boolean onAdvance(int phase, int registeredParties) {
callListeners(CuratorCacheListener::initialized);
+ synchronized (CuratorCacheImpl.this) {
+ currentChildPhaser = rootPhaser;
+ }
return true;
}
};
+ private Phaser currentChildPhaser = new Phaser(rootPhaser);
+
+ private synchronized Phaser getPhaserAndRegister() {
+ if (currentChildPhaser.getRegisteredParties() >= 0xffff) {
+ currentChildPhaser = new Phaser(rootPhaser);
Review Comment:
Is there a reason to prefer chain of child phasers versus a single layer deep tree off the single parent root phaser?
I suppose a tree has a limit of 64k children, but that should support 172x10^9 ZNodes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@curator.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [curator] tisonkun commented on a diff in pull request #479: Curator 690. curator cache fails to load the cache if there are more than 64 k child z nodes
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun commented on code in PR #479:
URL: https://github.com/apache/curator/pull/479#discussion_r1324654459
##########
curator-recipes/src/main/java/org/apache/curator/framework/recipes/cache/CuratorCacheImpl.java:
##########
@@ -59,14 +59,29 @@ class CuratorCacheImpl implements CuratorCache, CuratorCacheBridge {
private final StandardListenerManager<CuratorCacheListener> listenerManager = StandardListenerManager.standard();
private final Consumer<Exception> exceptionHandler;
- private final Phaser outstandingOps = new Phaser() {
+ private final Phaser rootPhaser = new Phaser() {
@Override
protected boolean onAdvance(int phase, int registeredParties) {
callListeners(CuratorCacheListener::initialized);
+ synchronized (CuratorCacheImpl.this) {
+ currentChildPhaser = rootPhaser;
+ }
return true;
}
};
+ private Phaser currentChildPhaser = new Phaser(rootPhaser);
+
+ private synchronized Phaser getPhaserAndRegister() {
+ if (currentChildPhaser.getRegisteredParties() >= 0xffff) {
+ currentChildPhaser = new Phaser(rootPhaser);
Review Comment:
```suggestion
currentChildPhaser = new Phaser(currentChildPhaser);
```
?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@curator.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [curator] tisonkun commented on pull request #479: Curator 690. curator cache fails to load the cache if there are more than 64 k child z nodes
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun commented on PR #479:
URL: https://github.com/apache/curator/pull/479#issuecomment-1719917023
Perhaps you don't allow maintainer edit. I make a patch based on yours at https://github.com/apache/curator/pull/480.
On merging your credit will retain.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@curator.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [curator] tisonkun commented on a diff in pull request #479: Curator 690. curator cache fails to load the cache if there are more than 64 k child z nodes
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun commented on code in PR #479:
URL: https://github.com/apache/curator/pull/479#discussion_r1326296925
##########
curator-recipes/src/main/java/org/apache/curator/framework/recipes/cache/CuratorCacheImpl.java:
##########
@@ -59,14 +59,29 @@ class CuratorCacheImpl implements CuratorCache, CuratorCacheBridge {
private final StandardListenerManager<CuratorCacheListener> listenerManager = StandardListenerManager.standard();
private final Consumer<Exception> exceptionHandler;
- private final Phaser outstandingOps = new Phaser() {
+ private final Phaser rootPhaser = new Phaser() {
@Override
protected boolean onAdvance(int phase, int registeredParties) {
callListeners(CuratorCacheListener::initialized);
+ synchronized (CuratorCacheImpl.this) {
+ currentChildPhaser = rootPhaser;
+ }
return true;
}
};
+ private Phaser currentChildPhaser = new Phaser(rootPhaser);
+
+ private synchronized Phaser getPhaserAndRegister() {
+ if (currentChildPhaser.getRegisteredParties() >= 0xffff) {
+ currentChildPhaser = new Phaser(rootPhaser);
Review Comment:
> prefer chain of child phasers versus a single layer deep tree off the single parent root phaser
You are right that the returned currentChildPhaser are referred in the method and called `arriveAndDeregister` accordingly. Then I don't have issue for this implementation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@curator.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [curator] tisonkun closed pull request #479: Curator 690. curator cache fails to load the cache if there are more than 64 k child z nodes
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun closed pull request #479: Curator 690. curator cache fails to load the cache if there are more than 64 k child z nodes
URL: https://github.com/apache/curator/pull/479
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@curator.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org