ctubbsii commented on code in PR #4746:
URL: https://github.com/apache/accumulo/pull/4746#discussion_r1686910802
##########
core/src/main/java/org/apache/accumulo/core/conf/AccumuloConfiguration.java:
##########
@@ -512,21 +512,25 @@ public T derive() {
if (rc == null || rc.count != uc) {
T newObj = converter.apply(AccumuloConfiguration.this);
- // very important to record the update count that was obtained before
recomputing.
- RefCount<T> nrc = new RefCount<>(uc, newObj);
-
- /*
- * The return value of compare and set is intentionally ignored here.
This code could loop
- * calling compare and set inorder to avoid returning a stale object.
However after this
- * function returns, the object could immediately become stale. So in
the big picture stale
- * objects can not be prevented. Looping here could cause thread
contention, but it would
- * not solve the overall stale object problem. That is why the return
value was ignored. The
- * following line is a least effort attempt to make the result of this
recomputation
- * available to the next caller.
- */
- refref.compareAndSet(rc, nrc);
-
- return nrc.obj;
+ if (newObj != null) {
Review Comment:
For this if-else statement, I think it would make sense if you reversed the
blocks and checked `if (newObj == null)` here instead. It is often easier to
grok code with fewer negations.
Plus, in this case, you can just throw a return statement in the if case,
and omit the else block indentation entirely. That would make the diff smaller
too.
--
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]
