Couple external-directory Lucene index commits to GigaMap.store() (#715)

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<Binary> 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    ,

gigamap/lucene/src/main/java/org/eclipse/store/gigamap/lucene/LuceneContext.java

@@ -79,11 +79,41 @@ public interface LuceneContext<E>
 	public DocumentPopulator<E> 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.
+     * <p>
+     * The effect of this flag depends on the directory type (see {@link #directoryCreator()}):
+     * <table border="1">
+     *   <caption>{@code autoCommit} &times; directory type</caption>
+     *   <tr><th></th><th>{@code autoCommit == true} (default)</th><th>{@code autoCommit == false}</th></tr>
+     *   <tr>
+     *     <td><b>Embedded</b> (directory creator is {@code null}; index data lives in the
+     *         persisted object graph)</td>
+     *     <td>Each mutation commits into the in-memory graph; the data reaches disk only on
+     *         {@code GigaMap.store()}. Already consistent at store boundaries.</td>
+     *     <td>The writer is flushed and committed once, exactly at the {@code GigaMap.store()}
+     *         boundary, instead of after every mutation.</td>
+     *   </tr>
+     *   <tr>
+     *     <td><b>External</b> (e.g. {@link DirectoryCreator#MMap(java.nio.file.Path)};
+     *         index data lives on disk, outside the GigaMap storage)</td>
+     *     <td>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.</td>
+     *     <td>The writer is flushed and committed at the {@code GigaMap.store()} boundary,
+     *         giving store-aligned durability. A manual {@link LuceneIndex#commit()} is also
+     *         honored.</td>
+     *   </tr>
+     * </table>
+     * <p>
+     * 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 <E> LuceneContext<E> New(
 		final AnalyzerCreator      analyzerCreator  ,
 		final DocumentPopulator<E> 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 <E> 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 <code>null</code> 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 <E> LuceneContext<E> New(
+		final DirectoryCreator     directoryCreator ,
+		final AnalyzerCreator      analyzerCreator  ,
+		final DocumentPopulator<E> documentPopulator,
+		final boolean              autoCommit
+	)
 	{
 		return new Default<>(
 			        directoryCreator  ,
 			notNull(analyzerCreator  ),
-			notNull(documentPopulator)
+			notNull(documentPopulator),
+			autoCommit
 		);
 	}
-	
-	
-	
+
+
+
 	public class Default<E> implements LuceneContext<E>
 	{
 		private final DirectoryCreator     directoryCreator;
 		private final AnalyzerCreator      analyzerCreator;
 		private final DocumentPopulator<E> 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<E> documentPopulator
 		)
+		{
+			this(directoryCreator, analyzerCreator, documentPopulator, true);
+		}
+
+		Default(
+			final DirectoryCreator     directoryCreator ,
+			final AnalyzerCreator      analyzerCreator  ,
+			final DocumentPopulator<E> documentPopulator,
+			final boolean              autoCommit
+		)
 		{
 			super();
 			this.directoryCreator  = directoryCreator ;
 			this.analyzerCreator   = analyzerCreator  ;
 			this.documentPopulator = documentPopulator;
+			this.manualCommit      = !autoCommit      ;
 		}
 
 		@Override
@@ -223,7 +301,13 @@ public DocumentPopulator<E> documentPopulator()
 		{
 			return this.documentPopulator;
 		}
-
+
+		@Override
+		public boolean autoCommit()
+		{
+			return !this.manualCommit;
+		}
+
 	}
 
 }

gigamap/lucene/src/main/java/org/eclipse/store/gigamap/lucene/LuceneIndex.java

@@ -241,7 +241,10 @@ public default List<E> query(final String queryText, final int maxResults)
      * finalizes recent additions, updates, or deletions of indexed entities.
      * <p>
      * 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.
+         * <p>
+         * 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();

gigamap/lucene/src/test/java/org/eclipse/store/gigamap/lucene/LuceneStoreBoundaryCommitTest.java

@@ -0,0 +1,163 @@
+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<Article>
+	{
+		@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<Article> ctx = LuceneContext.New(
+			DirectoryCreator.MMap(this.lucenePath),
+			AnalyzerCreator.Standard(),
+			new ArticlePopulator(),
+			false // manual / store-coupled commit
+		);
+
+		final GigaMap<Article> map = GigaMap.New();
+		final LuceneIndex<Article> 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<Article> map = GigaMap.New();
+		try(final LuceneIndex<Article> idx = map.index().register(LuceneIndex.Category(
+			LuceneContext.New(null, AnalyzerCreator.Standard(), new ArticlePopulator(), false)));
+			final EmbeddedStorageManager sm = EmbeddedStorage.start(map, this.storagePath))
+		{
+			map.add(new Article("eclipse", "graph directory"));
+			map.add(new Article("store", "boundary"));
+			map.store();
+		}
+
+		LuceneIndex<Article> idx2 = null;
+		try(final EmbeddedStorageManager sm = EmbeddedStorage.start(this.storagePath))
+		{
+			final GigaMap<Article> 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();
+			}
+		}
+	}
+}