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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
