kevinrr888 commented on code in PR #6025:
URL: https://github.com/apache/accumulo/pull/6025#discussion_r2640442963
##########
server/base/src/main/java/org/apache/accumulo/server/tablets/TabletTime.java:
##########
@@ -157,11 +160,9 @@ private LogicalTime(Long time) {
@Override
public void useMaxTimeFromWALog(long time) {
- time++;
+ final long finalTime = time + 1;
- if (this.nextTime.get() < time) {
- this.nextTime.set(time);
- }
+ this.nextTime.getAndUpdate(val -> Math.max(val, finalTime));
Review Comment:
Yeah, fixed in f827d1a99a1902ccd8386f65ad88a70f97392b48. When looking at
this again, realized this method is only called in the constructor of Tablet
before TabletTime is available to multiple threads, so technically no changes
were needed at all here and there was no bug. But only using one atomic op here
is still probably better.
##########
core/src/main/java/org/apache/accumulo/core/client/ClientSideIteratorScanner.java:
##########
@@ -177,6 +177,7 @@ public ClientSideIteratorScanner(final Scanner scanner) {
this.range = scanner.getRange();
this.size = scanner.getBatchSize();
this.retryTimeout = scanner.getTimeout(MILLISECONDS);
+ // TODO Is this intended to be getBatchTimeout()?
Review Comment:
Fixed in f827d1a99a1902ccd8386f65ad88a70f97392b48
--
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]
