HBASE-26675 Data race on Compactor.writer (#4035)

Signed-off-by: Xin Sun <ddupgs@gmail.com>
This commit is contained in:
Duo Zhang 2022-01-24 19:45:50 +08:00 committed by Andrew Purtell
parent 9435769420
commit 520b7e7c55
2 changed files with 16 additions and 16 deletions

View File

@ -95,7 +95,10 @@ public abstract class Compactor<T extends CellSink> {
private final boolean dropCacheMajor; private final boolean dropCacheMajor;
private final boolean dropCacheMinor; private final boolean dropCacheMinor;
protected T writer = null; // In compaction process only a single thread will access and write to this field, and
// getCompactionTargets is the only place we will access it other than the compaction thread, so
// make it volatile.
protected volatile T writer = null;
//TODO: depending on Store is not good but, realistically, all compactors currently do. //TODO: depending on Store is not good but, realistically, all compactors currently do.
Compactor(Configuration conf, HStore store) { Compactor(Configuration conf, HStore store) {
@ -546,17 +549,16 @@ public abstract class Compactor<T extends CellSink> {
dropDeletesFromRow, dropDeletesToRow); dropDeletesFromRow, dropDeletesToRow);
} }
public List<Path> getCompactionTargets(){ public List<Path> getCompactionTargets() {
if (writer == null){ T writer = this.writer;
if (writer == null) {
return Collections.emptyList(); return Collections.emptyList();
} }
synchronized (writer){ if (writer instanceof StoreFileWriter) {
if (writer instanceof StoreFileWriter){ return Arrays.asList(((StoreFileWriter) writer).getPath());
return Arrays.asList(((StoreFileWriter)writer).getPath());
}
return ((AbstractMultiFileWriter)writer).writers().stream().map(sfw -> sfw.getPath()).collect(
Collectors.toList());
} }
return ((AbstractMultiFileWriter) writer).writers().stream().map(sfw -> sfw.getPath())
.collect(Collectors.toList());
} }
/** /**

View File

@ -74,24 +74,22 @@ public class DefaultCompactor extends Compactor<StoreFileWriter> {
@Override @Override
protected void abortWriter() throws IOException { protected void abortWriter() throws IOException {
abortWriter(writer); abortWriter(writer);
// this step signals that the target file is no longer written and can be cleaned up
writer = null;
} }
protected void abortWriter(StoreFileWriter writer) throws IOException { protected final void abortWriter(StoreFileWriter writer) throws IOException {
Path leftoverFile = writer.getPath(); Path leftoverFile = writer.getPath();
try { try {
writer.close(); writer.close();
} catch (IOException e) { } catch (IOException e) {
LOG.warn("Failed to close the writer after an unfinished compaction.", e); LOG.warn("Failed to close the writer after an unfinished compaction.", e);
} finally {
//this step signals that the target file is no longer writen and can be cleaned up
writer = null;
} }
try { try {
store.getFileSystem().delete(leftoverFile, false); store.getFileSystem().delete(leftoverFile, false);
} catch (IOException e) { } catch (IOException e) {
LOG.warn( LOG.warn("Failed to delete the leftover file {} after an unfinished compaction.",
"Failed to delete the leftover file " + leftoverFile + " after an unfinished compaction.", leftoverFile, e);
e);
} }
} }
} }