Code cleanup

This commit is contained in:
Andrea Cavalli 2022-06-30 17:05:32 +02:00
parent 8e50976d27
commit caf55a633e
4 changed files with 62 additions and 47 deletions

View File

@ -5,13 +5,11 @@ import static it.cavallium.dbengine.client.UninterruptibleScheduler.uninterrupti
import com.google.common.cache.CacheBuilder; import com.google.common.cache.CacheBuilder;
import com.google.common.cache.CacheLoader; import com.google.common.cache.CacheLoader;
import com.google.common.cache.LoadingCache; import com.google.common.cache.LoadingCache;
import it.cavallium.dbengine.database.DiscardingCloseable;
import it.cavallium.dbengine.database.LLSnapshot; import it.cavallium.dbengine.database.LLSnapshot;
import it.cavallium.dbengine.database.LLUtils; import it.cavallium.dbengine.database.LLUtils;
import it.cavallium.dbengine.utils.SimpleResource; import it.cavallium.dbengine.utils.SimpleResource;
import java.io.IOException; import java.io.IOException;
import java.io.UncheckedIOException; import java.io.UncheckedIOException;
import java.lang.ref.Cleaner;
import java.time.Duration; import java.time.Duration;
import java.util.concurrent.ExecutorService; import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors; import java.util.concurrent.Executors;
@ -32,8 +30,6 @@ import org.jetbrains.annotations.Nullable;
import it.cavallium.dbengine.utils.ShortNamedThreadFactory; import it.cavallium.dbengine.utils.ShortNamedThreadFactory;
import reactor.core.Disposable; import reactor.core.Disposable;
import reactor.core.publisher.Mono; import reactor.core.publisher.Mono;
import reactor.core.publisher.Sinks;
import reactor.core.publisher.Sinks.Empty;
import reactor.core.scheduler.Scheduler; import reactor.core.scheduler.Scheduler;
import reactor.core.scheduler.Schedulers; import reactor.core.scheduler.Schedulers;
@ -116,31 +112,17 @@ public class CachedIndexSearcherManager extends SimpleResource implements IndexS
indexSearcher = searcherManager.acquire(); indexSearcher = searcherManager.acquire();
fromSnapshot = false; fromSnapshot = false;
} else { } else {
//noinspection resource
indexSearcher = snapshotsManager.resolveSnapshot(snapshot).getIndexSearcher(SEARCH_EXECUTOR); indexSearcher = snapshotsManager.resolveSnapshot(snapshot).getIndexSearcher(SEARCH_EXECUTOR);
fromSnapshot = true; fromSnapshot = true;
} }
indexSearcher.setSimilarity(similarity); indexSearcher.setSimilarity(similarity);
assert indexSearcher.getIndexReader().getRefCount() > 0; assert indexSearcher.getIndexReader().getRefCount() > 0;
var closed = new AtomicBoolean();
LLIndexSearcher llIndexSearcher; LLIndexSearcher llIndexSearcher;
if (fromSnapshot) { if (fromSnapshot) {
llIndexSearcher = new SnapshotIndexSearcher(indexSearcher, closed); llIndexSearcher = new SnapshotIndexSearcher(indexSearcher);
} else { } else {
llIndexSearcher = new MainIndexSearcher(indexSearcher, closed); llIndexSearcher = new MainIndexSearcher(indexSearcher, searcherManager);
} }
SimpleResource.CLEANER.register(llIndexSearcher, () -> {
if (closed.compareAndSet(false, true)) {
LOG.warn("An index searcher was not closed!");
if (!fromSnapshot) {
try {
searcherManager.release(indexSearcher);
} catch (IOException e) {
LOG.error("Failed to release the index searcher", e);
}
}
}
});
return llIndexSearcher; return llIndexSearcher;
} catch (Throwable ex) { } catch (Throwable ex) {
activeSearchers.decrementAndGet(); activeSearchers.decrementAndGet();
@ -230,8 +212,17 @@ public class CachedIndexSearcherManager extends SimpleResource implements IndexS
private class MainIndexSearcher extends LLIndexSearcher { private class MainIndexSearcher extends LLIndexSearcher {
public MainIndexSearcher(IndexSearcher indexSearcher, AtomicBoolean released) { public MainIndexSearcher(IndexSearcher indexSearcher, SearcherManager searcherManager) {
super(indexSearcher, released); super(indexSearcher, () -> releaseOnCleanup(searcherManager, indexSearcher));
}
private static void releaseOnCleanup(SearcherManager searcherManager, IndexSearcher indexSearcher) {
try {
LOG.warn("An index searcher was not closed!");
searcherManager.release(indexSearcher);
} catch (IOException ex) {
LOG.error("Failed to release the index searcher during cleanup: {}", indexSearcher, ex);
}
} }
@Override @Override
@ -247,11 +238,14 @@ public class CachedIndexSearcherManager extends SimpleResource implements IndexS
private class SnapshotIndexSearcher extends LLIndexSearcher { private class SnapshotIndexSearcher extends LLIndexSearcher {
public SnapshotIndexSearcher(IndexSearcher indexSearcher, public SnapshotIndexSearcher(IndexSearcher indexSearcher, AtomicBoolean closed) {
AtomicBoolean closed) {
super(indexSearcher, closed); super(indexSearcher, closed);
} }
public SnapshotIndexSearcher(IndexSearcher indexSearcher) {
super(indexSearcher);
}
@Override @Override
public void onClose() { public void onClose() {
dropCachedIndexSearcher(); dropCachedIndexSearcher();

View File

@ -30,6 +30,16 @@ public abstract class LLIndexSearcher extends SimpleResource implements Discardi
this.indexSearcher = indexSearcher; this.indexSearcher = indexSearcher;
} }
public LLIndexSearcher(IndexSearcher indexSearcher, AtomicBoolean closed, Runnable cleanAction) {
super(closed, cleanAction);
this.indexSearcher = indexSearcher;
}
public LLIndexSearcher(IndexSearcher indexSearcher, Runnable cleanAction) {
super(cleanAction);
this.indexSearcher = indexSearcher;
}
public IndexReader getIndexReader() { public IndexReader getIndexReader() {
ensureOpen(); ensureOpen();
return indexSearcher.getIndexReader(); return indexSearcher.getIndexReader();

View File

@ -148,25 +148,12 @@ public class SimpleIndexSearcherManager extends SimpleResource implements IndexS
} }
indexSearcher.setSimilarity(similarity); indexSearcher.setSimilarity(similarity);
assert indexSearcher.getIndexReader().getRefCount() > 0; assert indexSearcher.getIndexReader().getRefCount() > 0;
var closed = new AtomicBoolean();
LLIndexSearcher llIndexSearcher; LLIndexSearcher llIndexSearcher;
if (fromSnapshot) { if (fromSnapshot) {
llIndexSearcher = new SnapshotIndexSearcher(indexSearcher, closed); llIndexSearcher = new SnapshotIndexSearcher(indexSearcher);
} else { } else {
llIndexSearcher = new MainIndexSearcher(indexSearcher, closed); llIndexSearcher = new MainIndexSearcher(indexSearcher);
} }
SimpleResource.CLEANER.register(llIndexSearcher, () -> {
if (closed.compareAndSet(false, true)) {
LOG.warn("An index searcher was not closed!");
if (!fromSnapshot) {
try {
searcherManager.release(indexSearcher);
} catch (IOException e) {
LOG.error("Failed to release the index searcher", e);
}
}
}
});
return llIndexSearcher; return llIndexSearcher;
} catch (Throwable ex) { } catch (Throwable ex) {
activeSearchers.decrementAndGet(); activeSearchers.decrementAndGet();
@ -216,8 +203,17 @@ public class SimpleIndexSearcherManager extends SimpleResource implements IndexS
private class MainIndexSearcher extends LLIndexSearcher { private class MainIndexSearcher extends LLIndexSearcher {
public MainIndexSearcher(IndexSearcher indexSearcher, AtomicBoolean released) { public MainIndexSearcher(IndexSearcher indexSearcher) {
super(indexSearcher, released); super(indexSearcher, () -> releaseOnCleanup(searcherManager, indexSearcher));
}
private static void releaseOnCleanup(SearcherManager searcherManager, IndexSearcher indexSearcher) {
try {
LOG.warn("An index searcher was not closed!");
searcherManager.release(indexSearcher);
} catch (IOException ex) {
LOG.error("Failed to release the index searcher during cleanup: {}", indexSearcher, ex);
}
} }
@Override @Override
@ -233,9 +229,8 @@ public class SimpleIndexSearcherManager extends SimpleResource implements IndexS
private class SnapshotIndexSearcher extends LLIndexSearcher { private class SnapshotIndexSearcher extends LLIndexSearcher {
public SnapshotIndexSearcher(IndexSearcher indexSearcher, public SnapshotIndexSearcher(IndexSearcher indexSearcher) {
AtomicBoolean closed) { super(indexSearcher);
super(indexSearcher, closed);
} }
@Override @Override

View File

@ -6,6 +6,7 @@ import java.lang.ref.Cleaner;
import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicBoolean;
import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.Logger;
import org.jetbrains.annotations.Nullable;
public abstract class SimpleResource implements SafeCloseable { public abstract class SimpleResource implements SafeCloseable {
@ -23,15 +24,27 @@ public abstract class SimpleResource implements SafeCloseable {
this(true); this(true);
} }
public SimpleResource(@Nullable Runnable cleanerAction) {
this(true, cleanerAction);
}
protected SimpleResource(boolean canClose) { protected SimpleResource(boolean canClose) {
this(canClose, new AtomicBoolean()); this(canClose, null);
}
protected SimpleResource(boolean canClose, @Nullable Runnable cleanerAction) {
this(canClose, new AtomicBoolean(), cleanerAction);
} }
protected SimpleResource(AtomicBoolean closed) { protected SimpleResource(AtomicBoolean closed) {
this(true, closed); this(true, closed, null);
} }
private SimpleResource(boolean canClose, AtomicBoolean closed) { protected SimpleResource(AtomicBoolean closed, @Nullable Runnable cleanerAction) {
this(true, closed, cleanerAction);
}
private SimpleResource(boolean canClose, AtomicBoolean closed, @Nullable Runnable cleanerAction) {
this.canClose = canClose; this.canClose = canClose;
this.closed = closed; this.closed = closed;
@ -48,6 +61,9 @@ public abstract class SimpleResource implements SafeCloseable {
CLEANER.register(this, () -> { CLEANER.register(this, () -> {
if (!closed.get()) { if (!closed.get()) {
LOG.error("Resource leak of type {}", resourceClass, initializationStackTrace); LOG.error("Resource leak of type {}", resourceClass, initializationStackTrace);
if (cleanerAction != null) {
cleanerAction.run();
}
} }
}); });
} }