diff --git a/nitrite-mvstore-adapter/src/main/java/org/dizitart/no2/mvstore/MVStoreModuleBuilder.java b/nitrite-mvstore-adapter/src/main/java/org/dizitart/no2/mvstore/MVStoreModuleBuilder.java index 215ffbe2..63bcdddc 100644 --- a/nitrite-mvstore-adapter/src/main/java/org/dizitart/no2/mvstore/MVStoreModuleBuilder.java +++ b/nitrite-mvstore-adapter/src/main/java/org/dizitart/no2/mvstore/MVStoreModuleBuilder.java @@ -16,25 +16,26 @@ package org.dizitart.no2.mvstore; +import java.io.File; +import java.util.HashSet; +import java.util.Set; + +import org.dizitart.no2.store.events.StoreEventListener; +import org.h2.mvstore.FileStore; + import lombok.AccessLevel; import lombok.Getter; import lombok.Setter; import lombok.experimental.Accessors; -import org.dizitart.no2.store.events.StoreEventListener; -import org.h2.mvstore.FileStore; - -import java.io.File; -import java.util.HashSet; -import java.util.Set; /** * The MVStoreModuleBuilder class is responsible for building an instance of * {@link MVStoreModule}. It provides methods to set various configuration * options for the MVStore database. - * - * @since 4.0 - * @see MVStoreModule + * * @author Anindya Chatterjee + * @see MVStoreModule + * @since 4.0 */ @Getter @Setter @@ -81,6 +82,13 @@ public class MVStoreModuleBuilder { */ private boolean autoCommit = true; + /** + * Flag to enable/disable auto-compact mode. If set to true, fragmented + * chunks or chunks that are sufficiently below the target fill rate of 90% + * are reclaimed. This will typically shrink the file. + */ + private boolean autoCompact = true; + /** * Indicates whether the MVStore should be opened in recovery mode or not. */ @@ -174,6 +182,7 @@ public MVStoreModule build() { dbConfig.compress(compress()); dbConfig.compressHigh(compressHigh()); dbConfig.autoCommit(autoCommit()); + dbConfig.autoCompact(autoCompact()); dbConfig.recoveryMode(recoveryMode()); dbConfig.cacheSize(cacheSize()); dbConfig.cacheConcurrency(cacheConcurrency()); diff --git a/nitrite-mvstore-adapter/src/main/java/org/dizitart/no2/mvstore/MVStoreUtils.java b/nitrite-mvstore-adapter/src/main/java/org/dizitart/no2/mvstore/MVStoreUtils.java index e5c7cc7d..e816ddd2 100644 --- a/nitrite-mvstore-adapter/src/main/java/org/dizitart/no2/mvstore/MVStoreUtils.java +++ b/nitrite-mvstore-adapter/src/main/java/org/dizitart/no2/mvstore/MVStoreUtils.java @@ -16,17 +16,18 @@ package org.dizitart.no2.mvstore; -import lombok.extern.slf4j.Slf4j; +import static org.dizitart.no2.common.util.StringUtils.isNullOrEmpty; + +import java.io.File; +import java.util.Map; + import org.dizitart.no2.exceptions.InvalidOperationException; import org.dizitart.no2.exceptions.NitriteIOException; import org.dizitart.no2.mvstore.compat.v1.UpgradeUtil; import org.h2.mvstore.MVStore; import org.h2.mvstore.MVStoreException; -import java.io.File; -import java.util.Map; - -import static org.dizitart.no2.common.util.StringUtils.isNullOrEmpty; +import lombok.extern.slf4j.Slf4j; /** * @author Anindya Chatterjee. @@ -55,7 +56,7 @@ static MVStore openOrCreate(MVStoreConfig storeConfig) { } } catch (MVStoreException me) { if (me.getMessage().contains("file is locked")) { - throw new NitriteIOException("Database is already opened in other process"); + throw new NitriteIOException("Database is already opened in other process", me); } if (dbFile != null) { @@ -128,8 +129,10 @@ private static MVStore.Builder createBuilder(MVStoreConfig mvStoreConfig) { builder = builder.autoCommitBufferSize(mvStoreConfig.autoCommitBufferSize()); } - // auto compact disabled github issue #41 - builder.autoCompactFillRate(0); + if (!mvStoreConfig.autoCompact()) { + // disables background compaction + builder.autoCompactFillRate(0); + } if (mvStoreConfig.encryptionKey() != null) { builder = builder.encryptionKey(mvStoreConfig.encryptionKey()); diff --git a/nitrite-mvstore-adapter/src/main/java/org/dizitart/no2/mvstore/NitriteMVMap.java b/nitrite-mvstore-adapter/src/main/java/org/dizitart/no2/mvstore/NitriteMVMap.java index 64763bfc..0a2a195d 100644 --- a/nitrite-mvstore-adapter/src/main/java/org/dizitart/no2/mvstore/NitriteMVMap.java +++ b/nitrite-mvstore-adapter/src/main/java/org/dizitart/no2/mvstore/NitriteMVMap.java @@ -16,49 +16,58 @@ package org.dizitart.no2.mvstore; +import static org.dizitart.no2.common.util.ValidationUtils.notNull; + +import java.lang.ref.Cleaner; +import java.util.AbstractMap; +import java.util.Collections; +import java.util.Iterator; +import java.util.Map; +import java.util.NoSuchElementException; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.function.Supplier; + import org.dizitart.no2.common.RecordStream; import org.dizitart.no2.common.streams.SkippableIterator; import org.dizitart.no2.common.tuples.Pair; +import org.dizitart.no2.exceptions.NitriteIOException; import org.dizitart.no2.store.NitriteMap; import org.dizitart.no2.store.NitriteStore; import org.h2.mvstore.Cursor; import org.h2.mvstore.MVMap; import org.h2.mvstore.MVStore; -import java.util.AbstractMap; -import java.util.Collections; -import java.util.Iterator; -import java.util.Map; -import java.util.concurrent.atomic.AtomicBoolean; - -import static org.dizitart.no2.common.util.ValidationUtils.notNull; - /** - * @since 1.0 * @author Anindya Chatterjee + * @since 1.0 */ class NitriteMVMap implements NitriteMap { + private final MVMap mvMap; private final NitriteStore nitriteStore; private final MVStore mvStore; private final AtomicBoolean droppedFlag; private final AtomicBoolean closedFlag; + private final Set versionUsages; - NitriteMVMap(MVMap mvMap, NitriteStore nitriteStore) { + NitriteMVMap(final MVMap mvMap, final NitriteStore nitriteStore) { this.mvMap = mvMap; this.nitriteStore = nitriteStore; this.mvStore = mvMap.getStore(); this.closedFlag = new AtomicBoolean(false); this.droppedFlag = new AtomicBoolean(false); + this.versionUsages = ConcurrentHashMap.newKeySet(); } @Override - public boolean containsKey(Key key) { + public boolean containsKey(final Key key) { return mvMap.containsKey(key); } @Override - public Value get(Key key) { + public Value get(final Key key) { return mvMap.get(key); } @@ -69,7 +78,7 @@ public NitriteStore getStore() { @Override public void clear() { - MVStore.TxCounter txCounter = mvStore.registerVersionUsage(); + final MVStore.TxCounter txCounter = mvStore.registerVersionUsage(); try { mvMap.clear(); updateLastModifiedTime(); @@ -85,14 +94,14 @@ public String getName() { @Override public RecordStream values() { - return RecordStream.fromIterable(mvMap.values()); + return () -> versionedIterator(() -> mvMap.values().iterator()); } @Override - public Value remove(Key key) { - MVStore.TxCounter txCounter = mvStore.registerVersionUsage(); + public Value remove(final Key key) { + final MVStore.TxCounter txCounter = mvStore.registerVersionUsage(); try { - Value value = mvMap.remove(key); + final Value value = mvMap.remove(key); updateLastModifiedTime(); return value; } finally { @@ -102,13 +111,13 @@ public Value remove(Key key) { @Override public RecordStream keys() { - return RecordStream.fromIterable(mvMap.keySet()); + return () -> versionedIterator(() -> mvMap.keySet().iterator()); } @Override - public void put(Key key, Value value) { + public void put(final Key key, final Value value) { notNull(value, "value cannot be null"); - MVStore.TxCounter txCounter = mvStore.registerVersionUsage(); + final MVStore.TxCounter txCounter = mvStore.registerVersionUsage(); try { mvMap.put(key, value); updateLastModifiedTime(); @@ -123,11 +132,11 @@ public long size() { } @Override - public Value putIfAbsent(Key key, Value value) { + public Value putIfAbsent(final Key key, final Value value) { notNull(value, "value cannot be null"); - MVStore.TxCounter txCounter = mvStore.registerVersionUsage(); + final MVStore.TxCounter txCounter = mvStore.registerVersionUsage(); try { - Value v = mvMap.putIfAbsent(key, value); + final Value v = mvMap.putIfAbsent(key, value); updateLastModifiedTime(); return v; } finally { @@ -137,7 +146,7 @@ public Value putIfAbsent(Key key, Value value) { @Override public RecordStream> entries() { - return EntryIterator::new; + return () -> versionedIterator(EntryIterator::new); } /** @@ -201,7 +210,11 @@ public Map.Entry next() { @Override public RecordStream> reversedEntries() { - return () -> new ReverseIterator<>(mvMap); + return () -> versionedIterator(() -> new ReverseIterator<>(mvMap)); + } + + private Iterator versionedIterator(final Supplier> iteratorSupplier) { + return new VersionedIterator<>(mvStore, iteratorSupplier, versionUsages); } @Override @@ -215,22 +228,22 @@ public Key lastKey() { } @Override - public Key higherKey(Key key) { + public Key higherKey(final Key key) { return mvMap.higherKey(key); } @Override - public Key ceilingKey(Key key) { + public Key ceilingKey(final Key key) { return mvMap.ceilingKey(key); } @Override - public Key lowerKey(Key key) { + public Key lowerKey(final Key key) { return mvMap.lowerKey(key); } @Override - public Key floorKey(Key key) { + public Key floorKey(final Key key) { return mvMap.floorKey(key); } @@ -244,11 +257,12 @@ public void drop() { if (!droppedFlag.get()) { droppedFlag.compareAndSet(false, true); closedFlag.compareAndSet(false, true); + releaseVersionUsages(); - MVStore.TxCounter txCounter = mvStore.registerVersionUsage(); + final MVStore.TxCounter txCounter = mvStore.registerVersionUsage(); try { - nitriteStore.closeMap(getName()); - nitriteStore.removeMap(getName()); + nitriteStore.closeMap(mvMap.getName()); + nitriteStore.removeMap(mvMap.getName()); } finally { mvStore.deregisterVersionUsage(txCounter); } @@ -264,7 +278,8 @@ public boolean isDropped() { public void close() { if (!closedFlag.get() && !droppedFlag.get()) { closedFlag.compareAndSet(false, true); - nitriteStore.closeMap(getName()); + releaseVersionUsages(); + nitriteStore.closeMap(mvMap.getName()); } } @@ -272,4 +287,89 @@ public void close() { public boolean isClosed() { return closedFlag.get(); } + + private void releaseVersionUsages() { + for (final VersionUsage versionUsage : versionUsages) { + versionUsage.release(); + } + } + + private static class VersionedIterator implements Iterator, SkippableIterator { + + private final Iterator iterator; + private final Cleaner.Cleanable cleanable; + private final VersionUsage versionUsage; + private boolean exhausted; + + private VersionedIterator(final MVStore mvStore, + final Supplier> iteratorSupplier, + final Set versionUsages) { + + versionUsage = new VersionUsage(mvStore, mvStore.registerVersionUsage(), versionUsages); + versionUsages.add(versionUsage); + + try { + this.iterator = iteratorSupplier.get(); + this.cleanable = VersionUsage.CLEANER.register(this, versionUsage::release); + } catch (final RuntimeException | Error e) { + versionUsage.release(); + throw e; + } + } + + @Override + public boolean hasNext() { + if (exhausted) { + return false; + } + ensureOpen(); + try { + final boolean hasNext = iterator.hasNext(); + if (!hasNext) { + exhausted = true; + cleanable.clean(); + } + return hasNext; + } catch (final RuntimeException | Error e) { + cleanable.clean(); + throw e; + } + } + + @Override + public Element next() { + if (exhausted) { + throw new NoSuchElementException(); + } + ensureOpen(); + try { + return iterator.next(); + } catch (final RuntimeException | Error e) { + cleanable.clean(); + throw e; + } + } + + @Override + public long skip(final long count) { + ensureOpen(); + if (iterator instanceof SkippableIterator) { + return ((SkippableIterator) iterator).skip(count); + } + // The wrapper is uniformly skippable so BoundedStream never has to unwrap it; a + // delegate that cannot seek pays the same loop BoundedStream would have run itself. + long skipped = 0; + while (skipped < count && hasNext()) { + next(); + skipped++; + } + return skipped; + } + + private void ensureOpen() { + if (versionUsage.isReleased()) { + throw new NitriteIOException("MVStore is closed"); + } + } + } } diff --git a/nitrite-mvstore-adapter/src/main/java/org/dizitart/no2/mvstore/NitriteMVRTreeMap.java b/nitrite-mvstore-adapter/src/main/java/org/dizitart/no2/mvstore/NitriteMVRTreeMap.java index 219d22cd..e6ee72f8 100644 --- a/nitrite-mvstore-adapter/src/main/java/org/dizitart/no2/mvstore/NitriteMVRTreeMap.java +++ b/nitrite-mvstore-adapter/src/main/java/org/dizitart/no2/mvstore/NitriteMVRTreeMap.java @@ -16,36 +16,45 @@ package org.dizitart.no2.mvstore; +import java.lang.ref.Cleaner; +import java.util.Iterator; +import java.util.NoSuchElementException; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; +import java.util.function.Supplier; + import org.dizitart.no2.collection.NitriteId; import org.dizitart.no2.common.RecordStream; +import org.dizitart.no2.exceptions.NitriteIOException; import org.dizitart.no2.index.BoundingBox; import org.dizitart.no2.store.NitriteRTree; import org.dizitart.no2.store.NitriteStore; import org.h2.mvstore.MVStore; import org.h2.mvstore.rtree.MVRTreeMap; -import java.util.Iterator; - /** - * @since 1.0 * @author Anindya Chatterjee + * @since 1.0 */ class NitriteMVRTreeMap implements NitriteRTree { + private final MVRTreeMap mvMap; private final NitriteStore nitriteStore; private final MVStore mvStore; + private final Set versionUsages; - NitriteMVRTreeMap(MVRTreeMap mvMap, NitriteStore nitriteStore) { + NitriteMVRTreeMap(final MVRTreeMap mvMap, final NitriteStore nitriteStore) { this.mvMap = mvMap; this.nitriteStore = nitriteStore; this.mvStore = mvMap.getStore(); + this.versionUsages = ConcurrentHashMap.newKeySet(); } @Override - public void add(Key key, NitriteId nitriteId) { + public void add(final Key key, final NitriteId nitriteId) { if (nitriteId != null) { - MVSpatialKey spatialKey = getKey(key, nitriteId.getIdValue()); - MVStore.TxCounter txCounter = mvStore.registerVersionUsage(); + final MVSpatialKey spatialKey = getKey(key, nitriteId.getIdValue()); + final MVStore.TxCounter txCounter = mvStore.registerVersionUsage(); try { mvMap.add(spatialKey, key); } finally { @@ -55,10 +64,10 @@ public void add(Key key, NitriteId nitriteId) { } @Override - public void remove(Key key, NitriteId nitriteId) { + public void remove(final Key key, final NitriteId nitriteId) { if (nitriteId != null) { - MVSpatialKey spatialKey = getKey(key, nitriteId.getIdValue()); - MVStore.TxCounter txCounter = mvStore.registerVersionUsage(); + final MVSpatialKey spatialKey = getKey(key, nitriteId.getIdValue()); + final MVStore.TxCounter txCounter = mvStore.registerVersionUsage(); try { mvMap.remove(spatialKey); } finally { @@ -68,17 +77,15 @@ public void remove(Key key, NitriteId nitriteId) { } @Override - public RecordStream findIntersectingKeys(Key key) { - MVSpatialKey spatialKey = getKey(key, 0L); - MVRTreeMap.RTreeCursor treeCursor = mvMap.findIntersectingKeys(spatialKey); - return getRecordStream(treeCursor); + public RecordStream findIntersectingKeys(final Key key) { + final MVSpatialKey spatialKey = getKey(key, 0L); + return getRecordStream(() -> mvMap.findIntersectingKeys(spatialKey)); } @Override - public RecordStream findContainedKeys(Key key) { - MVSpatialKey spatialKey = getKey(key, 0L); - MVRTreeMap.RTreeCursor treeCursor = mvMap.findContainedKeys(spatialKey); - return getRecordStream(treeCursor); + public RecordStream findContainedKeys(final Key key) { + final MVSpatialKey spatialKey = getKey(key, 0L); + return getRecordStream(() -> mvMap.findContainedKeys(spatialKey)); } @Override @@ -86,7 +93,7 @@ public long size() { return mvMap.sizeAsLong(); } - private MVSpatialKey getKey(Key key, long id) { + private MVSpatialKey getKey(final Key key, final long id) { if (key == null || key.equals(BoundingBox.EMPTY)) { return new MVSpatialKey(id); } else { @@ -95,24 +102,14 @@ private MVSpatialKey getKey(Key key, long id) { } } - private RecordStream getRecordStream(MVRTreeMap.RTreeCursor treeCursor) { - //noinspection Convert2Diamond - return RecordStream.fromIterable(() -> new Iterator() { - @Override - public boolean hasNext() { - return treeCursor.hasNext(); - } - - @Override - public NitriteId next() { - MVSpatialKey next = (MVSpatialKey) treeCursor.next(); - return NitriteId.createId(Long.toString(next.getId())); - } - }); + private RecordStream getRecordStream( + final Supplier> cursorSupplier) { + return RecordStream.fromIterable(() -> new VersionedCursor(cursorSupplier)); } @Override public void close() { + releaseVersionUsages(); nitriteStore.closeRTree(mvMap.getName()); } @@ -123,8 +120,82 @@ public void clear() { @Override public void drop() { + releaseVersionUsages(); mvMap.clear(); nitriteStore.closeRTree(mvMap.getName()); nitriteStore.removeRTree(mvMap.getName()); } + + private void releaseVersionUsages() { + for (final VersionUsage versionUsage : versionUsages) { + versionUsage.release(); + } + } + + private class VersionedCursor implements Iterator { + + private final MVRTreeMap.RTreeCursor treeCursor; + private final Cleaner.Cleanable cleanable; + private final VersionUsage versionUsage; + private boolean exhausted; + + private VersionedCursor(final Supplier> cursorSupplier) { + + versionUsage = new VersionUsage(mvStore, mvStore.registerVersionUsage(), versionUsages); + versionUsages.add(versionUsage); + + try { + treeCursor = cursorSupplier.get(); + cleanable = VersionUsage.CLEANER.register(this, versionUsage::release); + } catch (final RuntimeException | Error e) { + versionUsage.release(); + throw e; + } + } + + @Override + public boolean hasNext() { + if (exhausted) { + return false; + } + ensureOpen(); + try { + final boolean hasNext = treeCursor.hasNext(); + if (!hasNext) { + exhausted = true; + cleanable.clean(); + } + return hasNext; + } catch (final RuntimeException | Error e) { + cleanable.clean(); + throw e; + } + } + + @Override + public NitriteId next() { + if (exhausted) { + throw new NoSuchElementException(); + } + ensureOpen(); + try { + final MVSpatialKey next = (MVSpatialKey) treeCursor.next(); + if (next == null) { + exhausted = true; + cleanable.clean(); + throw new NoSuchElementException(); + } + return NitriteId.createId(Long.toString(next.getId())); + } catch (final RuntimeException | Error e) { + cleanable.clean(); + throw e; + } + } + + private void ensureOpen() { + if (versionUsage.isReleased()) { + throw new NitriteIOException("MVStore is closed"); + } + } + } } diff --git a/nitrite-mvstore-adapter/src/main/java/org/dizitart/no2/mvstore/NitriteMVStore.java b/nitrite-mvstore-adapter/src/main/java/org/dizitart/no2/mvstore/NitriteMVStore.java index 59fa5851..c6a9c0f7 100644 --- a/nitrite-mvstore-adapter/src/main/java/org/dizitart/no2/mvstore/NitriteMVStore.java +++ b/nitrite-mvstore-adapter/src/main/java/org/dizitart/no2/mvstore/NitriteMVStore.java @@ -17,7 +17,16 @@ package org.dizitart.no2.mvstore; -import lombok.extern.slf4j.Slf4j; +import static org.h2.mvstore.DataUtils.ERROR_BLOCK_NOT_FOUND; +import static org.h2.mvstore.DataUtils.ERROR_CHUNK_NOT_FOUND; +import static org.h2.mvstore.DataUtils.ERROR_FILE_CORRUPT; +import static org.h2.mvstore.DataUtils.ERROR_READING_FAILED; +import static org.h2.mvstore.DataUtils.ERROR_SERIALIZATION; +import static org.h2.mvstore.DataUtils.ERROR_WRITING_FAILED; + +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; + import org.dizitart.no2.common.util.StringUtils; import org.dizitart.no2.exceptions.NitriteIOException; import org.dizitart.no2.index.BoundingBox; @@ -31,17 +40,18 @@ import org.h2.mvstore.MVStoreException; import org.h2.mvstore.rtree.MVRTreeMap; -import java.util.Map; -import java.util.concurrent.ConcurrentHashMap; - -import static org.h2.mvstore.DataUtils.*; +import lombok.extern.slf4j.Slf4j; /** - * @since 1.0 * @author Anindya Chatterjee + * @since 1.0 */ @Slf4j public class NitriteMVStore extends AbstractNitriteStore { + + private static final String COMPACT_THREADS_PROPERTY = "h2.compactThreads"; + private static final Object COMPACT_THREADS_LOCK = new Object(); + private MVStore mvStore; private final Map> nitriteMapRegistry; private final Map> nitriteRTreeMapRegistry; @@ -121,7 +131,7 @@ public void close() { nitriteRTreeMapRegistry.clear(); if (getStoreConfig().autoCompact()) { - mvStore.close(-1); + compactAndClose(); } else { mvStore.close(); } @@ -196,6 +206,47 @@ public String getStoreVersion() { return "MVStore/" + org.h2.engine.Constants.VERSION; } + /** + * Closes the store with a full compaction, single-threaded. + * + *

MVStore reads h2.compactThreads on each compacting close, defaulting to a + * quarter of the available processors, and parallel compaction races on page references in + * 2.4.240 - see h2#4286. + * Pinning the property to 1 for the duration of the close is the upstream workaround; the + * lock is what keeps two concurrent closes from restoring each other's value. Both can go + * once the fix ships. + * + *

The lock is deliberately private rather than the Properties instance + * itself: Properties is a Hashtable, so holding its monitor across + * a compaction would stall every System.getProperty call in the JVM for as long + * as the compaction runs. + */ + private void compactAndClose() { + synchronized (COMPACT_THREADS_LOCK) { + final String originalCompactThreads = System.getProperty(COMPACT_THREADS_PROPERTY); + try { + System.setProperty(COMPACT_THREADS_PROPERTY, "1"); + mvStore.close(-1); + } finally { + if (originalCompactThreads == null) { + System.clearProperty(COMPACT_THREADS_PROPERTY); + } else { + System.setProperty(COMPACT_THREADS_PROPERTY, originalCompactThreads); + } + } + } + } + + /** + * The underlying MVStore, for tests that need to assert on store internals such as the + * chunk fill rate. + * + * @return the backing {@link MVStore}, or {@code null} before {@link #openOrCreate()} + */ + MVStore getMvStore() { + return mvStore; + } + private void initEventBus() { if (getStoreConfig().eventListeners() != null) { for (StoreEventListener eventListener : getStoreConfig().eventListeners()) { diff --git a/nitrite-mvstore-adapter/src/main/java/org/dizitart/no2/mvstore/VersionUsage.java b/nitrite-mvstore-adapter/src/main/java/org/dizitart/no2/mvstore/VersionUsage.java new file mode 100644 index 00000000..f8537be0 --- /dev/null +++ b/nitrite-mvstore-adapter/src/main/java/org/dizitart/no2/mvstore/VersionUsage.java @@ -0,0 +1,52 @@ +/* + * Copyright (c) 2019-2020. Nitrite author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.dizitart.no2.mvstore; + +import java.lang.ref.Cleaner; +import java.util.Set; +import java.util.concurrent.atomic.AtomicBoolean; + +import org.h2.mvstore.MVStore; + +import lombok.RequiredArgsConstructor; + +@RequiredArgsConstructor +class VersionUsage { + + /** + * Shared by every iterator and cursor in the adapter - one daemon thread is enough to release + * the versions of iterators that were abandoned rather than drained. + */ + static final Cleaner CLEANER = Cleaner.create(); + + private final AtomicBoolean released = new AtomicBoolean(false); + + private final MVStore mvStore; + private final MVStore.TxCounter txCounter; + private final Set versionUsages; + + boolean isReleased() { + return released.get(); + } + + void release() { + if (released.compareAndSet(false, true)) { + versionUsages.remove(this); + mvStore.deregisterVersionUsage(txCounter); + } + } +} diff --git a/nitrite-mvstore-adapter/src/test/java/org/dizitart/no2/integration/NitriteBuilderTest.java b/nitrite-mvstore-adapter/src/test/java/org/dizitart/no2/integration/NitriteBuilderTest.java index f49a6088..72e0b880 100644 --- a/nitrite-mvstore-adapter/src/test/java/org/dizitart/no2/integration/NitriteBuilderTest.java +++ b/nitrite-mvstore-adapter/src/test/java/org/dizitart/no2/integration/NitriteBuilderTest.java @@ -17,6 +17,26 @@ package org.dizitart.no2.integration; +import static org.dizitart.no2.collection.Document.createDocument; +import static org.dizitart.no2.common.module.NitriteModule.module; +import static org.dizitart.no2.common.util.StringUtils.isNullOrEmpty; +import static org.dizitart.no2.integration.TestUtil.createDb; +import static org.dizitart.no2.integration.TestUtil.getRandomTempDbFile; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; + +import java.io.BufferedWriter; +import java.io.File; +import java.io.FileWriter; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Paths; +import java.util.LinkedHashSet; +import java.util.Random; + import org.dizitart.no2.Nitrite; import org.dizitart.no2.NitriteBuilder; import org.dizitart.no2.NitriteConfig; @@ -45,22 +65,6 @@ import org.junit.Rule; import org.junit.Test; -import java.io.BufferedWriter; -import java.io.File; -import java.io.FileWriter; -import java.io.IOException; -import java.nio.file.Files; -import java.nio.file.Paths; -import java.util.LinkedHashSet; -import java.util.Random; - -import static org.dizitart.no2.collection.Document.createDocument; -import static org.dizitart.no2.common.module.NitriteModule.module; -import static org.dizitart.no2.common.util.StringUtils.isNullOrEmpty; -import static org.dizitart.no2.integration.TestUtil.createDb; -import static org.dizitart.no2.integration.TestUtil.getRandomTempDbFile; -import static org.junit.Assert.*; - /** * @author Anindya Chatterjee. */ @@ -91,7 +95,7 @@ public void cleanup() { TestUtil.deleteDb(filePath); } - if (fakeDb != null && !fakeDb.isClosed()){ + if (fakeDb != null && !fakeDb.isClosed()) { fakeDb.close(); } @@ -119,7 +123,7 @@ public void testConfig() throws IOException { assertEquals(storeConfig.autoCommitBufferSize(), 1); assertEquals(config.findIndexer("Custom").getClass(), CustomIndexer.class); assertFalse(storeConfig.autoCommit()); - assertFalse(storeConfig.autoCompact()); + assertTrue(storeConfig.autoCompact()); assertTrue(storeConfig.compress()); assertFalse(storeConfig.isReadOnly()); assertFalse(storeConfig.isInMemory()); @@ -169,8 +173,8 @@ public void testConfigWithFile() { public void testConfigWithFileNull() { File file = null; MVStoreModule module = MVStoreModule.withConfig() - .filePath(file) - .build(); + .filePath(file) + .build(); db = Nitrite.builder().loadModule(module).openOrCreate(); StoreConfig storeConfig = db.getStore().getStoreConfig(); diff --git a/nitrite-mvstore-adapter/src/test/java/org/dizitart/no2/integration/collection/CollectionSortedFindCostTest.java b/nitrite-mvstore-adapter/src/test/java/org/dizitart/no2/integration/collection/CollectionSortedFindCostTest.java index 1befebbd..6bade76e 100644 --- a/nitrite-mvstore-adapter/src/test/java/org/dizitart/no2/integration/collection/CollectionSortedFindCostTest.java +++ b/nitrite-mvstore-adapter/src/test/java/org/dizitart/no2/integration/collection/CollectionSortedFindCostTest.java @@ -39,7 +39,6 @@ import static org.dizitart.no2.integration.TestUtil.getRandomTempDbFile; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertTrue; /** * The cost half of {@link CollectionSortedFindTest}, which needs a store that actually @@ -51,13 +50,13 @@ * removes the decode: over 2000 rows carrying a 150-element array the sorted page went from * ~115 ms to ~2 ms. *

- * Both halves of every comparison here are the same collection shape sorted two different - * ways, never a fat collection against a lean one. That earlier framing assumed the index - * walk cancelled between the halves, and it does not: the fat collection's index lives in a - * file two orders of magnitude larger, so walking it costs more I/O for reasons that have - * nothing to do with decoding. Measured cold on macOS, a fat sorted page cost 4.6x a lean one - * with the optimisation present and working - through a 3x threshold, which is why this - * guard failed on that runner and nowhere else. + * The guard here has no clock in it. Two wall-clock versions of it were tried and both + * failed on CI while the optimisation was present and working - the first comparing fat rows + * against lean ones on macOS, the second comparing an indexed sort against an unindexed one on + * Ubuntu, where the indexed half measured slower than its control. A shared runner does + * not hold a millisecond ratio still, and no threshold rescues a measurement that inverts. The + * decision the optimisation actually makes is recorded in the {@link FindPlan}, so that is what + * is asserted. * * @author Anindya Chatterjee */ @@ -65,15 +64,6 @@ public class CollectionSortedFindCostTest { private static final int ROWS = 2000; private static final int PAYLOAD = 150; - /** - * Fresh collections per sample, because the cost under test is only visible on a cold one. - * Re-querying a collection that was just written measures a cache: over these same 2000 fat - * rows a blocking sort cost 353 ms, then 83 ms, then 0.8 ms. The old guard warmed once and - * timed the three runs after it, so it was comparing two numbers from which the decode - - * the whole subject of the test - had already been cached away. - */ - private static final int SAMPLES = 3; - private final String fileName = getRandomTempDbFile(); private Nitrite db; private int collectionSeq; @@ -121,65 +111,6 @@ public void testSortedPageIsPlannedFromTheIndex() { blocking.getFindPlan().getSortIndexDescriptor()); } - /** - * The same query, over the same rows, ordered by an indexed field and by an unindexed one. - * Only the second can be answered from an index; the first is what this optimisation - * exists to make cheap. Everything else - row count, document size, the file, the single - * row returned - is identical between the halves, so what is left is the decode of the - * {@link #ROWS} rows neither query should have returned. - *

- * Cold, and the best of {@link #SAMPLES}. Each half is measured once on a collection - * no query has touched, because a second look at the same collection measures a cache - * rather than a decode. That makes any single sample noisy, so each half is sampled on - * several fresh collections and the quickest is kept: contention can only ever add time, so - * the quickest run is the one least contaminated by the machine. - *

- * The threshold is 2x against a measured 4.5x on the worst run of a laptop under load and - * 45x on the best. A regression does not narrow this ratio - it removes it, because the two - * halves become the same code path and the ratio becomes 1. - */ - @Test - public void testSortedPageDoesNotDecodeTheRowsItDiscards() { - warmTheQueryPath(); - - double indexed = Double.MAX_VALUE; - double blocking = Double.MAX_VALUE; - for (int sample = 0; sample < SAMPLES; sample++) { - // Alternated so that any drift over the run lands on both halves alike. - blocking = Math.min(blocking, coldSortedPageCost("unindexed")); - indexed = Math.min(indexed, coldSortedPageCost("seq")); - } - - assertTrue("a sorted page over an indexed field took " + indexed + "ms against " - + blocking + "ms over an unindexed one, same rows and same documents - it is " - + "still decoding the rows it discards", blocking > indexed * 2); - } - - /** - * JIT only. Run on its own collection so the measured ones stay cache-cold. - */ - private void warmTheQueryPath() { - NitriteCollection collection = fatCollection("warm"); - for (int i = 0; i < 3; i++) { - drain(collection, "seq"); - drain(collection, "unindexed"); - } - } - - private double coldSortedPageCost(String sortField) { - NitriteCollection collection = fatCollection("cost"); - long start = System.nanoTime(); - drain(collection, sortField); - return (System.nanoTime() - start) / 1e6; - } - - private static void drain(NitriteCollection collection, String sortField) { - FindOptions page = FindOptions.orderBy(sortField, SortOrder.Descending).limit(1); - for (Document ignored : collection.find(ALL, page)) { - // force the fetch - } - } - /** * {@link #ROWS} rows carrying a {@link #PAYLOAD}-element array, except the one the page * returns. Descending on either sort field returns an end of the range, and giving that row diff --git a/nitrite-mvstore-adapter/src/test/java/org/dizitart/no2/mvstore/MVStoreFileGrowthTest.java b/nitrite-mvstore-adapter/src/test/java/org/dizitart/no2/mvstore/MVStoreFileGrowthTest.java new file mode 100644 index 00000000..aa4bbbca --- /dev/null +++ b/nitrite-mvstore-adapter/src/test/java/org/dizitart/no2/mvstore/MVStoreFileGrowthTest.java @@ -0,0 +1,115 @@ +/* + * Copyright (c) 2017-2021 Nitrite author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.dizitart.no2.mvstore; + +import static org.dizitart.no2.collection.Document.createDocument; +import static org.dizitart.no2.index.IndexOptions.indexOptions; +import static org.dizitart.no2.index.IndexType.NON_UNIQUE; +import static org.dizitart.no2.integration.TestUtil.createDb; +import static org.dizitart.no2.integration.TestUtil.deleteDb; +import static org.dizitart.no2.integration.TestUtil.getRandomTempDbFile; +import static org.junit.Assert.assertTrue; + +import java.io.File; +import java.time.Instant; + +import org.dizitart.no2.Nitrite; +import org.dizitart.no2.collection.NitriteCollection; +import org.junit.Test; + +/** + * A store that is written to far more often than it grows must not grow anyway. + * + *

Repeatedly updating the same handful of documents leaves each chunk holding one live page and + * a great many obsolete ones. Nothing reclaims those chunks unless MVStore is allowed to compact, + * so the file climbs without bound while the live data stays a few kilobytes - the report in + * gh-1284 reached ~800MB around + * 100 live documents. + * + * @author Anindya Chatterjee + */ +public class MVStoreFileGrowthTest { + + private static final int LIVE_DOCUMENTS = 25; + private static final int UPDATES_PER_ROUND = 2_000; + + /** + * The fill rate the reclaimed store settles at, against the ~14-18% an unreclaimed one decays + * to over the same rounds. It is an equilibrium rather than a target: compaction runs on the + * back of write activity, so the rate stops falling but does not climb back once idle. + */ + private static final int MIN_CHUNK_FILL_RATE = 25; + + @Test(timeout = 120_000) + public void testRepeatedUpdatesDoNotStrandObsoleteChunks() { + final String dbPath = getRandomTempDbFile(); + final File dbFile = new File(dbPath); + final long initialFileSize; + final int chunkFillRate; + + try (final Nitrite db = createDb(dbPath)) { + final NitriteCollection collection = db.getCollection("file-growth"); + collection.createIndex("key"); + collection.createIndex(indexOptions(NON_UNIQUE), "revision"); + + for (int i = 0; i < LIVE_DOCUMENTS; i++) { + collection.insert(createDocument("key", i) + .put("revision", 0) + .put("lastUpdated", Instant.now().toString())); + } + db.commit(); + initialFileSize = dbFile.length(); + + // Two rounds, because the first is what fills the chunks and the second is what shows + // whether anything is reclaiming them. + updateEveryDocument(collection); + db.commit(); + updateEveryDocument(collection); + db.commit(); + + chunkFillRate = ((NitriteMVStore) db.getStore()).getMvStore() + .getFileStore().getChunksFillRate(); + } + + // close(-1) compacts synchronously, so this one is not subject to the housekeeping thread + // getting scheduled - the file is back to its live size by the time close() returns. + final long finalFileSize = dbFile.length(); + + try { + assertTrue(String.format( + "chunks are only %d%% live after %d updates - obsolete chunks are not being reclaimed", + chunkFillRate, 2 * UPDATES_PER_ROUND * LIVE_DOCUMENTS), + chunkFillRate >= MIN_CHUNK_FILL_RATE); + + assertTrue(String.format( + "file is %d bytes after close against %d bytes of the same live data - it was not compacted", + finalFileSize, initialFileSize), + finalFileSize <= initialFileSize); + } finally { + deleteDb(dbPath); + } + } + + private void updateEveryDocument(final NitriteCollection collection) { + for (int i = 0; i < UPDATES_PER_ROUND; i++) { + collection.find().forEach(document -> { + document.put("lastUpdated", Instant.now().toString()); + collection.update(document); + }); + } + } +} diff --git a/nitrite-mvstore-adapter/src/test/java/org/dizitart/no2/mvstore/NitriteMVMapTest.java b/nitrite-mvstore-adapter/src/test/java/org/dizitart/no2/mvstore/NitriteMVMapTest.java index 9ee2d3f1..931811d5 100644 --- a/nitrite-mvstore-adapter/src/test/java/org/dizitart/no2/mvstore/NitriteMVMapTest.java +++ b/nitrite-mvstore-adapter/src/test/java/org/dizitart/no2/mvstore/NitriteMVMapTest.java @@ -17,49 +17,137 @@ package org.dizitart.no2.mvstore; -import org.h2.mvstore.MVMap; -import org.junit.Test; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertThrows; +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.inOrder; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; +import java.util.Arrays; +import java.util.Collections; import java.util.HashSet; +import java.util.Iterator; +import java.util.NoSuchElementException; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; -import static org.mockito.Mockito.*; +import org.dizitart.no2.exceptions.NitriteIOException; +import org.dizitart.no2.store.NitriteStore; +import org.h2.mvstore.MVMap; +import org.h2.mvstore.MVStore; +import org.junit.Before; +import org.junit.Test; +import org.mockito.InOrder; -@SuppressWarnings("unchecked") public class NitriteMVMapTest { + private MVMap mvMap; + private MVStore mvStore; + private MVStore.TxCounter txCounter; + private NitriteMVMap nitriteMVMap; + + @Before + public void setUp() { + //noinspection unchecked + mvMap = (MVMap) mock(MVMap.class); + mvStore = mock(MVStore.class); + txCounter = mock(MVStore.TxCounter.class); + NitriteStore nitriteStore = mock(NitriteStore.class); + when(mvMap.getStore()).thenReturn(mvStore); + when(mvStore.registerVersionUsage()).thenReturn(txCounter); + nitriteMVMap = new NitriteMVMap<>(mvMap, nitriteStore); + } + @Test public void testValues() { - NitriteMVMap nitriteMVMap = new NitriteMVMap<>( - (MVMap) mock(MVMap.class), null); + when(mvMap.values()).thenReturn(Collections.emptyList()); assertTrue(nitriteMVMap.values().toList().isEmpty()); + + InOrder inOrder = inOrder(mvStore, mvMap); + inOrder.verify(mvStore).registerVersionUsage(); + //noinspection ResultOfMethodCallIgnored + inOrder.verify(mvMap).values(); + inOrder.verify(mvStore).deregisterVersionUsage(txCounter); assertFalse(nitriteMVMap.isEmpty()); } @Test public void testKeys() { - MVMap objectObjectMap = (MVMap) mock(MVMap.class); - when(objectObjectMap.keySet()).thenReturn(new HashSet<>()); - NitriteMVMap nitriteMVMap = new NitriteMVMap<>(objectObjectMap, null); + when(mvMap.keySet()).thenReturn(new HashSet<>()); assertTrue(nitriteMVMap.keys().toList().isEmpty()); - verify(objectObjectMap).keySet(); + + verify(mvMap).keySet(); + verify(mvStore).deregisterVersionUsage(txCounter); assertFalse(nitriteMVMap.isEmpty()); } + @Test + public void testEntries() { + when(mvMap.entrySet()).thenReturn(Collections.singletonMap("key", "value").entrySet()); + assertEquals(1, nitriteMVMap.entries().toList().size()); + + verify(mvStore).registerVersionUsage(); + verify(mvStore).deregisterVersionUsage(txCounter); + } + + @Test + public void testReversedEntries() { + when(mvMap.getVersion()).thenReturn(1L); + when(mvMap.openVersion(1L)).thenReturn(mvMap); + when(mvMap.lastKey()).thenReturn(1); + when(mvMap.floorKey(1)).thenReturn(1); + + assertEquals(1, nitriteMVMap.reversedEntries().toList().size()); + verify(mvStore).registerVersionUsage(); + verify(mvStore).deregisterVersionUsage(txCounter); + } + + @Test + public void testIteratorCreationFailureReleasesVersion() { + when(mvMap.values()).thenThrow(new IllegalStateException()); + + assertThrows(IllegalStateException.class, () -> nitriteMVMap.values().iterator()); + verify(mvStore).registerVersionUsage(); + verify(mvStore).deregisterVersionUsage(txCounter); + } + + @Test + public void testExhaustedIteratorRemainsExhausted() { + when(mvMap.values()).thenReturn(Collections.singletonList("value")); + Iterator iterator = nitriteMVMap.values().iterator(); + + assertTrue(iterator.hasNext()); + assertEquals("value", iterator.next()); + assertFalse(iterator.hasNext()); + assertFalse(iterator.hasNext()); + assertThrows(NoSuchElementException.class, iterator::next); + } + + @Test + public void testAbandonedIteratorReleasesVersionOnClose() { + when(mvMap.values()).thenReturn(Arrays.asList("first", "second")); + Iterator iterator = nitriteMVMap.values().iterator(); + + assertEquals("first", iterator.next()); + verify(mvStore, never()).deregisterVersionUsage(txCounter); + + nitriteMVMap.close(); + verify(mvStore).deregisterVersionUsage(txCounter); + assertThrows(NitriteIOException.class, iterator::hasNext); + } + @Test(expected = NullPointerException.class) public void testConstructor() { - NitriteMVMap actualNitriteMVMap = new NitriteMVMap<>( - (MVMap) mock(MVMap.class), null); + NitriteMVMap actualNitriteMVMap = new NitriteMVMap<>(mvMap, null); actualNitriteMVMap.close(); assertFalse(actualNitriteMVMap.isEmpty()); } @Test public void testIsEmpty() { - MVMap objectObjectMap = (MVMap) mock(MVMap.class); - when(objectObjectMap.isEmpty()).thenReturn(true); - assertTrue((new NitriteMVMap<>(objectObjectMap, null)).isEmpty()); - verify(objectObjectMap).isEmpty(); + when(mvMap.isEmpty()).thenReturn(true); + assertTrue(nitriteMVMap.isEmpty()); + verify(mvMap).isEmpty(); } } - diff --git a/nitrite-mvstore-adapter/src/test/java/org/dizitart/no2/mvstore/NitriteMVRTreeMapTest.java b/nitrite-mvstore-adapter/src/test/java/org/dizitart/no2/mvstore/NitriteMVRTreeMapTest.java new file mode 100644 index 00000000..7a392e1b --- /dev/null +++ b/nitrite-mvstore-adapter/src/test/java/org/dizitart/no2/mvstore/NitriteMVRTreeMapTest.java @@ -0,0 +1,115 @@ +/* + * Copyright (c) 2017-2021 Nitrite author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +package org.dizitart.no2.mvstore; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertThrows; +import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.clearInvocations; +import static org.mockito.Mockito.inOrder; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.verify; + +import java.util.Iterator; + +import org.dizitart.no2.collection.NitriteId; +import org.dizitart.no2.common.RecordStream; +import org.dizitart.no2.exceptions.NitriteIOException; +import org.dizitart.no2.index.BoundingBox; +import org.dizitart.no2.store.NitriteStore; +import org.h2.mvstore.MVStore; +import org.h2.mvstore.rtree.MVRTreeMap; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.mockito.InOrder; + +public class NitriteMVRTreeMapTest { + private MVStore mvStore; + private MVRTreeMap mvMap; + private NitriteMVRTreeMap nitriteMVRTreeMap; + + @Before + public void setUp() { + mvStore = spy(new MVStore.Builder().open()); + mvMap = spy(mvStore.openMap("test", new MVRTreeMap.Builder<>())); + NitriteStore nitriteStore = mock(NitriteStore.class); + nitriteMVRTreeMap = new NitriteMVRTreeMap<>(mvMap, nitriteStore); + + nitriteMVRTreeMap.add(new BoundingBox(0, 1, 0, 1), NitriteId.createId("1")); + clearInvocations(mvStore, mvMap); + } + + @After + public void tearDown() { + mvStore.closeImmediately(); + } + + @Test + public void testIntersectingCursorRetainsVersionUntilExhausted() { + RecordStream recordStream = + nitriteMVRTreeMap.findIntersectingKeys(new BoundingBox(-1, 2, -1, 2)); + verify(mvMap, never()).findIntersectingKeys(any()); + + Iterator iterator = recordStream.iterator(); + InOrder inOrder = inOrder(mvStore, mvMap); + inOrder.verify(mvStore).registerVersionUsage(); + inOrder.verify(mvMap).findIntersectingKeys(any()); + assertVersionRetainedUntilExhausted(iterator); + } + + @Test + public void testContainedCursorRetainsVersionUntilExhausted() { + RecordStream recordStream = + nitriteMVRTreeMap.findContainedKeys(new BoundingBox(-1, 2, -1, 2)); + verify(mvMap, never()).findContainedKeys(any()); + + Iterator iterator = recordStream.iterator(); + InOrder inOrder = inOrder(mvStore, mvMap); + inOrder.verify(mvStore).registerVersionUsage(); + inOrder.verify(mvMap).findContainedKeys(any()); + assertVersionRetainedUntilExhausted(iterator); + } + + @Test + public void testAbandonedCursorReleasesVersionOnClose() { + Iterator iterator = nitriteMVRTreeMap + .findIntersectingKeys(new BoundingBox(-1, 2, -1, 2)) + .iterator(); + + assertEquals(NitriteId.createId("1"), iterator.next()); + verify(mvStore, never()).deregisterVersionUsage(any()); + + nitriteMVRTreeMap.close(); + verify(mvStore).deregisterVersionUsage(any()); + assertThrows(NitriteIOException.class, iterator::hasNext); + } + + private void assertVersionRetainedUntilExhausted(Iterator iterator) { + assertTrue(iterator.hasNext()); + assertEquals(NitriteId.createId("1"), iterator.next()); + verify(mvStore, never()).deregisterVersionUsage(any()); + + assertFalse(iterator.hasNext()); + verify(mvStore).deregisterVersionUsage(any()); + } +} diff --git a/nitrite-mvstore-adapter/src/test/java/org/dizitart/no2/mvstore/NitriteMVStoreTest.java b/nitrite-mvstore-adapter/src/test/java/org/dizitart/no2/mvstore/NitriteMVStoreTest.java index 955748e2..bda0c1ff 100644 --- a/nitrite-mvstore-adapter/src/test/java/org/dizitart/no2/mvstore/NitriteMVStoreTest.java +++ b/nitrite-mvstore-adapter/src/test/java/org/dizitart/no2/mvstore/NitriteMVStoreTest.java @@ -17,14 +17,36 @@ package org.dizitart.no2.mvstore; -import org.junit.Test; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertThrows; +import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.mock; + +import java.lang.reflect.Field; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Iterator; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; -import static org.junit.Assert.*; +import org.dizitart.no2.exceptions.NitriteIOException; +import org.dizitart.no2.store.NitriteMap; +import org.h2.mvstore.MVStore; +import org.junit.Test; public class NitriteMVStoreTest { + @Test public void testConstructor() { - NitriteMVStore actualNitriteMVStore = new NitriteMVStore(); + final NitriteMVStore actualNitriteMVStore = new NitriteMVStore(); assertNull(actualNitriteMVStore.getStoreConfig()); assertTrue(actualNitriteMVStore.isClosed()); assertFalse(actualNitriteMVStore.hasUnsavedChanges()); @@ -33,7 +55,7 @@ public void testConstructor() { @Test public void testOpenOrCreate() { - NitriteMVStore nitriteMVStore = new NitriteMVStore(); + final NitriteMVStore nitriteMVStore = new NitriteMVStore(); nitriteMVStore.setStoreConfig(new MVStoreConfig()); nitriteMVStore.openOrCreate(); assertFalse(nitriteMVStore.isReadOnly()); @@ -55,5 +77,105 @@ public void testHasUnsavedChanges() { public void testGetStoreVersion() { assertNotNull((new NitriteMVStore()).getStoreVersion()); } -} + @Test + public void testIteratorCannotReadAfterStoreClose() throws Exception { + + final Path storeFile = Files.createTempFile("nitrite-lifecycle-", ".db"); + Files.delete(storeFile); + final NitriteMVStore nitriteMVStore = new NitriteMVStore(); + final MVStoreConfig config = new MVStoreConfig(); + config.filePath(storeFile.toString()); + config.autoCompact(true); + nitriteMVStore.setStoreConfig(config); + + try { + nitriteMVStore.openOrCreate(); + final NitriteMap map = nitriteMVStore.openMap("test", Integer.class, String.class); + for (int i = 0; i < 100; i++) { + map.put(i, "value-" + i); + } + + final Iterator iterator = map.values().iterator(); + assertTrue(iterator.hasNext()); + iterator.next(); + + nitriteMVStore.close(); + + final NitriteIOException exception = assertThrows(NitriteIOException.class, iterator::hasNext); + assertEquals("MVStore is closed", exception.getMessage()); + } finally { + if (!nitriteMVStore.isClosed()) { + nitriteMVStore.close(); + } + Files.deleteIfExists(storeFile); + } + } + + @Test + public void testCompactingClosesAreSerialized() throws Exception { + + final String originalCompactThreads = System.getProperty("h2.compactThreads"); + final CountDownLatch firstCloseStarted = new CountDownLatch(1); + final CountDownLatch releaseFirstClose = new CountDownLatch(1); + final CountDownLatch secondCloseAttempted = new CountDownLatch(1); + final CountDownLatch secondCloseStarted = new CountDownLatch(1); + final ExecutorService executorService = Executors.newFixedThreadPool(2); + + try { + System.setProperty("h2.compactThreads", "4"); + final MVStore firstMVStore = mock(MVStore.class); + doAnswer(invocation -> { + assertEquals("1", System.getProperty("h2.compactThreads")); + firstCloseStarted.countDown(); + assertTrue(releaseFirstClose.await(5, TimeUnit.SECONDS)); + return null; + }).when(firstMVStore).close(anyInt()); + + final MVStore secondMVStore = mock(MVStore.class); + doAnswer(invocation -> { + secondCloseStarted.countDown(); + assertEquals("1", System.getProperty("h2.compactThreads")); + return null; + }).when(secondMVStore).close(anyInt()); + + final NitriteMVStore firstStore = createCompactingStore(firstMVStore); + final NitriteMVStore secondStore = createCompactingStore(secondMVStore); + final Future firstClose = executorService.submit(firstStore::close); + assertTrue(firstCloseStarted.await(5, TimeUnit.SECONDS)); + + final Future secondClose = executorService.submit(() -> { + secondCloseAttempted.countDown(); + secondStore.close(); + }); + assertTrue(secondCloseAttempted.await(5, TimeUnit.SECONDS)); + assertFalse(secondCloseStarted.await(200, TimeUnit.MILLISECONDS)); + + releaseFirstClose.countDown(); + firstClose.get(5, TimeUnit.SECONDS); + secondClose.get(5, TimeUnit.SECONDS); + // the second close restores what the first one saved, not the "1" it saw in flight + assertEquals("4", System.getProperty("h2.compactThreads")); + } finally { + releaseFirstClose.countDown(); + executorService.shutdownNow(); + if (originalCompactThreads == null) { + System.clearProperty("h2.compactThreads"); + } else { + System.setProperty("h2.compactThreads", originalCompactThreads); + } + } + } + + private NitriteMVStore createCompactingStore(final MVStore mvStore) throws Exception { + final NitriteMVStore nitriteMVStore = new NitriteMVStore(); + final MVStoreConfig config = new MVStoreConfig(); + config.autoCompact(true); + nitriteMVStore.setStoreConfig(config); + + final Field mvStoreField = NitriteMVStore.class.getDeclaredField("mvStore"); + mvStoreField.setAccessible(true); + mvStoreField.set(nitriteMVStore, mvStore); + return nitriteMVStore; + } +}