You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by GitBox <gi...@apache.org> on 2022/05/17 13:05:05 UTC
[GitHub] [accumulo] EdColeman opened a new pull request, #2714: Update documentation with purpose of the "cache"
EdColeman opened a new pull request, #2714:
URL: https://github.com/apache/accumulo/pull/2714
- Remove the TODO with an explanation that the class supports legacy
clients.
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] EdColeman merged pull request #2714: Update documentation with purpose of the "cache"
Posted by GitBox <gi...@apache.org>.
EdColeman merged PR #2714:
URL: https://github.com/apache/accumulo/pull/2714
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] ctubbsii commented on a diff in pull request #2714: Update documentation with purpose of the "cache"
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on code in PR #2714:
URL: https://github.com/apache/accumulo/pull/2714#discussion_r874861639
##########
core/src/main/java/org/apache/accumulo/fate/zookeeper/ZooCacheFactory.java:
##########
@@ -26,9 +26,14 @@
/**
* A factory for {@link ZooCache} instances.
+ * <p>
+ * Implementation note: We are using this "cache" to track all the instances that have been created,
Review Comment:
The word "cache" here is confusing since what it refers to is not an instance of any kind of cache, but we are dealing with ZooCache objects in this class. It would be better to refer to it directly,
```suggestion
* Implementation note: We are using the instances map to track all the instances that have been created,
```
Otherwise, these changes are good.
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] milleruntime commented on a diff in pull request #2714: Update documentation with purpose of the "cache"
Posted by GitBox <gi...@apache.org>.
milleruntime commented on code in PR #2714:
URL: https://github.com/apache/accumulo/pull/2714#discussion_r874865319
##########
core/src/main/java/org/apache/accumulo/fate/zookeeper/ZooCacheFactory.java:
##########
@@ -26,9 +26,14 @@
/**
* A factory for {@link ZooCache} instances.
+ * <p>
+ * Implementation note: We are using this "cache" to track all the instances that have been created,
+ * so we can explicitly close them when the last legacy client has gone away. This is part of the
+ * "SingletonManager" code, and it is likely that ZooCacheFactory and ZooKeeperInstance can be
+ * removed when legacy client code support is no longer required.
Review Comment:
```suggestion
* <p>
* A "cache" that tracks all ZooCache instances that have been created, to be explicitly closed.
* This class supports legacy client code as part of the SingletonManager. It is likely that
* ZooCacheFactory and ZooKeeperInstance can be removed when legacy client code support is no longer required.
```
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org