JanusGraph / janusgraph

JanusGraph: an open-source, distributed graph database
https://janusgraph.org
Other
5.34k stars 1.17k forks source link

Serialization error querying for vertex properties using GraphBinary #3173

Open bill-poole opened 2 years ago

bill-poole commented 2 years ago

I'm new to using JanusGraph, so apologies if this is caused by something I've done wrong.

The following code in Gremlin.Net produces the following error in JanusGraph.

g.AddV("person").Property(Cardinality.Single, "name", "jorge").Iterate();
var result = g.V().Has("person", "name", "jorge").Properties<VertexProperty>().ToList();
793973 [gremlin-server-session-1] WARN  org.apache.tinkerpop.gremlin.server.op.AbstractEvalOpProcessor  - The result [[vp[name->jorge]]] in the request ffcf64e5-6dac-4f8e-a1e6-e4c0c53f1b19 could not be serialized and returned.
java.lang.IllegalStateException
        at com.google.common.base.Preconditions.checkState(Preconditions.java:492)
        at org.janusgraph.graphdb.relations.RelationIdentifier.getInVertexId(RelationIdentifier.java:63)
        at org.janusgraph.graphdb.tinkerpop.io.binary.RelationIdentifierGraphBinarySerializer.writeNonNullableValue(RelationIdentifierGraphBinarySerializer.java:43)
        at org.janusgraph.graphdb.tinkerpop.io.binary.RelationIdentifierGraphBinarySerializer.writeNonNullableValue(RelationIdentifierGraphBinarySerializer.java:24)
        at org.janusgraph.graphdb.tinkerpop.io.binary.JanusGraphTypeSerializer.writeValue(JanusGraphTypeSerializer.java:89)
        at org.janusgraph.graphdb.tinkerpop.io.binary.JanusGraphTypeSerializer.write(JanusGraphTypeSerializer.java:72)
        at org.apache.tinkerpop.gremlin.structure.io.binary.GraphBinaryWriter.write(GraphBinaryWriter.java:98)
        at org.apache.tinkerpop.gremlin.structure.io.binary.types.VertexPropertySerializer.writeValue(VertexPropertySerializer.java:56)
        at org.apache.tinkerpop.gremlin.structure.io.binary.types.VertexPropertySerializer.writeValue(VertexPropertySerializer.java:33)
        at org.apache.tinkerpop.gremlin.structure.io.binary.types.SimpleTypeSerializer.writeValue(SimpleTypeSerializer.java:91)
        at org.apache.tinkerpop.gremlin.structure.io.binary.types.SimpleTypeSerializer.write(SimpleTypeSerializer.java:73)
        at org.apache.tinkerpop.gremlin.structure.io.binary.GraphBinaryWriter.write(GraphBinaryWriter.java:112)
        at org.apache.tinkerpop.gremlin.structure.io.binary.types.TraverserSerializer.writeValue(TraverserSerializer.java:49)
        at org.apache.tinkerpop.gremlin.structure.io.binary.types.TraverserSerializer.writeValue(TraverserSerializer.java:33)
        at org.apache.tinkerpop.gremlin.structure.io.binary.types.SimpleTypeSerializer.writeValue(SimpleTypeSerializer.java:91)
        at org.apache.tinkerpop.gremlin.structure.io.binary.types.SimpleTypeSerializer.write(SimpleTypeSerializer.java:73)
        at org.apache.tinkerpop.gremlin.structure.io.binary.GraphBinaryWriter.write(GraphBinaryWriter.java:112)
        at org.apache.tinkerpop.gremlin.structure.io.binary.types.CollectionSerializer.writeValue(CollectionSerializer.java:52)
        at org.apache.tinkerpop.gremlin.structure.io.binary.types.ListSerializer.writeValue(ListSerializer.java:44)
        at org.apache.tinkerpop.gremlin.structure.io.binary.types.ListSerializer.writeValue(ListSerializer.java:29)
        at org.apache.tinkerpop.gremlin.structure.io.binary.types.SimpleTypeSerializer.writeValue(SimpleTypeSerializer.java:91)
        at org.apache.tinkerpop.gremlin.structure.io.binary.types.SimpleTypeSerializer.write(SimpleTypeSerializer.java:73)
        at org.apache.tinkerpop.gremlin.structure.io.binary.GraphBinaryWriter.write(GraphBinaryWriter.java:112)
        at org.apache.tinkerpop.gremlin.driver.ser.binary.ResponseMessageSerializer.writeValue(ResponseMessageSerializer.java:84)
        at org.apache.tinkerpop.gremlin.driver.ser.GraphBinaryMessageSerializerV1.serializeResponseAsBinary(GraphBinaryMessageSerializerV1.java:150)
        at org.apache.tinkerpop.gremlin.server.op.AbstractOpProcessor.makeFrame(AbstractOpProcessor.java:286)
        at org.apache.tinkerpop.gremlin.server.op.session.SessionOpProcessor.handleIterator(SessionOpProcessor.java:630)
        at org.apache.tinkerpop.gremlin.server.op.session.SessionOpProcessor.lambda$iterateBytecodeTraversal$5(SessionOpProcessor.java:421)
        at java.util.concurrent.FutureTask.run(FutureTask.java:266)
        at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
        at java.util.concurrent.FutureTask.run(FutureTask.java:266)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
        at java.lang.Thread.run(Thread.java:750)
