From caf55a633e1279e5590cb95d9d15c2c93e4c659b Mon Sep 17 00:00:00 2001 From: Andrea Cavalli Date: Thu, 30 Jun 2022 17:05:32 +0200 Subject: [PATCH] Code cleanup --- .../disk/CachedIndexSearcherManager.java | 42 ++++++++----------- .../database/disk/LLIndexSearcher.java | 10 +++++ .../disk/SimpleIndexSearcherManager.java | 35 +++++++--------- .../dbengine/utils/SimpleResource.java | 22 ++++++++-- 4 files changed, 62 insertions(+), 47 deletions(-) diff --git a/src/main/java/it/cavallium/dbengine/database/disk/CachedIndexSearcherManager.java b/src/main/java/it/cavallium/dbengine/database/disk/CachedIndexSearcherManager.java index 6cdd472..e92425b 100644 --- a/src/main/java/it/cavallium/dbengine/database/disk/CachedIndexSearcherManager.java +++ b/src/main/java/it/cavallium/dbengine/database/disk/CachedIndexSearcherManager.java @@ -5,13 +5,11 @@ import static it.cavallium.dbengine.client.UninterruptibleScheduler.uninterrupti import com.google.common.cache.CacheBuilder; import com.google.common.cache.CacheLoader; import com.google.common.cache.LoadingCache; -import it.cavallium.dbengine.database.DiscardingCloseable; import it.cavallium.dbengine.database.LLSnapshot; import it.cavallium.dbengine.database.LLUtils; import it.cavallium.dbengine.utils.SimpleResource; import java.io.IOException; import java.io.UncheckedIOException; -import java.lang.ref.Cleaner; import java.time.Duration; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; @@ -32,8 +30,6 @@ import org.jetbrains.annotations.Nullable; import it.cavallium.dbengine.utils.ShortNamedThreadFactory; import reactor.core.Disposable; 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.Schedulers; @@ -116,31 +112,17 @@ public class CachedIndexSearcherManager extends SimpleResource implements IndexS indexSearcher = searcherManager.acquire(); fromSnapshot = false; } else { - //noinspection resource indexSearcher = snapshotsManager.resolveSnapshot(snapshot).getIndexSearcher(SEARCH_EXECUTOR); fromSnapshot = true; } indexSearcher.setSimilarity(similarity); assert indexSearcher.getIndexReader().getRefCount() > 0; - var closed = new AtomicBoolean(); LLIndexSearcher llIndexSearcher; if (fromSnapshot) { - llIndexSearcher = new SnapshotIndexSearcher(indexSearcher, closed); + llIndexSearcher = new SnapshotIndexSearcher(indexSearcher); } 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; } catch (Throwable ex) { activeSearchers.decrementAndGet(); @@ -230,8 +212,17 @@ public class CachedIndexSearcherManager extends SimpleResource implements IndexS private class MainIndexSearcher extends LLIndexSearcher { - public MainIndexSearcher(IndexSearcher indexSearcher, AtomicBoolean released) { - super(indexSearcher, released); + public MainIndexSearcher(IndexSearcher indexSearcher, SearcherManager searcherManager) { + 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 @@ -247,11 +238,14 @@ public class CachedIndexSearcherManager extends SimpleResource implements IndexS private class SnapshotIndexSearcher extends LLIndexSearcher { - public SnapshotIndexSearcher(IndexSearcher indexSearcher, - AtomicBoolean closed) { + public SnapshotIndexSearcher(IndexSearcher indexSearcher, AtomicBoolean closed) { super(indexSearcher, closed); } + public SnapshotIndexSearcher(IndexSearcher indexSearcher) { + super(indexSearcher); + } + @Override public void onClose() { dropCachedIndexSearcher(); diff --git a/src/main/java/it/cavallium/dbengine/database/disk/LLIndexSearcher.java b/src/main/java/it/cavallium/dbengine/database/disk/LLIndexSearcher.java index 6f9a2e8..d2c0710 100644 --- a/src/main/java/it/cavallium/dbengine/database/disk/LLIndexSearcher.java +++ b/src/main/java/it/cavallium/dbengine/database/disk/LLIndexSearcher.java @@ -30,6 +30,16 @@ public abstract class LLIndexSearcher extends SimpleResource implements Discardi 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() { ensureOpen(); return indexSearcher.getIndexReader(); diff --git a/src/main/java/it/cavallium/dbengine/database/disk/SimpleIndexSearcherManager.java b/src/main/java/it/cavallium/dbengine/database/disk/SimpleIndexSearcherManager.java index c8fe28d..57ee4fe 100644 --- a/src/main/java/it/cavallium/dbengine/database/disk/SimpleIndexSearcherManager.java +++ b/src/main/java/it/cavallium/dbengine/database/disk/SimpleIndexSearcherManager.java @@ -148,25 +148,12 @@ public class SimpleIndexSearcherManager extends SimpleResource implements IndexS } indexSearcher.setSimilarity(similarity); assert indexSearcher.getIndexReader().getRefCount() > 0; - var closed = new AtomicBoolean(); LLIndexSearcher llIndexSearcher; if (fromSnapshot) { - llIndexSearcher = new SnapshotIndexSearcher(indexSearcher, closed); + llIndexSearcher = new SnapshotIndexSearcher(indexSearcher); } 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; } catch (Throwable ex) { activeSearchers.decrementAndGet(); @@ -216,8 +203,17 @@ public class SimpleIndexSearcherManager extends SimpleResource implements IndexS private class MainIndexSearcher extends LLIndexSearcher { - public MainIndexSearcher(IndexSearcher indexSearcher, AtomicBoolean released) { - super(indexSearcher, released); + public MainIndexSearcher(IndexSearcher indexSearcher) { + 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 @@ -233,9 +229,8 @@ public class SimpleIndexSearcherManager extends SimpleResource implements IndexS private class SnapshotIndexSearcher extends LLIndexSearcher { - public SnapshotIndexSearcher(IndexSearcher indexSearcher, - AtomicBoolean closed) { - super(indexSearcher, closed); + public SnapshotIndexSearcher(IndexSearcher indexSearcher) { + super(indexSearcher); } @Override diff --git a/src/main/java/it/cavallium/dbengine/utils/SimpleResource.java b/src/main/java/it/cavallium/dbengine/utils/SimpleResource.java index ac88412..321501b 100644 --- a/src/main/java/it/cavallium/dbengine/utils/SimpleResource.java +++ b/src/main/java/it/cavallium/dbengine/utils/SimpleResource.java @@ -6,6 +6,7 @@ import java.lang.ref.Cleaner; import java.util.concurrent.atomic.AtomicBoolean; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.jetbrains.annotations.Nullable; public abstract class SimpleResource implements SafeCloseable { @@ -23,15 +24,27 @@ public abstract class SimpleResource implements SafeCloseable { this(true); } + public SimpleResource(@Nullable Runnable cleanerAction) { + this(true, cleanerAction); + } + 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) { - 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.closed = closed; @@ -48,6 +61,9 @@ public abstract class SimpleResource implements SafeCloseable { CLEANER.register(this, () -> { if (!closed.get()) { LOG.error("Resource leak of type {}", resourceClass, initializationStackTrace); + if (cleanerAction != null) { + cleanerAction.run(); + } } }); }