comnetwork commented on a change in pull request #3421:
URL: https://github.com/apache/hbase/pull/3421#discussion_r657733172
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/CompactingMemStore.java
##########
@@ -453,9 +453,14 @@ void flushInMemory() {
inMemoryCompaction();
}
- private void flushInMemory(MutableSegment currActive) {
+ protected void flushInMemory(MutableSegment currActive) {
LOG.trace("IN-MEMORY FLUSH: Pushing active segment into compaction
pipeline");
- pushActiveToPipeline(currActive);
+ // NOTE: Due to concurrent writes and because we first add cell size to
currActive.getDataSize
Review comment:
@Apache9 , Seems that we not need synchronization here：
1. Only one write thread can enter flushInMemory method.
2. An alternative fix is we use synchronization mechanism here to prevent
all writes , wait for all pending write completed(i.e.
currActive.waitForUpdates is called) and if currActive is still empty ,then we
set currActive.flushed back to false,but I am not inclined to use so heavy
synchronization in write path, and I think we would better maintain lockless
implementation for CompactingMemStore.add just as now and
currActive.waitForUpdates would better be left in background
InMemoryCompactionRunnable.
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
