ctubbsii commented on code in PR #5338:
URL: https://github.com/apache/accumulo/pull/5338#discussion_r1956825770
##########
core/src/main/java/org/apache/accumulo/core/zookeeper/ZooCache.java:
##########
@@ -484,13 +484,18 @@ protected void copyStats(ZcStat userStat, ZcStat
cachedStat) {
* Clears this cache.
*/
protected void clear() {
- Preconditions.checkState(!closed);
+ if (closed) {
+ return;
+ }
nodeCache.clear();
updateCount.incrementAndGet();
log.trace("{} cleared all from cache", cacheId);
}
public void close() {
+ if (closed) {
Review Comment:
I came here to make the same comment. I know it was changed back on purpose.
But the reasoning stated was "that seems fine here". While it may be "fine", I
still think the use of compareAndSet is cleaner, and is better than the using
volatile directly. Internally, the AtomicBoolean uses a volatile, so it has the
same memory effects either way... but the AtomicBoolean is easier to reason
about, because it makes it clear what the memory effects are without needing to
check the flags on the variable, and compareAndSet is a nice clean API.
--
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]
