You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by sp...@apache.org on 2026/09/04 13:49:19 UTC
(tinkerpop) 14/20: Add opt-in vertex-property id persistence to TinkerStorageGraph storage
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 41423dedbe40eb3f3218729ba4ca3d31540ef234
Author: Stephen Mallette <st...@amazon.com>
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)