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
commit 7832c81a9c53879a8e27c0d544dba0d28baf00b1 Author: Stephen Mallette <[email protected]> AuthorDate: Wed Aug 19 14:37:00 2026 +0000 Add opt-in vertex-property id persistence to TinkerStorageGraph storage Add gremlin.tinkergraph.storage.preserveVertexPropertyIds (default false). When enabled, the GraphBinary codec persists each vertex-property id so it is stable across reopen; by default those ids are regenerated on load to keep the store smaller. A per-vertex-record flag makes each record self-describing, so a store written with the option reopens correctly even if the reader's setting differs. A new configureCodec hook lets the codec read its own config at open. Also adds codec tests: dictionary growth across log commits, dictionary rewrite on compaction, preserve-ids on/off, and a bytes/element size-regression guard (well under the ~168 bytes/element the old whole-object format cost). Assisted-by: Claude Code:claude-opus-4-8 --- .../gremlin/tinkergraph/structure/TinkerGraph.java | 8 ++ .../structure/storage/AbstractLogStorage.java | 8 ++ .../structure/storage/GraphBinaryStorage.java | 21 ++++ .../structure/storage/GraphBinaryStorageTest.java | 119 +++++++++++++++++++++ 4 files changed, 156 insertions(+) diff --git a/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerGraph.java b/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerGraph.java index 65a02537c6..f011cd5202 100644 --- a/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerGraph.java +++ b/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerGraph.java @@ -75,6 +75,14 @@ public interface TinkerGraph extends Graph { * set. */ String GREMLIN_TINKERGRAPH_STORAGE_COMPACT_THRESHOLD = "gremlin.tinkergraph.storage.compactThreshold"; + /** + * Whether a {@link TinkerStorageGraph} storage engine persists auto-generated vertex-property ids so they are + * stable across a close and reopen. Defaults to {@code false}: vertex-property ids are regenerated on load, which + * keeps the store smaller. Element and edge ids are always preserved regardless of this setting. Each record is + * self-describing, so a store written with this enabled reopens correctly even if the setting later differs. Only + * meaningful when {@link #GREMLIN_TINKERGRAPH_STORAGE} is set. + */ + String GREMLIN_TINKERGRAPH_STORAGE_PRESERVE_VP_IDS = "gremlin.tinkergraph.storage.preserveVertexPropertyIds"; String GREMLIN_TINKERGRAPH_ALLOW_NULL_PROPERTY_VALUES = "gremlin.tinkergraph.allowNullPropertyValues"; String GREMLIN_TINKERGRAPH_SERVICE = "gremlin.tinkergraph.service"; String GREMLIN_TINKERGRAPH_VERTEX_LABEL_CARDINALITY = "gremlin.tinkergraph.vertexLabelCardinality"; diff --git a/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/storage/AbstractLogStorage.java b/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/storage/AbstractLogStorage.java index b2bdf249f5..4c8eefeb08 100644 --- a/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/storage/AbstractLogStorage.java +++ b/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/storage/AbstractLogStorage.java @@ -155,10 +155,18 @@ public abstract class AbstractLogStorage implements TinkerStorage { TinkerGraph.GREMLIN_TINKERGRAPH_STORAGE_COMPACT_THRESHOLD, DEFAULT_COMPACT_THRESHOLD_BYTES); // seed the counter with any pre-existing log so a graph reopened with a large log still compacts promptly this.logBytesSinceCompaction = logFile.exists() ? logFile.length() : 0; + configureCodec(config); ensureDirectory(); establishStoreVersion(); } + /** + * Read any codec-specific configuration. Called once during {@link #open}. Default is a no-op. + */ + protected void configureCodec(final Configuration config) { + // no-op by default + } + @Override public void replay(final AbstractTinkerGraph graph) { beginReplay(); diff --git a/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/storage/GraphBinaryStorage.java b/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/storage/GraphBinaryStorage.java index 1266dee705..7c6d0cff77 100644 --- a/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/storage/GraphBinaryStorage.java +++ b/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/storage/GraphBinaryStorage.java @@ -18,6 +18,7 @@ */ package org.apache.tinkerpop.gremlin.tinkergraph.structure.storage; +import org.apache.commons.configuration2.Configuration; import org.apache.tinkerpop.gremlin.structure.Edge; import org.apache.tinkerpop.gremlin.structure.Property; import org.apache.tinkerpop.gremlin.structure.Vertex; @@ -33,6 +34,7 @@ import org.apache.tinkerpop.gremlin.structure.util.detached.DetachedVertex; import org.apache.tinkerpop.gremlin.structure.util.detached.DetachedVertexProperty; import org.apache.tinkerpop.gremlin.tinkergraph.structure.AbstractTinkerGraph; import org.apache.tinkerpop.gremlin.tinkergraph.structure.TinkerEdge; +import org.apache.tinkerpop.gremlin.tinkergraph.structure.TinkerGraph; import org.apache.tinkerpop.gremlin.tinkergraph.structure.TinkerVertex; import java.io.DataOutputStream; @@ -84,6 +86,17 @@ public final class GraphBinaryStorage extends AbstractLogStorage { private final Map<String, Integer> keyToId = new HashMap<>(); private final List<String> idToKey = new ArrayList<>(); + /** + * When true, persist auto-generated vertex-property ids so they are stable across reopen. Written per vertex + * record so a store reopens correctly regardless of the reader's setting. + */ + private boolean preserveVertexPropertyIds = false; + + @Override + protected void configureCodec(final Configuration config) { + this.preserveVertexPropertyIds = config.getBoolean(TinkerGraph.GREMLIN_TINKERGRAPH_STORAGE_PRESERVE_VP_IDS, false); + } + @Override protected void beginReplay() { keyToId.clear(); @@ -213,6 +226,9 @@ public final class GraphBinaryStorage extends AbstractLogStorage { for (final String label : labels) writeVarInt(buf, keyToId.get(label)); + // self-describing flag: whether each value below carries a persisted vertex-property id + buf.writeByte(preserveVertexPropertyIds ? 1 : 0); + // group vertex properties by key so multi-properties (list/set) round-trip final Map<String, List<VertexProperty<Object>>> groups = new LinkedHashMap<>(); final Iterator<VertexProperty<Object>> vps = v.properties(); @@ -227,6 +243,8 @@ public final class GraphBinaryStorage extends AbstractLogStorage { writeVarInt(buf, values.size()); for (final VertexProperty<Object> vp : values) { writeScalar(buf, vp.value()); + if (preserveVertexPropertyIds) + writeScalar(buf, vp.id()); final List<Property<Object>> metas = new ArrayList<>(); vp.properties().forEachRemaining(metas::add); writeVarInt(buf, metas.size()); @@ -315,6 +333,7 @@ public final class GraphBinaryStorage extends AbstractLogStorage { labels.add(idToKey.get(readVarInt(buf))); b.setLabels(labels); } + final boolean hasVpIds = buf.readByte() != 0; final int keyGroupCount = readVarInt(buf); for (int g = 0; g < keyGroupCount; g++) { final String key = idToKey.get(readVarInt(buf)); @@ -322,6 +341,8 @@ public final class GraphBinaryStorage extends AbstractLogStorage { for (int j = 0; j < valueCount; j++) { final Object value = readScalar(buf); final DetachedVertexProperty.Builder vpb = DetachedVertexProperty.build().setLabel(key).setValue(value); + if (hasVpIds) + vpb.setId(readScalar(buf)); final int metaCount = readVarInt(buf); for (int m = 0; m < metaCount; m++) { final String metaKey = idToKey.get(readVarInt(buf)); 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 cbde701a22..f5347f0920 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 @@ -21,6 +21,7 @@ package org.apache.tinkerpop.gremlin.tinkergraph.structure.storage; import org.apache.commons.configuration2.Configuration; import org.apache.tinkerpop.gremlin.structure.T; import org.apache.tinkerpop.gremlin.structure.Vertex; +import org.apache.tinkerpop.gremlin.structure.VertexProperty; import org.apache.tinkerpop.gremlin.tinkergraph.structure.TinkerGraph; import org.apache.tinkerpop.gremlin.tinkergraph.structure.TinkerStorageGraph; import org.junit.Test; @@ -384,6 +385,124 @@ public class GraphBinaryStorageTest extends AbstractTinkerStorageConformanceTest } } + @Test + public void shouldRegenerateVertexPropertyIdsByDefault() { + // default: vertex-property ids are not persisted; the property still round-trips (value + meta), the id is + // simply reassigned on load + TinkerStorageGraph graph = open(); + try { + final VertexProperty<Object> vp = graph.addVertex(T.id, 1).property(VertexProperty.Cardinality.list, "name", "marko"); + vp.property("since", 2010); + graph.tx().commit(); + } finally { + graph.close(); + } + graph = open(); + try { + final VertexProperty<Object> vp = graph.vertices(1).next().<Object>properties("name").next(); + assertEquals("marko", vp.value()); + assertEquals(Integer.valueOf(2010), vp.<Integer>property("since").value()); + } finally { + graph.close(); + } + } + + @Test + public void shouldPreserveVertexPropertyIdsWhenConfigured() { + final Configuration conf = config(); + conf.setProperty(TinkerGraph.GREMLIN_TINKERGRAPH_STORAGE_PRESERVE_VP_IDS, true); + final Object vpId; + TinkerStorageGraph graph = TinkerStorageGraph.open(conf); + try { + final VertexProperty<Object> vp = graph.addVertex(T.id, 1).property(VertexProperty.Cardinality.list, "name", "marko"); + vpId = vp.id(); + graph.tx().commit(); + } finally { + graph.close(); + } + graph = TinkerStorageGraph.open(conf); + try { + final VertexProperty<Object> vp = graph.vertices(1).next().<Object>properties("name").next(); + assertEquals("marko", vp.value()); + assertEquals("vertex-property id should be preserved across reopen", vpId, vp.id()); + } finally { + graph.close(); + } + } + + @Test + public void shouldReplayDictionaryGrowthAcrossLogCommits() throws Exception { + // each commit introduces a new property key, so the dictionary grows via OP_DICT_APPEND across successive log + // frames. Reopening from a log with no snapshot must rebuild the dictionary incrementally and resolve all refs. + final Configuration conf = config(); + conf.setProperty(TinkerGraph.GREMLIN_TINKERGRAPH_STORAGE_COMPACT_THRESHOLD, 0L); // keep the log, no auto-compaction + final String location = conf.getString(TinkerGraph.GREMLIN_TINKERGRAPH_GRAPH_LOCATION); + TinkerStorageGraph graph = TinkerStorageGraph.open(conf); + for (int i = 0; i < 10; i++) { + graph.addVertex(T.id, i, "key" + i, i); + graph.tx().commit(); + } + graph.tx().close(); + final byte[] log = Files.readAllBytes(new File(location, GraphBinaryStorage.LOG_FILE).toPath()); + graph.close(); + + // restore a log-only store (no snapshot) and reopen + Files.deleteIfExists(new File(location, GraphBinaryStorage.SNAPSHOT_FILE).toPath()); + Files.write(new File(location, GraphBinaryStorage.LOG_FILE).toPath(), log); + graph = TinkerStorageGraph.open(conf); + try { + assertEquals(10, countOf(graph.vertices())); + for (int i = 0; i < 10; i++) + assertEquals(Integer.valueOf(i), graph.vertices(i).next().value("key" + i)); + } finally { + graph.close(); + } + } + + @Test + public void shouldRewriteDictionaryOnCompactionAndReopen() { + TinkerStorageGraph graph = open(); + try { + for (int i = 0; i < 10; i++) + graph.addVertex(T.id, i, "key" + i, i); + graph.tx().commit(); + graph.compact(); // writes a fresh full-dictionary snapshot header, then element frames + } finally { + graph.close(); + } + graph = open(); + try { + assertEquals(10, countOf(graph.vertices())); + for (int i = 0; i < 10; i++) + assertEquals(Integer.valueOf(i), graph.vertices(i).next().value("key" + i)); + } finally { + graph.close(); + } + } + + @Test + public void shouldStoreFewBytesPerElement() { + // regression guard: the dictionary-encoded format must stay well under the ~168 bytes/element the old + // whole-object format cost for a comparable graph (3 vertex props, 2 edge props, E=V). + final int vertexCount = 200; + final TinkerStorageGraph graph = open(); + final String location = graph.configuration().getString(TinkerGraph.GREMLIN_TINKERGRAPH_GRAPH_LOCATION); + try { + for (int i = 0; i < vertexCount; i++) + graph.addVertex(T.id, i, "name", "v" + i, "age", i % 100, "score", i * 1.5d); + for (int i = 0; i < vertexCount; i++) + graph.vertices(i).next().addEdge("knows", graph.vertices((i + 1) % vertexCount).next(), + T.id, 1_000_000 + i, "weight", i * 0.5d, "count", i % 7); + graph.tx().commit(); + graph.compact(); + final long bytes = new File(location, GraphBinaryStorage.SNAPSHOT_FILE).length(); + final double perElement = (double) bytes / (2 * vertexCount); + assertTrue("expected well under 168 bytes/element (whole-object baseline), got " + perElement, perElement < 100.0); + } finally { + graph.close(); + } + } + private static String rootMessage(final Throwable t) { Throwable cur = t; while (cur.getCause() != null && cur.getCause() != cur)