FlorianHockmann commented 2 years ago

Looks like you found a bug in the GraphBinary serialization. I could confirm this also directly with the Gremlin Console, so irrespective of Gremlin.NET.

We should of course fix this bug, but in the meantime you can probably simply adapt your traversal a bit. Properties() returns a list of VertexProperty objects. I'm not sure if that's really what you want. If you just want to get a Dictionary of property keys with their values, then the valueMap() step is probably what you want:

var result = g.V().Has("person", "name", "jorge").ValueMap<string, object>().ToList();

Apart from that, two general notes regarding Gremlin.Net & JanusGraph that might be helpful for you:

  1. Using the async API is generally more efficient as it lets you await traversals and the thread can therefore execute something else in the meantime instead of being blocked until the server responds. You do that simply like this:

    var result = await g.V().Has("person", "name", "jorge").ValueMap<string, object>().Promise(t => t.ToList());
  2. You might want to use JanusGraph.Net which wraps Gremlin.Net and adds support for JanusGraph specific types.

bill-poole commented 2 years ago

Thanks very much @FlorianHockmann! I was curious as to what the E2 generic type parameter should be for the for the GraphTraversal.Properties<S, E>.Properties<E2>() method. I figured it should always return a traversal of VertexProperty objects (i.e., it seems like the E2 generic method type parameter is redundant because it should always be VertexProperty), so was trying to learn from doing as such.

I also have found it hard to find definitions for the S and E type parameters for GraphTraversal<S, E>. They don't seem to have any definition in the XML comments or online documentation - even for the Java library. So I've been experimenting to try to work out exactly what they mean.

I don't suppose you could point me at the relevant documentation?

bill-poole commented 2 years ago

Sorry one further question, I couldn't find any documentation that stated whether a series of async traversals will execute on the server in the same order that they are sent. For example, is there any guarantee that "jorge" will be added before "josh" in the following?

await Task.WhenAll(
  gtx.AddV("person")
    .Property(Cardinality.Single, "name", "jorge").Promise(t => t.Iterate()),
  gtx.AddV("person")
    .Property(Cardinality.Single, "name", "josh").Promise(t => t.Iterate())
  );

The semantics of the above imply that "jorge" and "josh" are being added concurrently and that a single-threaded transaction could add them in either order - i.e., there is a race condition. If so, then how does one execute a series of async traversals in a specific sequence without awaiting each one? The problem with awaiting each traversal before sending the next to the server is the roundtrip latency for each traversal.

bill-poole commented 2 years ago

I also have found it hard to find definitions for the S and E type parameters for GraphTraversal<S, E>. They don't seem to have any definition in the XML comments or online documentation - even for the Java library. So I've been experimenting to try to work out exactly what they mean.

I found the relevant documentation here. S stands for start and E stands for end.

FlorianHockmann commented 2 years ago

The semantics of the above imply that "jorge" and "josh" are being added concurrently and that a single-threaded transaction could add them in either order - i.e., there is a race condition. If so, then how does one execute a series of async traversals in a specific sequence without awaiting each one? The problem with awaiting each traversal before sending the next to the server is the roundtrip latency for each traversal.

Right, I think awaiting each traversal is the only option that guarantees serial execution. Gremlin would need some identifier that is incremented for each traversal coming from a certain client and then let the server only execute traversals in the expected order. Without such a logic, a simple network hiccup that delays the first traversal would be enough to let the server execute the second traversal first.

If you however just want to execute a small number of traversals in a defined order, than sending them together as one transaction could be an option. This is documented here in general with a short example here for Gremlin.Net.

I found the relevant documentation here. S stands for start and E stands for end.

Glad that you've already found some docs on this. The .NET docs are somewhat lacking in general as they were originally mostly auto generated so they don't provide much value, at least for all the Gremlin steps.

One general ask from my side: Please don't ask further questions here in this issue. Instead, use one of our community channels like Discord or the janusgraph-users mailing list. We're using GitHub issues only for bug reports or feature requests. Discussions and usage questions should be handled on one of the community channels that better support these back-and-forth type of discussions. Otherwise, it becomes hard to find the important bug reports.

tyoung12290 commented 2 years ago

@FlorianHockmann I have been running into this as well are you aware of a fix on the gremlin side in the works? Our team has been debating using GraphSON for awhile longer or switching to graphBinary and swapping properties for valueMap as you mentioned.