keith-turner commented on a change in pull request #2320:
URL: https://github.com/apache/accumulo/pull/2320#discussion_r745768097
##########
File path:
server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java
##########
@@ -874,6 +877,7 @@ public void flush(long tableFlushID) {
} else {
initiateMinor = true;
}
+ updateCounter.getAndIncrement();
Review comment:
I think it would be better to place this counter in the DataFileManager
class. Also would not make it an atomic long, would just make it a long. The
variable is tightly coupled to the map dataFileSizes in DataFileManager. Any
time that map is update, the counter needs to be incremented. This map update
and counter increment should all be done in the same sync block, so there is no
need to make the counter an atomic long. Making it an atomic long gives the
impression that it can be accessed on its own when in reality it should only be
accessed in the same sync scope as the map.
The DataFileManager class uses/relies on being accessed when the tablet lock
is held, so it does not really need its own synch.
So could add the counter to the DataFileManager class and updated whenever
the map is updated and also add a method to get the counter from
DataFileManager. Rely on the tablet lock for sync, there is another comment
related to this.
--
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]
