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/27 14:21:14 UTC

[GitHub] [accumulo] EdColeman opened a new pull request, #2741: Add method to clear local store cache, update client service handler.

EdColeman opened a new pull request, #2741:
URL: https://github.com/apache/accumulo/pull/2741

     - Alternate impl of PR #2740


-- 
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 closed pull request #2741: Add method to clear local store cache, update client service handler.

Posted by GitBox <gi...@apache.org>.
EdColeman closed pull request #2741: Add method to clear local store cache, update client service handler.
URL: https://github.com/apache/accumulo/pull/2741


-- 
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 commented on pull request #2741: Add method to clear local store cache, update client service handler.

Posted by GitBox <gi...@apache.org>.
EdColeman commented on PR #2741:
URL: https://github.com/apache/accumulo/pull/2741#issuecomment-1142348674

   closing in favor of #2740


-- 
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 #2741: Add method to clear local store cache, update client service handler.

Posted by GitBox <gi...@apache.org>.
ctubbsii commented on code in PR #2741:
URL: https://github.com/apache/accumulo/pull/2741#discussion_r884066302


##########
server/base/src/main/java/org/apache/accumulo/server/conf/store/PropStore.java:
##########
@@ -114,4 +114,12 @@ public interface PropStore {
    */
   void registerAsListener(PropCacheKey<?> propCacheKey, PropChangeListener listener);
 
+  /**
+   * Force a change event. This will signal the store to refresh the local cache on the next read.
+   * This does not generate a global event to other processes / caches.
+   *
+   * @param propCacheKey
+   *          the prop cache key
+   */
+  void clearLocal(PropCacheKey<?> propCacheKey);

Review Comment:
   This javadoc is really the first time it is explained anywhere in this interface that there's a locally cached copy of anything. Aside from the key being named "PropCacheKey", it's not really obvious that implementations of this interface will be doing any kind of caching. In fact, I think PropCacheKey should really be named PropStoreKey, because it's so tightly coupled to the PropStore interface, and a cache isn't necessarily implemented.
   
   I think instead of this method, it would be good to have two changes:
   
   1. Rename PropCacheKey to PropStoreKey (and subclasses accordingly)
   2. Add a method to this interface called `getCache()` that returns an instance of the cache, to make it more obvious that it is expected for a PropStore implementation to use one
   
   Then, instead of calling this `clearLocal` method, they could just do `context.propStore().getCache().remove(propStoreKey)`
   



-- 
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 pull request #2741: Add method to clear local store cache, update client service handler.

Posted by GitBox <gi...@apache.org>.
ctubbsii commented on PR #2741:
URL: https://github.com/apache/accumulo/pull/2741#issuecomment-1142326277

   This PR is basically a duplicate of what I was doing with #2740. Can we close this and collaborate / come to consensus there instead of having two open PRs trying to do the same basic thing?


-- 
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