This is an automated email from the ASF dual-hosted git repository.
spmallette pushed a commit to branch tinkergraph-storage
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git
The following commit(s) were added to refs/heads/tinkergraph-storage by this
push:
new 08f6937de5 Restore TinkerStorageGraph index definitions on reopen
08f6937de5 is described below
commit 08f6937de529f74e8424935d474411087be77fac
Author: Stephen Mallette <[email protected]>
AuthorDate: Wed Sep 9 15:58:14 2026 -0400
Restore TinkerStorageGraph index definitions on reopen
An index created with createIndex() was not part of the durable state, so a
graph that was reopened came back correct but with every indexed lookup
silently degraded to a linear scan, with nothing to signal it. The indexed
keys
are now recorded beside the engine's files and the indexes are rebuilt after
replay, covering data written before the restart. A dropped index stays
dropped, and a record that cannot be read opens the graph with no indexes
rather than failing.
Definitions are kept outside the transaction log deliberately: an index only
affects how fast a lookup runs and never its result, so a definition lost
to a
crash costs a rebuild rather than any data. The file is owned by the graph
rather than the engine, so the TinkerStorage SPI is unchanged and a custom
engine gets the behaviour for free.
Assisted-by: Claude Code:claude-opus-5
Claude-Session: https://claude.ai/code/session_01KgH2VCpRw57sbFg5GoAiVV
---
.../reference/implementations-tinkergraph.asciidoc | 6 +
.../tinkergraph/structure/TinkerStorageGraph.java | 42 +++++
.../structure/storage/IndexDefinitions.java | 205 +++++++++++++++++++++
.../structure/storage/GraphBinaryStorageTest.java | 105 +++++++++++
4 files changed, 358 insertions(+)
diff --git a/docs/src/reference/implementations-tinkergraph.asciidoc
b/docs/src/reference/implementations-tinkergraph.asciidoc
index 0fc5ed45df..d4558a4ddc 100644
--- a/docs/src/reference/implementations-tinkergraph.asciidoc
+++ b/docs/src/reference/implementations-tinkergraph.asciidoc
@@ -468,6 +468,12 @@ committed state. Compaction runs when the graph is closed
and can be requested e
unbounded log, compaction also runs automatically once the log grows past
`gremlin.tinkergraph.storage.compactThreshold`
bytes. Setting that threshold to `0` disables automatic compaction.
+The property keys a graph indexes are recorded alongside its data and the
indexes are rebuilt when the graph is
+opened again, so an index created with `createIndex()` survives a restart and
covers the data that was already
+stored. An index dropped with `dropIndex()` stays dropped. Index definitions
are held outside the transaction log
+because an index only affects how fast a lookup runs and never its result, so
a definition lost to a crash costs a
+rebuild rather than any data. If the record of them cannot be read the graph
still opens, with no indexes.
+
Element and edge ids are always preserved across a reopen. Auto-generated
vertex-property ids are not, by default,
so a vertex property may receive a different id after a reopen. Setting
`gremlin.tinkergraph.storage.preserveVertexPropertyIds`
to `true` persists those ids as well, at the cost of a larger store.
diff --git
a/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerStorageGraph.java
b/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerStorageGraph.java
index dc68a89ea5..43c61a9fce 100644
---
a/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerStorageGraph.java
+++
b/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerStorageGraph.java
@@ -36,6 +36,7 @@ import
org.apache.tinkerpop.gremlin.tinkergraph.process.traversal.strategy.optim
import
org.apache.tinkerpop.gremlin.tinkergraph.process.traversal.strategy.optimization.TinkerGraphStepStrategy;
import org.apache.tinkerpop.gremlin.tinkergraph.services.TinkerServiceRegistry;
import
org.apache.tinkerpop.gremlin.tinkergraph.structure.storage.DirectoryLock;
+import
org.apache.tinkerpop.gremlin.tinkergraph.structure.storage.IndexDefinitions;
import org.apache.tinkerpop.gremlin.util.iterator.IteratorUtils;
import java.io.File;
@@ -84,6 +85,12 @@ public final class TinkerStorageGraph extends
AbstractTinkerGraph {
private final TinkerTransaction transaction = new TinkerTransaction(this);
+ /**
+ * Set while indexes recorded for the store are being recreated on open,
so applying them does not rewrite the
+ * file they were just read from.
+ */
+ private boolean restoringIndexes = false;
+
private final Map<Object, TinkerElementContainer<TinkerVertex>> vertices =
new ConcurrentHashMap<>();
private final Map<Object, TinkerElementContainer<TinkerEdge>> edges = new
ConcurrentHashMap<>();
@@ -131,6 +138,9 @@ public final class TinkerStorageGraph extends
AbstractTinkerGraph {
} finally {
loading = false;
}
+ // recreate the recorded indexes now that replay has rebuilt
the elements they cover, so
+ // createKeyIndex backfills over the restored data rather than
only over writes that follow
+ restoreIndexes(dir);
} catch (RuntimeException | Error ex) {
// don't leak the lock if the engine fails to open or replay
directoryLock.close();
@@ -609,6 +619,7 @@ public final class TinkerStorageGraph extends
AbstractTinkerGraph {
} else {
throw new IllegalArgumentException("Class is not indexable: " +
elementClass);
}
+ recordIndexes();
}
/**
@@ -627,5 +638,36 @@ public final class TinkerStorageGraph extends
AbstractTinkerGraph {
} else {
throw new IllegalArgumentException("Class is not indexable: " +
elementClass);
}
+ recordIndexes();
+ }
+
+ /**
+ * Recreate the indexes recorded for this store. Runs after replay so that
{@code createKeyIndex} backfills over
+ * the elements it has just rebuilt. The definitions are already on disk,
so recording is suppressed while they
+ * are applied.
+ */
+ private void restoreIndexes(final File directory) {
+ final IndexDefinitions definitions = IndexDefinitions.read(directory);
+ if (definitions.isEmpty())
+ return;
+ restoringIndexes = true;
+ try {
+ definitions.vertexKeys().forEach(key -> createIndex(key,
Vertex.class));
+ definitions.edgeKeys().forEach(key -> createIndex(key,
Edge.class));
+ } finally {
+ restoringIndexes = false;
+ }
+ }
+
+ /**
+ * Record the current set of indexed keys beside the engine's files, so a
reopen restores them. Index definitions
+ * are not part of the transactional log; see {@link IndexDefinitions} for
why that is sound. A graph with no
+ * storage engine keeps everything in memory and writes nothing.
+ */
+ private void recordIndexes() {
+ if (null == storage || restoringIndexes)
+ return;
+ new IndexDefinitions(getIndexedKeys(Vertex.class),
getIndexedKeys(Edge.class))
+ .write(new File(storageDirectory));
}
}
diff --git
a/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/storage/IndexDefinitions.java
b/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/storage/IndexDefinitions.java
new file mode 100644
index 0000000000..07e63b9bd6
--- /dev/null
+++
b/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/storage/IndexDefinitions.java
@@ -0,0 +1,205 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.apache.tinkerpop.gremlin.tinkergraph.structure.storage;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.File;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.OutputStreamWriter;
+import java.io.Writer;
+import java.nio.charset.StandardCharsets;
+import java.nio.channels.FileChannel;
+import java.nio.file.AtomicMoveNotSupportedException;
+import java.nio.file.Files;
+import java.nio.file.StandardCopyOption;
+import java.nio.file.StandardOpenOption;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Set;
+
+/**
+ * The set of property keys a persistent graph indexes, held in a small file
beside the storage engine's own files.
+ * <p/>
+ * Index definitions are deliberately kept out of the transactional log. An
index is purely an optimization, so losing
+ * a definition to a crash costs a rebuild rather than any data: a crash
between {@code createIndex} and the write here
+ * drops a definition the caller can recreate, and one after {@code dropIndex}
resurrects an index that is dropped
+ * again. Neither can change a query result, which is what makes a file
outside the write-ahead log an honest place to
+ * keep this. Nothing that affects correctness may be stored this way.
+ * <p/>
+ * The file is line oriented and readable, one record per line: {@code V} or
{@code E}, a tab, then the property key
+ * with backslash, tab, newline and carriage return escaped, so a key
containing any of them survives a round trip.
+ */
+public final class IndexDefinitions {
+
+ private static final Logger logger =
LoggerFactory.getLogger(IndexDefinitions.class);
+
+ static final String INDEX_FILE = "INDEXES";
+
+ private static final String VERTEX = "V";
+ private static final String EDGE = "E";
+
+ private final Set<String> vertexKeys;
+ private final Set<String> edgeKeys;
+
+ public IndexDefinitions(final Set<String> vertexKeys, final Set<String>
edgeKeys) {
+ this.vertexKeys = new LinkedHashSet<>(vertexKeys);
+ this.edgeKeys = new LinkedHashSet<>(edgeKeys);
+ }
+
+ public Set<String> vertexKeys() {
+ return vertexKeys;
+ }
+
+ public Set<String> edgeKeys() {
+ return edgeKeys;
+ }
+
+ public boolean isEmpty() {
+ return vertexKeys.isEmpty() && edgeKeys.isEmpty();
+ }
+
+ /**
+ * Read the definitions recorded in {@code directory}, or an empty set if
none have been recorded.
+ * <p/>
+ * A file that cannot be read is reported and treated as empty rather than
raised. The graph then opens with no
+ * indexes, which is exactly how it behaved before definitions were
recorded at all, so an unreadable file can
+ * never make a store less openable than the data it holds.
+ */
+ public static IndexDefinitions read(final File directory) {
+ final File file = new File(directory, INDEX_FILE);
+ if (!file.isFile())
+ return new IndexDefinitions(new LinkedHashSet<>(), new
LinkedHashSet<>());
+
+ final Set<String> vertexKeys = new LinkedHashSet<>();
+ final Set<String> edgeKeys = new LinkedHashSet<>();
+ try {
+ final List<String> lines = Files.readAllLines(file.toPath(),
StandardCharsets.UTF_8);
+ for (final String line : lines) {
+ if (line.isEmpty() || line.charAt(0) == '#')
+ continue;
+ final int tab = line.indexOf('\t');
+ if (tab < 0)
+ throw new IOException("Malformed index definition line: "
+ line);
+ final String key = unescape(line.substring(tab + 1));
+ switch (line.substring(0, tab)) {
+ case VERTEX: vertexKeys.add(key); break;
+ case EDGE: edgeKeys.add(key); break;
+ default: throw new IOException("Unknown index element type
in line: " + line);
+ }
+ }
+ } catch (IOException ex) {
+ logger.warn("Could not read index definitions from {}; opening
with no indexes. " +
+ "Recreate them with createIndex() if they are wanted.",
file, ex);
+ return new IndexDefinitions(new LinkedHashSet<>(), new
LinkedHashSet<>());
+ }
+ return new IndexDefinitions(vertexKeys, edgeKeys);
+ }
+
+ /**
+ * Replace the definitions recorded in {@code directory}. Written to a
temporary file, forced to the device and
+ * renamed into place, so a crash leaves either the previous set or the
new one and never a partial file.
+ */
+ public void write(final File directory) {
+ final File file = new File(directory, INDEX_FILE);
+ if (isEmpty()) {
+ try {
+ Files.deleteIfExists(file.toPath());
+ syncDirectory(directory);
+ } catch (IOException ex) {
+ logger.warn("Could not remove index definitions file {}",
file, ex);
+ }
+ return;
+ }
+
+ final File tmp = new File(directory, INDEX_FILE + ".tmp");
+ try {
+ try (final FileOutputStream fos = new FileOutputStream(tmp);
+ final Writer out = new OutputStreamWriter(fos,
StandardCharsets.UTF_8)) {
+ out.write("# TinkerGraph index definitions; recreated on
open\n");
+ for (final String key : vertexKeys)
+ out.write(VERTEX + '\t' + escape(key) + '\n');
+ for (final String key : edgeKeys)
+ out.write(EDGE + '\t' + escape(key) + '\n');
+ out.flush();
+ fos.getFD().sync();
+ }
+ atomicMove(tmp, file);
+ syncDirectory(directory);
+ } catch (IOException ex) {
+ logger.warn("Could not record index definitions in {}; they will
not survive a reopen", file, ex);
+ }
+ }
+
+ private static String escape(final String key) {
+ final StringBuilder sb = new StringBuilder(key.length());
+ for (int i = 0; i < key.length(); i++) {
+ final char c = key.charAt(i);
+ switch (c) {
+ case '\\': sb.append("\\\\"); break;
+ case '\t': sb.append("\\t"); break;
+ case '\n': sb.append("\\n"); break;
+ case '\r': sb.append("\\r"); break;
+ default: sb.append(c);
+ }
+ }
+ return sb.toString();
+ }
+
+ private static String unescape(final String value) throws IOException {
+ final StringBuilder sb = new StringBuilder(value.length());
+ for (int i = 0; i < value.length(); i++) {
+ final char c = value.charAt(i);
+ if (c != '\\') {
+ sb.append(c);
+ continue;
+ }
+ if (++i == value.length())
+ throw new IOException("Index definition ends with a dangling
escape: " + value);
+ switch (value.charAt(i)) {
+ case '\\': sb.append('\\'); break;
+ case 't': sb.append('\t'); break;
+ case 'n': sb.append('\n'); break;
+ case 'r': sb.append('\r'); break;
+ default: throw new IOException("Unknown escape in index
definition: " + value);
+ }
+ }
+ return sb.toString();
+ }
+
+ private static void atomicMove(final File source, final File target)
throws IOException {
+ try {
+ Files.move(source.toPath(), target.toPath(),
+ StandardCopyOption.ATOMIC_MOVE,
StandardCopyOption.REPLACE_EXISTING);
+ } catch (AtomicMoveNotSupportedException anse) {
+ Files.move(source.toPath(), target.toPath(),
StandardCopyOption.REPLACE_EXISTING);
+ }
+ }
+
+ private static void syncDirectory(final File directory) {
+ try (final FileChannel dirChannel =
FileChannel.open(directory.toPath(), StandardOpenOption.READ)) {
+ dirChannel.force(true);
+ } catch (IOException ex) {
+ // some platforms (notably Windows) cannot open a directory as a
channel; the atomic rename is the
+ // durability guarantee there, so treat inability to sync the
directory as non-fatal
+ }
+ }
+}
diff --git
a/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/storage/GraphBinaryStorageTest.java
b/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/storage/GraphBinaryStorageTest.java
index c001d94afd..61af24234c 100644
---
a/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/storage/GraphBinaryStorageTest.java
+++
b/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/storage/GraphBinaryStorageTest.java
@@ -18,7 +18,10 @@
*/
package org.apache.tinkerpop.gremlin.tinkergraph.structure.storage;
+import org.apache.commons.configuration2.BaseConfiguration;
import org.apache.commons.configuration2.Configuration;
+import org.apache.tinkerpop.gremlin.structure.Edge;
+import org.apache.tinkerpop.gremlin.structure.Graph;
import org.apache.tinkerpop.gremlin.structure.T;
import org.apache.tinkerpop.gremlin.structure.Vertex;
import org.apache.tinkerpop.gremlin.structure.VertexProperty;
@@ -33,7 +36,9 @@ import java.io.File;
import java.io.IOException;
import java.io.RandomAccessFile;
import java.nio.ByteBuffer;
+import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
+import java.util.Collections;
import java.util.HashMap;
import java.util.Iterator;
import java.util.Map;
@@ -313,6 +318,106 @@ public class GraphBinaryStorageTest extends
AbstractTinkerStorageConformanceTest
graph.close();
}
+ @Test
+ public void shouldRestoreIndexDefinitionsOnReopen() throws Exception {
+ TinkerStorageGraph graph = open();
+ graph.createIndex("name", Vertex.class);
+ graph.createIndex("weight", Edge.class);
+ final Vertex marko = graph.addVertex(T.id, 1, "name", "marko");
+ final Vertex josh = graph.addVertex(T.id, 2, "name", "josh");
+ marko.addEdge("knows", josh, T.id, 10, "weight", 0.5d);
+ graph.tx().commit();
+ graph.close();
+
+ graph = open();
+ try {
+ // the definitions come back, and the index covers data written
before the restart rather than only
+ // writes that follow it
+ assertEquals(Collections.singleton("name"),
graph.getIndexedKeys(Vertex.class));
+ assertEquals(Collections.singleton("weight"),
graph.getIndexedKeys(Edge.class));
+ assertEquals(1L, (long) graph.traversal().V().has("name",
"josh").count().next());
+ assertEquals(1L, (long) graph.traversal().E().has("weight",
0.5d).count().next());
+ } finally {
+ graph.close();
+ }
+ }
+
+ @Test
+ public void shouldNotRestoreADroppedIndex() throws Exception {
+ TinkerStorageGraph graph = open();
+ graph.createIndex("name", Vertex.class);
+ graph.addVertex(T.id, 1, "name", "marko");
+ graph.tx().commit();
+ graph.dropIndex("name", Vertex.class);
+ graph.close();
+
+ graph = open();
+ try {
+ assertTrue("a dropped index must not come back",
graph.getIndexedKeys(Vertex.class).isEmpty());
+ // data is unaffected by the index being gone; the lookup just
falls back to a scan
+ assertEquals(1L, (long) graph.traversal().V().has("name",
"marko").count().next());
+ } finally {
+ graph.close();
+ }
+ }
+
+ @Test
+ public void shouldOpenWithoutIndexesWhenDefinitionsAreUnreadable() throws
Exception {
+ TinkerStorageGraph graph = open();
+ final String location =
graph.configuration().getString(TinkerGraph.GREMLIN_TINKERGRAPH_STORAGE_DIRECTORY);
+ graph.createIndex("name", Vertex.class);
+ graph.addVertex(T.id, 1, "name", "marko");
+ graph.tx().commit();
+ graph.close();
+
+ // a corrupt sidecar must degrade to the behaviour before definitions
were recorded at all, never block a
+ // store whose data is perfectly readable
+ Files.write(new File(location, IndexDefinitions.INDEX_FILE).toPath(),
+ "this is not an index
definition".getBytes(StandardCharsets.UTF_8));
+
+ graph = open();
+ try {
+ assertTrue(graph.getIndexedKeys(Vertex.class).isEmpty());
+ assertEquals(1L, (long) graph.traversal().V().has("name",
"marko").count().next());
+ } finally {
+ graph.close();
+ }
+ }
+
+ @Test
+ public void shouldRoundTripIndexKeysNeedingEscapes() throws Exception {
+ final String awkward = "a\tb\nc\\d";
+ TinkerStorageGraph graph = open();
+ graph.createIndex(awkward, Vertex.class);
+ graph.addVertex(T.id, 1, awkward, "value");
+ graph.tx().commit();
+ graph.close();
+
+ graph = open();
+ try {
+ assertEquals(Collections.singleton(awkward),
graph.getIndexedKeys(Vertex.class));
+ assertEquals(1L, (long) graph.traversal().V().has(awkward,
"value").count().next());
+ } finally {
+ graph.close();
+ }
+ }
+
+ @Test
+ public void shouldNotRecordIndexesWithoutAStorageEngine() {
+ // a TinkerStorageGraph with no engine is transactional but in-memory,
so it must touch no disk at all
+ final Configuration conf = new BaseConfiguration();
+ conf.setProperty(Graph.GRAPH, TinkerStorageGraph.class.getName());
+ final TinkerStorageGraph graph = TinkerStorageGraph.open(conf);
+ try {
+ graph.createIndex("name", Vertex.class);
+ graph.addVertex(T.id, 1, "name", "marko");
+ graph.tx().commit();
+ assertEquals(Collections.singleton("name"),
graph.getIndexedKeys(Vertex.class));
+ } finally {
+ graph.close();
+ }
+ }
+
@Test
public void shouldFailOnCorruptFrameWithBadCrc() throws Exception {
TinkerStorageGraph graph = open();
