bitflicker64 commented on code in PR #3375:
URL: https://github.com/apache/hugegraph/pull/3375#discussion_r4334091457
##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/store/BackendStoreProvider.java:
##########
@@ -38,6 +38,25 @@ public interface BackendStoreProvider {
// Current backend store driver version
String driverVersion();
+ /**
+ * The name of the serializer this backend reads and writes (see
+ * SerializerFactory): "binary" for the binary-only backends. A graph
+ * takes it unless its config names a serializer explicitly.
+ */
+ default String serializerName() {
+ return "binary";
+ }
+
+ /**
+ * Whether a graph on this backend may use a serializer other than
+ * {@link #serializerName()}. False for backends whose tables parse
+ * stored keys in one fixed format (hstore, rocksdb): rows written with
+ * another serializer are stored but fail to read.
+ */
+ default boolean acceptsOtherSerializer() {
+ return false;
Review Comment:
Important: Blocking: yes. Summary: Making `serializerName()="binary"` and
`acceptsOtherSerializer()=false` the interface defaults applies the
hstore/rocksdb rule to every backend that does not override them, including
plugin backends. A plugin registered through
`HugeGraphPlugin.registerBackend()` and `registerSerializer()` (loaded by
`RegisterUtil` via `ServiceLoader`) inherits these defaults. If its graph
config names its own serializer, for example `serializer=myserializer`, the
graph now fails to open with `The <x> backend requires serializer=binary`. If
the config has no serializer line, the plugin silently gets `binary` where it
used to get `text`. Evidence: `HugeGraphPlugin.java:39-45` exposes both
registration hooks; `AbstractBackendStoreProvider` does not override either
method; `StandardHugeGraph.serializerName()` throws whenever `configured !=
backendDefault && !provider.acceptsOtherSerializer()`. Requested change: make
the interface default lenient (`acceptsOtherSerial
izer()` returning true; for `serializerName()`, consider keeping the old
`text` fallback or requiring an override). Then override both methods in
`RocksDBStoreProvider` (which `RocksDBSstStoreProvider` inherits) and
`HstoreProvider`, so only the backends that really parse binary keys refuse
another serializer. Add a test that a provider which does not override the
methods still accepts an explicit serializer.
##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/profile/GraphsAPI.java:
##########
@@ -492,7 +492,6 @@ public Object create(@Context GraphManager manager,
// Auto-fill standalone (RocksDB) mode defaults
configs.putIfAbsent("backend", "rocksdb");
}
- configs.putIfAbsent("serializer", "binary");
Review Comment:
Important: Blocking: yes. Summary: This config, like the system-graph config
in `GraphManager` (the removed `sysGraphConfig.put("serializer", "binary")`),
is persisted to PD meta and loaded by every other server. With the line gone,
the stored config has no `serializer`. A server still on the previous release
in the same cluster (during a rolling upgrade, or after a downgrade) opens
these graphs with its old `CoreOptions.SERIALIZER` default `text` on hstore. It
writes rows through `TextSerializer` that can never be read back, which is the
#3225 failure this PR fixes. Evidence: `GraphManager.createGraph` calls
`metaManager.addGraphConfig(graphSpace, name, configs)` then `notifyGraphAdd`;
other servers rebuild the graph in `loadAddedGraph` from
`metaManager.getGraphConfig(...)`; the system graph config goes through
`metaManager.addSysGraphConfig`; on the base commit `CoreOptions.SERIALIZER` is
`disallowEmpty(), "text"`. Requested change: keep writing the serializer the
backend resol
ves into the persisted config for hstore and rocksdb, for example
`putIfAbsent("serializer", "binary")` when the backend is hstore or rocksdb
here and in `sysGraphConfig`. The new check accepts an explicit `binary` on
those backends, so this costs nothing and keeps the meta readable by older
servers until the option is removed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
