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") allowedOnPath = ".*/(HStore|TestHStore).java")
void replaceStoreFiles(Collection<HStoreFile> compactedFiles, Collection<HStoreFile> result, void replaceStoreFiles(Collection<HStoreFile> compactedFiles, Collection<HStoreFile> result,
boolean writeCompactionMarker) throws IOException { boolean writeCompactionMarker) throws IOException {
storeEngine.replaceStoreFiles(compactedFiles, result); storeEngine.replaceStoreFiles(compactedFiles, result, () -> {
synchronized(filesCompacting) {
filesCompacting.removeAll(compactedFiles);
}
});
if (writeCompactionMarker) { if (writeCompactionMarker) {
writeCompactionWalRecord(compactedFiles, result); 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 // 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. // sizes later so it's not super-critical if we miss these.
RegionServerServices rsServices = region.getRegionServerServices(); 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); List<HStoreFile> openedFiles = openStoreFiles(toBeAddedFiles, false);
// propogate the file changes to the underlying store file manager // 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, public void replaceStoreFiles(Collection<HStoreFile> compactedFiles,
Collection<HStoreFile> newFiles) throws IOException { Collection<HStoreFile> newFiles, Runnable actionUnderLock) throws IOException {
storeFileTracker.replace(StoreUtils.toStoreFileInfo(compactedFiles), storeFileTracker.replace(StoreUtils.toStoreFileInfo(compactedFiles),
StoreUtils.toStoreFileInfo(newFiles)); StoreUtils.toStoreFileInfo(newFiles));
writeLock(); writeLock();
try { try {
storeFileManager.addCompactionResults(compactedFiles, newFiles); storeFileManager.addCompactionResults(compactedFiles, newFiles);
actionUnderLock.run();
} finally { } finally {
writeUnlock(); writeUnlock();
} }

View File

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