HBASE-26674 Should modify filesCompacting under storeWriteLock (#4040)
Signed-off-by: Josh Elser <elserj@apache.org>
This commit is contained in:
parent
d4f2b66a43
commit
8fbc9a2606
|
@ -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();
|
||||||
|
|
|
@ -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();
|
||||||
}
|
}
|
||||||
|
|
|
@ -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) {
|
||||||
|
|
Loading…
Reference in New Issue