[GitHub] [hbase] comnetwork commented on a change in pull request #3777: HBASE-26384 Segment already flushed to hfile may still be remained in…

[GitBox <gi...@apache.org>]

comnetwork commented on a change in pull request #3777:
URL: https://github.com/apache/hbase/pull/3777#discussion_r740113588



##########
File path: hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHStore.java
##########
@@ -1921,6 +1922,225 @@ private void doWriteTestLargeCellAndSmallCellConcurrently(
     }
   }
 
+  /**
+   * <pre>
+   * This test is for HBASE-26384,
+   * test {@link CompactingMemStore#flattenOneSegment} and {@link CompactingMemStore#snapshot()}
+   * execute concurrently.
+   * The threads sequence before HBASE-26384 is(The bug only exists for branch-2,and I add UTs
+   * for both branch-2 and master):
+   * 1. The {@link CompactingMemStore} size exceeds
+   *    {@link CompactingMemStore#getInmemoryFlushSize()},the write thread adds a new
+   *    {@link ImmutableSegment}  to the head of {@link CompactingMemStore#pipeline},and start a
+   *    in memory compact thread to execute {@link CompactingMemStore#inMemoryCompaction}.
+   * 2. The in memory compact thread starts and then stopping before
+   *    {@link CompactingMemStore#flattenOneSegment}.
+   * 3. The snapshot thread starts {@link CompactingMemStore#snapshot} concurrently,after the
+   *    snapshot thread executing {@link CompactingMemStore#getImmutableSegments},the in memory
+   *    compact thread continues.
+   *    Assuming {@link VersionedSegmentsList#version} returned from
+   *    {@link CompactingMemStore#getImmutableSegments} is v.
+   * 4. The snapshot thread stopping before {@link CompactingMemStore#swapPipelineWithNull}.
+   * 5. The in memory compact thread completes {@link CompactingMemStore#flattenOneSegment},
+   *    {@link CompactionPipeline#version} is still v.
+   * 6. The snapshot thread continues {@link CompactingMemStore#swapPipelineWithNull}, and because
+   *    {@link CompactionPipeline#version} is v, {@link CompactingMemStore#swapPipelineWithNull}
+   *    thinks it is successful and continue flushing,but the {@link ImmutableSegment} in
+   *    {@link CompactionPipeline} has changed because
+   *    {@link CompactingMemStore#flattenOneSegment},so the {@link ImmutableSegment} is not
+   *    removed in fact and still remaining in {@link CompactionPipeline}.
+   *
+   * After HBASE-26384, the 5-6 step is changed to following, which is expected behavior:
+   * 5. The in memory compact thread completes {@link CompactingMemStore#flattenOneSegment},
+   *    {@link CompactingMemStore#flattenOneSegment} change {@link CompactionPipeline#version} to
+   *    v+1.

Review comment:
       Yes ，I explain this at  the end of the JIRA description， because this `version `bug for  `CompactingMemStore#flattenOneSegment` is only exists for branch-2， for master，`version` is  correctly increased  for `CompactingMemStore#flattenOneSegment` ，so I made separate PRs for master(#3777) and branch-2(#3779) , so the code for master and brach-2 could keep consistent and master could also has UTs for this problem.
   and I included folowing comments at the head of this test method comments:
    /*....
     * The threads sequence before HBASE-26384 is(**The bug only exists for branch-2,and I add UTs
     * for both branch-2 and master**):
     * .../ 
   




-- 
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: issues-unsubscribe@hbase.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org