HBASE-26674 Should modify filesCompacting under storeWriteLock (#4040)

Signed-off-by: Josh Elser <elserj@apache.org>
This commit is contained in:
Duo Zhang 2022-01-19 13:59:35 +08:00
parent d4f2b66a43
commit 8fbc9a2606
3 changed files with 11 additions and 8 deletions

View File

@ -1229,13 +1229,14 @@ public class HStore implements Store, HeapSize, StoreConfigInformation,
allowedOnPath = ".*/(HStore|TestHStore).java")
void replaceStoreFiles(Collection<HStoreFile> compactedFiles, Collection<HStoreFile> result,
boolean writeCompactionMarker) throws IOException {
storeEngine.replaceStoreFiles(compactedFiles, result);
storeEngine.replaceStoreFiles(compactedFiles, result, () -> {
synchronized(filesCompacting) {
filesCompacting.removeAll(compactedFiles);
}
});
if (writeCompactionMarker) {
writeCompactionWalRecord(compactedFiles, result);
}
synchronized (filesCompacting) {
filesCompacting.removeAll(compactedFiles);
}
// These may be null when the RS is shutting down. The space quota Chores will fix the Region
// sizes later so it's not super-critical if we miss these.
RegionServerServices rsServices = region.getRegionServerServices();

View File

@ -410,7 +410,8 @@ public abstract class StoreEngine<SF extends StoreFlusher, CP extends Compaction
List<HStoreFile> openedFiles = openStoreFiles(toBeAddedFiles, false);
// propogate the file changes to the underlying store file manager
replaceStoreFiles(toBeRemovedStoreFiles, openedFiles); // won't throw an exception
replaceStoreFiles(toBeRemovedStoreFiles, openedFiles, () -> {
}); // won't throw an exception
}
/**
@ -493,12 +494,13 @@ public abstract class StoreEngine<SF extends StoreFlusher, CP extends Compaction
}
public void replaceStoreFiles(Collection<HStoreFile> compactedFiles,
Collection<HStoreFile> newFiles) throws IOException {
Collection<HStoreFile> newFiles, Runnable actionUnderLock) throws IOException {
storeFileTracker.replace(StoreUtils.toStoreFileInfo(compactedFiles),
StoreUtils.toStoreFileInfo(newFiles));
writeLock();
try {
storeFileManager.addCompactionResults(compactedFiles, newFiles);
actionUnderLock.run();
} finally {
writeUnlock();
}

View File

@ -1034,14 +1034,14 @@ public class TestHStore {
// call first time after files changed
spiedStoreEngine.refreshStoreFiles();
assertEquals(2, this.store.getStorefilesCount());
verify(spiedStoreEngine, times(1)).replaceStoreFiles(any(), any());
verify(spiedStoreEngine, times(1)).replaceStoreFiles(any(), any(), any());
// call second time
spiedStoreEngine.refreshStoreFiles();
// ensure that replaceStoreFiles is not called, i.e, the times does not change, if files are not
// refreshed,
verify(spiedStoreEngine, times(1)).replaceStoreFiles(any(), any());
verify(spiedStoreEngine, times(1)).replaceStoreFiles(any(), any(), any());
}
private long countMemStoreScanner(StoreScanner scanner) {