SwaraliJoshi commented on code in PR #7552:
URL: https://github.com/apache/hbase/pull/7552#discussion_r3365919869
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHStore.java:
##########
@@ -1775,6 +1776,204 @@ public void testReclaimChunkWhenScaning() throws
IOException {
}
}
+ @Test
+ public void testReaderRaceConditionDuringCompaction() throws Exception {
+ HBaseTestingUtil util = new HBaseTestingUtil();
+ util.startMiniCluster(1);
+
+ try {
+ Configuration conf = util.getConfiguration();
+
+ byte[] FAMILY = Bytes.toBytes("f");
+ byte[] Q = Bytes.toBytes("q");
+ TableName TABLE = TableName.valueOf("race_test");
+
+ TableDescriptor td = TableDescriptorBuilder.newBuilder(TABLE)
+ .setColumnFamily(
+ ColumnFamilyDescriptorBuilder.newBuilder(FAMILY)
+ .setMaxVersions(1)
+ .build())
+ .build();
+
+ util.getAdmin().createTable(td);
+
+ HRegion region =
+ util.getMiniHBaseCluster().getRegions(TABLE).get(0);
+
+ HStore store = region.getStore(FAMILY);
+
+ for (int i = 0; i < 4; i++) {
+ Put p = new Put(Bytes.toBytes("row-" + i));
+ p.addColumn(FAMILY, Q, Bytes.toBytes(i));
+ region.put(p);
+ region.flush(true); // force store file
+ }
+
+ // sanity check
+ assertEquals(4, store.getStorefilesCount());
+
+ DefaultStoreFileManager sfm =
+ (DefaultStoreFileManager) store.getStoreEngine().getStoreFileManager();
+
+ HStoreFile victim =
+ sfm.getStoreFiles().iterator().next();
+
+ AtomicReference<Throwable> failure = new AtomicReference<>();
+ CountDownLatch start = new CountDownLatch(1);
+ CountDownLatch done = new CountDownLatch(2);
+
+ Thread remover = new Thread(() -> {
+ try {
+ start.await();
+ victim.closeStoreFile(true); // async-style close
Review Comment:
The discharger, while archiving a compacted file, reaches closeStoreFile
through this chain:
`HStore.removeCompactedfiles` →
`storeFileTracker.removeStoreFiles(filesToRemove)` (HStore.java:2333) →
`HFileArchiver.archiveStoreFiles` → `resolveAndArchiveFile` →
`FileableStoreFile.moveAndClose` → `this.close()`:
```
HFileArchiver.java
Lines 887-890
public void close() throws IOException {
file.closeStoreFile(true); // -> HStoreFile.closeStoreFile ->
initialReader = null
}
```
This nulling runs under **archiveLock** only — not under
storeEngine.writeLock — and it happens at line 2333, before clearCompactedfiles
(line 2357) removes the file from the compactedfiles list. (Note: the
r.close(evictOnClose) at line 2306 closes the reader object but operates on a
local variable; it does not null the field. The field nulling is only the
closeStoreFile above.)
Now during this time if addCompactionResults is running, we see NPE because
compaction/selection thread (HStore.compact/removeUnneededFiles →
`replaceStoreFiles` → `StoreEngine.replaceStoreFiles` →
`addCompactionResults`): runs under storeEngine.writeLock and sorts
Iterables.concat(compactedfiles, newCompactedfiles).
--
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]
