From 18cab8db7b21e96a5ec23d053f4ef65ef378a873 Mon Sep 17 00:00:00 2001 From: fh-ms Date: Fri, 26 Jun 2026 11:07:49 +0200 Subject: [PATCH 1/2] Couple external-directory Lucene index commits to GigaMap.store() (#715) The LuceneIndex committed eagerly on every mutation. With the default embedded GraphDirectory that is harmless (the commit only touches the in-memory fileEntries map, persisted on store()), but with an external MMap(path) directory each mutation was flushed to disk immediately, decoupled from store(): a mutation not followed by a store (or a crash in between) left the on-disk index diverged from the persisted entities. Make the commit behavior selectable and store-aligned: - LuceneContext: add a public New(directoryCreator, analyzerCreator, documentPopulator, autoCommit) factory. Store the flag inverted as manualCommit so the false-default of an added field preserves the legacy autoCommit==true behavior when loading pre-existing stores. - LuceneIndex.Default.internalCommitOnStore(): flush+commit the writer when autoCommit is disabled and there are uncommitted changes. - BinaryHandlerLuceneIndexDefault: invoke internalCommitOnStore() before serializing, so the writer is flushed into fileEntries (graph) / to disk (external) exactly at the store() boundary. This also fixes a latent bug where a graph index with autoCommit=false persisted an empty/stale fileEntries map. Document the autoCommit x directory-type matrix and the remaining external-directory caveat (index and GigaMap storage are separate targets, so coupling aligns the commits temporally but is not a single atomic transaction). Add LuceneStoreBoundaryCommitTest covering MMap store-boundary commit (and no eager commit) plus GraphDirectory reload with autoCommit=false. --- .../BinaryHandlerLuceneIndexDefault.java | 18 ++ .../store/gigamap/lucene/LuceneContext.java | 104 +++++++++-- .../store/gigamap/lucene/LuceneIndex.java | 37 +++- .../lucene/LuceneStoreBoundaryCommitTest.java | 165 ++++++++++++++++++ 4 files changed, 313 insertions(+), 11 deletions(-) create mode 100644 gigamap/lucene/src/test/java/org/eclipse/store/gigamap/lucene/LuceneStoreBoundaryCommitTest.java diff --git a/gigamap/lucene/src/main/java/org/eclipse/store/gigamap/lucene/BinaryHandlerLuceneIndexDefault.java b/gigamap/lucene/src/main/java/org/eclipse/store/gigamap/lucene/BinaryHandlerLuceneIndexDefault.java index 5e17ffa00..8a821ce77 100644 --- a/gigamap/lucene/src/main/java/org/eclipse/store/gigamap/lucene/BinaryHandlerLuceneIndexDefault.java +++ b/gigamap/lucene/src/main/java/org/eclipse/store/gigamap/lucene/BinaryHandlerLuceneIndexDefault.java @@ -76,6 +76,24 @@ public static BinaryHandlerLuceneIndexDefault New() // methods // //////////// + @Override + public void store( + final Binary data , + final LuceneIndex.Default instance, + final long objectId, + final PersistenceStoreHandler handler + ) + { + // Couple the Lucene commit to the GigaMap store boundary (no-op unless autoCommit is + // disabled). Must run BEFORE super.store(): super.store calls internalStore (which stores + // the fileEntries reference) and then storeChildren (which stores the map contents). For a + // graph directory the writer may not have flushed yet, so committing here ensures + // fileEntries is populated before either reads it. + instance.internalCommitOnStore(); + + super.store(data, instance, objectId, handler); + } + @Override protected void internalStore( final Binary data , diff --git a/gigamap/lucene/src/main/java/org/eclipse/store/gigamap/lucene/LuceneContext.java b/gigamap/lucene/src/main/java/org/eclipse/store/gigamap/lucene/LuceneContext.java index 24615d022..06c1c93d3 100644 --- a/gigamap/lucene/src/main/java/org/eclipse/store/gigamap/lucene/LuceneContext.java +++ b/gigamap/lucene/src/main/java/org/eclipse/store/gigamap/lucene/LuceneContext.java @@ -79,11 +79,41 @@ public interface LuceneContext public DocumentPopulator documentPopulator(); /** - * Determines whether the operations in this context are automatically committed - * to the underlying storage or require explicit commits. + * Determines whether index mutations are committed eagerly, immediately at mutation + * time, or only when explicitly requested. + *

+ * The effect of this flag depends on the directory type (see {@link #directoryCreator()}): + * + * + * + * + * + * + * + * + * + * + * + * + * + *
{@code autoCommit} × directory type
{@code autoCommit == true} (default){@code autoCommit == false}
Embedded (directory creator is {@code null}; index data lives in the + * persisted object graph)Each mutation commits into the in-memory graph; the data reaches disk only on + * {@code GigaMap.store()}. Already consistent at store boundaries.The writer is flushed and committed once, exactly at the {@code GigaMap.store()} + * boundary, instead of after every mutation.
External (e.g. {@link DirectoryCreator#MMap(java.nio.file.Path)}; + * index data lives on disk, outside the GigaMap storage)Each mutation commits to disk immediately, decoupled from + * {@code GigaMap.store()}: a mutation not followed by a store (or a crash in + * between) leaves the on-disk index diverged from the persisted entities.The writer is flushed and committed at the {@code GigaMap.store()} boundary, + * giving store-aligned durability. A manual {@link LuceneIndex#commit()} is also + * honored.
+ *

+ * Note: an external index is a separate persistence target from the GigaMap storage. + * Coupling its commit to {@code store()} aligns the two commits temporally but is not a + * single atomic transaction across both targets; a crash mid-{@code store()} can still + * diverge. An embedded (graph) index has no such gap, as it is part of the persisted graph. * - * @return true if the context is set to automatically commit changes; - * false if manual commits are required. + * @return {@code true} (the default) to commit eagerly at mutation time; + * {@code false} to couple commits to {@code GigaMap.store()} / explicit + * {@link LuceneIndex#commit()}. */ public default boolean autoCommit() { @@ -178,32 +208,80 @@ public static LuceneContext New( final AnalyzerCreator analyzerCreator , final DocumentPopulator documentPopulator ) + { + return New( + directoryCreator , + notNull(analyzerCreator ), + notNull(documentPopulator), + true + ); + } + + /** + * Creates a new instance of {@link LuceneContext} for handling Lucene operations, with + * explicit control over the commit behavior. + * + * @param the type of entity to be indexed and searched with the resulting {@link LuceneContext} + * @param directoryCreator an implementation of {@link DirectoryCreator} responsible for + * creating a directory where the Lucene index will be stored, + * or null if the data should be stored inside the index + * @param analyzerCreator an implementation of {@link AnalyzerCreator} responsible for + * creating analyzers used for text processing in Lucene + * @param documentPopulator an implementation of {@link DocumentPopulator} responsible for + * mapping entity data into Lucene {@link Document} objects + * @param autoCommit see {@link LuceneContext#autoCommit()}; pass {@code false} to couple + * commits to {@code GigaMap.store()} / explicit {@link LuceneIndex#commit()} + * instead of committing eagerly at mutation time + * @return an instance of {@link LuceneContext} configured with the given parameters + */ + public static LuceneContext New( + final DirectoryCreator directoryCreator , + final AnalyzerCreator analyzerCreator , + final DocumentPopulator documentPopulator, + final boolean autoCommit + ) { return new Default<>( directoryCreator , notNull(analyzerCreator ), - notNull(documentPopulator) + notNull(documentPopulator), + autoCommit ); } - - - + + + public class Default implements LuceneContext { private final DirectoryCreator directoryCreator; private final AnalyzerCreator analyzerCreator; private final DocumentPopulator documentPopulator; - + // Inverted on purpose: a field added to this persisted type defaults to false when an + // older store (without the field) is loaded. Storing manualCommit (not autoCommit) makes + // that false-default mean autoCommit()==true, preserving the legacy behavior on reload. + private final boolean manualCommit; + Default( final DirectoryCreator directoryCreator , final AnalyzerCreator analyzerCreator , final DocumentPopulator documentPopulator ) + { + this(directoryCreator, analyzerCreator, documentPopulator, true); + } + + Default( + final DirectoryCreator directoryCreator , + final AnalyzerCreator analyzerCreator , + final DocumentPopulator documentPopulator, + final boolean autoCommit + ) { super(); this.directoryCreator = directoryCreator ; this.analyzerCreator = analyzerCreator ; this.documentPopulator = documentPopulator; + this.manualCommit = !autoCommit ; } @Override @@ -223,7 +301,13 @@ public DocumentPopulator documentPopulator() { return this.documentPopulator; } - + + @Override + public boolean autoCommit() + { + return !this.manualCommit; + } + } } diff --git a/gigamap/lucene/src/main/java/org/eclipse/store/gigamap/lucene/LuceneIndex.java b/gigamap/lucene/src/main/java/org/eclipse/store/gigamap/lucene/LuceneIndex.java index 76eaeb189..0e1744655 100644 --- a/gigamap/lucene/src/main/java/org/eclipse/store/gigamap/lucene/LuceneIndex.java +++ b/gigamap/lucene/src/main/java/org/eclipse/store/gigamap/lucene/LuceneIndex.java @@ -241,7 +241,10 @@ public default List query(final String queryText, final int maxResults) * finalizes recent additions, updates, or deletions of indexed entities. *

* Note: If {@link LuceneContext#autoCommit()} returns {@code true}, which is the default, - * this method doesn't need to be invoked explicitly. + * this method doesn't need to be invoked explicitly. When {@code autoCommit()} is + * {@code false}, pending changes are also committed automatically at each + * {@code GigaMap.store()} boundary, so an explicit call is only needed to commit between + * stores. */ public void commit(); @@ -835,6 +838,38 @@ private void optCommit() throws IOException this.markStateChangeChildren(); } + /** + * Couples the Lucene commit to the {@link GigaMap} store boundary when + * {@link LuceneContext#autoCommit()} is {@code false}: invoked by + * {@link BinaryHandlerLuceneIndexDefault#store} right before the index is serialized, + * it flushes and commits any pending writer changes so the persisted state reflects all + * mutations up to this {@code store()} exactly. For a graph directory this populates the + * {@link Default#fileEntries} map before it is read for serialization; for an external + * directory it syncs the on-disk index at the store boundary. + *

+ * No-op when {@code autoCommit()} is {@code true} (the eager commits already ran), when + * the writer has not been initialized yet, or when there are no uncommitted changes. + */ + void internalCommitOnStore() + { + synchronized(this.gigaMap) + { + if(!this.context.autoCommit() + && this.writer != null + && this.writer.hasUncommittedChanges()) + { + try + { + this.internalCommit(); + } + catch(final IOException e) + { + throw new IORuntimeException(e); + } + } + } + } + private void internalCommit() throws IOException { this.writer.flush(); diff --git a/gigamap/lucene/src/test/java/org/eclipse/store/gigamap/lucene/LuceneStoreBoundaryCommitTest.java b/gigamap/lucene/src/test/java/org/eclipse/store/gigamap/lucene/LuceneStoreBoundaryCommitTest.java new file mode 100644 index 000000000..67b893fc4 --- /dev/null +++ b/gigamap/lucene/src/test/java/org/eclipse/store/gigamap/lucene/LuceneStoreBoundaryCommitTest.java @@ -0,0 +1,165 @@ +package org.eclipse.store.gigamap.lucene; + +/*- + * #%L + * EclipseStore GigaMap Lucene + * %% + * Copyright (C) 2023 - 2026 MicroStream Software + * %% + * This program and the accompanying materials are made + * available under the terms of the Eclipse Public License 2.0 + * which is available at https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + * #L% + */ + +import org.apache.lucene.document.Document; +import org.apache.lucene.index.DirectoryReader; +import org.apache.lucene.store.Directory; +import org.apache.lucene.store.MMapDirectory; +import org.eclipse.store.gigamap.types.GigaMap; +import org.eclipse.store.storage.embedded.types.EmbeddedStorage; +import org.eclipse.store.storage.embedded.types.EmbeddedStorageManager; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import java.nio.file.Path; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +/** + * Verifies that with {@link LuceneContext#autoCommit()} == {@code false} the Lucene commit is + * coupled to the {@link GigaMap#store()} boundary instead of happening eagerly at mutation time + * (issue #715). + */ +public class LuceneStoreBoundaryCommitTest +{ + @TempDir + Path lucenePath; + + @TempDir + Path storagePath; + + // ── entity ──────────────────────────────────────────────────────────────── + + private static class Article + { + final String title; + final String content; + + Article(final String title, final String content) + { + this.title = title; + this.content = content; + } + } + + private static class ArticlePopulator extends DocumentPopulator

+ { + @Override + public void populate(final Document document, final Article entity) + { + document.add(createTextField("title", entity.title)); + document.add(createTextField("content", entity.content)); + } + } + + private static long committedDocsOnDisk(final Path path) throws Exception + { + // read-only reader over the last on-disk commit; does not need the writer lock and is + // independent of the live near-real-time reader, so it reflects exactly what store() + // (or an explicit commit()) flushed to disk. + try(final Directory dir = new MMapDirectory(path); + final DirectoryReader reader = DirectoryReader.open(dir)) + { + return reader.numDocs(); + } + } + + + // ── external (MMap) directory ─────────────────────────────────────────────── + + @Test + void mmapCommitIsCoupledToStore() throws Exception + { + final LuceneContext
ctx = LuceneContext.New( + DirectoryCreator.MMap(this.lucenePath), + AnalyzerCreator.Standard(), + new ArticlePopulator(), + false // manual / store-coupled commit + ); + + final GigaMap
map = GigaMap.New(); + final LuceneIndex
idx = map.index().register(LuceneIndex.Category(ctx)); + + try(final EmbeddedStorageManager sm = EmbeddedStorage.start(map, this.storagePath)) + { + map.add(new Article("eclipse", "first")); + + // near-real-time reader sees the change, but nothing is committed to disk yet + assertEquals(1, idx.query("title:eclipse").size(), + "NRT reader must see the change before any commit"); + + map.store(); + assertEquals(1, committedDocsOnDisk(this.lucenePath), + "store() must commit pending writer changes to the external directory"); + + // a further mutation without a store must NOT reach disk (no eager commit) + map.add(new Article("store", "second")); + assertEquals(2, idx.query("title:eclipse OR title:store").size(), + "NRT reader must see the second change immediately"); + assertEquals(1, committedDocsOnDisk(this.lucenePath), + "a mutation not followed by store() must not be committed to disk"); + + map.store(); + assertEquals(2, committedDocsOnDisk(this.lucenePath), + "the next store() must commit the second change to disk"); + } + finally + { + idx.close(); + } + } + + + // ── embedded (graph) directory ────────────────────────────────────────────── + + @Test + void graphDirectorySurvivesStoreWithManualCommit() + { + // null directoryCreator → GraphDirectory: index data lives in the persisted fileEntries + // map. With autoCommit=false the writer must be flushed into that map at store() time, + // otherwise store() would persist an empty/stale index (the bug fixed by #715). + final GigaMap
map = GigaMap.New(); + final LuceneIndex
idx = map.index().register(LuceneIndex.Category( + LuceneContext.New(null, AnalyzerCreator.Standard(), new ArticlePopulator(), false))); + + try(final EmbeddedStorageManager sm = EmbeddedStorage.start(map, this.storagePath)) + { + map.add(new Article("eclipse", "graph directory")); + map.add(new Article("store", "boundary")); + map.store(); + } + idx.close(); + + LuceneIndex
idx2 = null; + try(final EmbeddedStorageManager sm = EmbeddedStorage.start(this.storagePath)) + { + final GigaMap
reloaded = sm.root(); + idx2 = reloaded.index().get(LuceneIndex.class); + + assertEquals(1, idx2.query("title:eclipse").size(), + "graph index data must survive store() with autoCommit=false"); + assertEquals(2, reloaded.size(), + "reloaded GigaMap must contain both entities"); + } + finally + { + if(idx2 != null) + { + idx2.close(); + } + } + } +} From 72d5240e7e3762e9a85ec3b7b6c5905318edc521 Mon Sep 17 00:00:00 2001 From: fh-ms Date: Fri, 26 Jun 2026 11:25:13 +0200 Subject: [PATCH 2/2] Close index via try-with-resources in graph-directory test --- .../store/gigamap/lucene/LuceneStoreBoundaryCommitTest.java | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/gigamap/lucene/src/test/java/org/eclipse/store/gigamap/lucene/LuceneStoreBoundaryCommitTest.java b/gigamap/lucene/src/test/java/org/eclipse/store/gigamap/lucene/LuceneStoreBoundaryCommitTest.java index 67b893fc4..230227ad2 100644 --- a/gigamap/lucene/src/test/java/org/eclipse/store/gigamap/lucene/LuceneStoreBoundaryCommitTest.java +++ b/gigamap/lucene/src/test/java/org/eclipse/store/gigamap/lucene/LuceneStoreBoundaryCommitTest.java @@ -132,16 +132,14 @@ void graphDirectorySurvivesStoreWithManualCommit() // map. With autoCommit=false the writer must be flushed into that map at store() time, // otherwise store() would persist an empty/stale index (the bug fixed by #715). final GigaMap
map = GigaMap.New(); - final LuceneIndex
idx = map.index().register(LuceneIndex.Category( + try(final LuceneIndex
idx = map.index().register(LuceneIndex.Category( LuceneContext.New(null, AnalyzerCreator.Standard(), new ArticlePopulator(), false))); - - try(final EmbeddedStorageManager sm = EmbeddedStorage.start(map, this.storagePath)) + final EmbeddedStorageManager sm = EmbeddedStorage.start(map, this.storagePath)) { map.add(new Article("eclipse", "graph directory")); map.add(new Article("store", "boundary")); map.store(); } - idx.close(); LuceneIndex
idx2 = null; try(final EmbeddedStorageManager sm = EmbeddedStorage.start(this.storagePath))