[HBASE-26210] HBase Write should be doomed to hang when cell size exceeds InmemoryFlushSize for CompactingMemStore

• Type: Bug
• Status: Resolved
• Priority: Critical
• Resolution: Fixed
• Affects Version/s: 3.0.0-alpha-1, 2.4.5
• Fix Version/s: 2.5.0, 3.0.0-alpha-2, 2.4.6, 2.3.7
• Component/s: in-memory-compaction
• Hadoop Flags: Reviewed
• Assignee: chenglei
• Reporter: chenglei
• Created: 19/Aug/21 11:40
• Updated: 02/Sep/21 18:50
• Resolved: 01/Sep/21 10:26

Description

Besides HBASE-26026, there is another case HBase Write would be hang when using CompactingMemstore :
The problem is caused by CompactingMemStore.checkAndAddToActiveSize and CompactingMemStore.shouldFlushInMemory :

425   private boolean checkAndAddToActiveSize(MutableSegment currActive, Cell cellToAdd,
426      MemStoreSizing memstoreSizing) {
427    if (shouldFlushInMemory(currActive, cellToAdd, memstoreSizing)) {
428      if (currActive.setInMemoryFlushed()) {
429        flushInMemory(currActive);
430        if (setInMemoryCompactionFlag()) {
431         // The thread is dispatched to do in-memory compaction in the background
              ......
 }

CompactingMemStore.checkAndAddToActiveSize invokes following CompactingMemStore.shouldFlushInMemory firstly , when the size of the cellToAdd is larger than inmemoryFlushSize(the default value is 2MB), this CompactingMemStore.shouldFlushInMemory method always return true and current HBase Write Thread may repeatedly enter CompactingMemStore.shouldFlushInMemory again and again and never get a chance to add the cellToAdd .At the same time, because of the MVCC, all the HBase writes after this write would be hang.

 protected boolean shouldFlushInMemory(MutableSegment currActive, Cell cellToAdd,
      MemStoreSizing memstoreSizing) {
     long cellSize = MutableSegment.getCellLength(cellToAdd);
     long segmentDataSize = currActive.getDataSize();
     while (segmentDataSize + cellSize < inmemoryFlushSize || inWalReplay) {
        // when replaying edits from WAL there is no need in in-memory flush regardless the size
        // otherwise size below flush threshold try to update atomically
       if (currActive.compareAndSetDataSize(segmentDataSize, segmentDataSize + cellSize)) {
          if (memstoreSizing != null) {
             memstoreSizing.incMemStoreSize(cellSize, 0, 0, 0);
          }
          // enough space for cell - no need to flush
          return false;
       }
       segmentDataSize = currActive.getDataSize();
     }
     // size above flush threshold
     return true;
  }

My PR not skip adding the size，it still add the size before really adding the cell to currActive , but it just change checking currActive size logic in CompactingMemStore.shouldFlushInMemory a little to make it similar to normal DefaultMemStore flushing to disk : it not make sure the size of currActive strictly less than inmemoryFlushSize any more, instead, it can tolerate the size of currActive just crossing the inmemoryFlushSize boundary at most one cell. Before adding the cell size, we make sure currActive size less than inmemoryFlushSize , and after adding cell size to the currActive size, we check whether its size exceeding inmemoryFlushSize , if it is , we flush the currActive in memory.

Further more, I also flatten the CompactingMemStore.shouldFlushInMemory into CompactingMemStore.checkAndAddToActiveSize and extract the flush in memory logic into a separate method to make the workflow more readable.

Issue Links

• links to: GitHub Pull Request #3604