TinkerPop 3.2 vs TinkerPop 3.3 Serialization difference for Predicate class

[Debasish Kanhar]

Hi All,

So as you all know I've been developing Client drivers for JanusGraph. I was able to test my initial version of library against JanusGraph 0.3.0 which uses TP 3.3.3. I now want to test the same set of driver for JanusGraph 0.2.1 which used TP 3.2.9 but I feel I've hit a bump with respect to Serialization for Predicate class.

So, my existent features like Adding of GeoShapes, Querying GeoShape and RelationIdentifier serialization/deserialization works. But any of my features leveraging TinkerPop's Predicate (PSerializer) doesn't work. I feel like this is some sort of serialization issue.

So, when I've implemented TextContainsFuzzy, which was working against JG 0.3.0/TP 3.3.3 (Using GraphSON 3.0) but the same doesn't work on GraphSON 2.0.

The following is my serialized object being sent to Gremlin-Server (Gremlin-server logs specify this)

Using TextFuzzy:

Error message:

org.apache.tinkerpop.gremlin.driver.ser.AbstractGraphSONMessageSerializerV2d0  - Request [PooledUnsafeDirectByteBuf(ridx: 303, widx: 303, cap: 337)] could not be deserialized by org.apache.tinkerpop.gremlin.driver.ser.AbstractGraphSONMessageSerializerV2d0.
org.apache.tinkerpop.shaded.jackson.databind.JsonMappingException: Could not deserialize the JSON value as required. Nested exception: org.apache.tinkerpop.shaded.jackson.databind.JsonMappingException: Could not deserialize the JSON value as required. Nested exception: java.lang.IllegalStateException: org.apache.tinkerpop.gremlin.process.traversal.P.textContainsFuzzy([Ljava.lang.Object;)

And the serialized object being sent:

{
  "requestId": {
    "@type": "g:UUID",
    "@value": "7328b342-2ed3-43a5-b589-f6688b264ee7"
  },
  "processor": "traversal",
  "op": "bytecode",
  "args": {
    "gremlin": {
      "@type": "g:Bytecode",
      "@value": {
        "step": [
          [
            "V"
          ],
          [
            "has",
            "name",
            {
              "@type": "g:P",
              "@value": {
                "predicate": "textContainsFuzzy",
                "value": "herculeas"
              }
            }
          ]
        ]
      }
    },
    "aliases": {
      "g": "gg"
    }
  }
}

The serialized object really looks good, like the way it is expected. Doesn't it? Or else I cant seem to dechiper what's wrong.

I'm also facing similar errors which using the same PSerializer class for extending Geo Predicates like geoWithin, but I guess both are tied to same problem, and if one gets solved, other will also :-)

If needed, I can attach the serialized object for geoWithin too. But any help in debugging this will be really helpful, as this is last piece of development I'm expecting before releasing version 1 of drivers with support for both GraphSON 2.0 and GraphSON 3.0

Thanks.

[Debasish Kanhar]

And, the following are serializers registered in my gremlin-server.yaml for JanusGraph 0.2.1 (I use the default packeged set of serializers):

serializers:
  - { className: org.apache.tinkerpop.gremlin.driver.ser.GryoMessageSerializerV1d0, config: { ioRegistries: [org.janusgraph.graphdb.tinkerpop.JanusGraphIoRegistry] }}
  - { className: org.apache.tinkerpop.gremlin.driver.ser.GryoLiteMessageSerializerV1d0, config: {ioRegistries: [org.janusgraph.graphdb.tinkerpop.JanusGraphIoRegistry] }}
  - { className: org.apache.tinkerpop.gremlin.driver.ser.GryoMessageSerializerV1d0, config: { serializeResultToString: true }}
  - { className: org.apache.tinkerpop.gremlin.driver.ser.GraphSONMessageSerializerGremlinV1d0, config: { ioRegistries: [org.janusgraph.graphdb.tinkerpop.JanusGraphIoRegistryV1d0] }}
  - { className: org.apache.tinkerpop.gremlin.driver.ser.GraphSONMessageSerializerGremlinV2d0, config: { ioRegistries: [org.janusgraph.graphdb.tinkerpop.JanusGraphIoRegistry] }}
  - { className: org.apache.tinkerpop.gremlin.driver.ser.GraphSONMessageSerializerV1d0, config: { ioRegistries: [org.janusgraph.graphdb.tinkerpop.JanusGraphIoRegistryV1d0] }}

[Florian Hockmann]

JanusGraph simply lacks the required GraphSON serializers and deserializers for predicates. I created #1060 to track this issue and fixed it with PR #1061 which was merged into master and not into the 0.2 branch and is therefore not present in version 0.2.1.

That is also why I decided to only support JanusGraph 0.3.0 with JanusGraph.Net.

[Debasish Kanhar]

Hi Florian,

Thanks for the link to PR. Didn't know I had stumbled upon the same issue already. Oops :-P

Anyways, quick questions, are there any plan to merge those changes to 0.2 branch? Reason I'm asking is because I feel a lot of users still haven't migrated to 0.3 series of JanusGraph, meaning that the drivers will still remain far from being usable. I did see your code changes in the PR, and they seem like few changes, which can be implemented. But wanted to know what are plans for merging it to 0.2 branch, and if no one else is working, I can take that up. Also I'll need a small walkthrough on what things need to be done for the same.

Also, I've create a PR #2 to push initial version of library to official repo. I was unable to add You, Jason, or Misha to reviewers. Maybe because I'm not committer? Can you check that out. We will need your comments to push that out.

[Florian Hockmann]

I'm not against merging this into 0.2, but I'm actually not sure whether we want to do that. My understanding is that new features go mainly into master and that the 0.2 branch mostly gets bug fixes and small improvements.

Users should upgrade to 0.3.0 in my opinion when they want to use new features like JanusGraph predicates in languages like Python and C#.

Also I'll need a small walkthrough on what things need to be done for the same.

I'm actually not sure about the workflow myself. Usually, PRs are merged into a feature branch and then from there up into master and not the other way around. One solution is probably to simply cherry-pick the commit. Can someone else shed some light on this?

I was unable to add You, Jason, or Misha to reviewers. Maybe because I'm not committer?

Yes, commit permissions are necessary to add reviewers as far as I know. But I will try to find some time to review this.

[Jason Plurad]

Apologies for the slow response. Back from a long holiday weekend in the USA.

My opinion is that we need to keep pushing forward on releases and actively encourage users to move up to the latest release. The JanusGraph policy has been for new features to go into the master branch only. If there are products out there in production on JanusGraph 0.2, they aren't using GLVs or are using the plain TinkerPop GLVs and decided that was good enough.

The new dotnet and python drivers are separate from the main repository, so it is possible that they could have a different approach to what JanusGraph releases they want to support. However, I'd suggest that we need to continue sticking with primarily supporting the latest release, especially since GLVs are still a relatively new idea in TinkerPop. Managing the dependency differences between the TinkerPop versions is not worth the extra effort, especially considering that TinkerPop 3.4 is on the horizon.

We haven't been assigning reviewers to specific PRs. As a primarily volunteer community, we rely on committers going to the queue and reviewing what is out there. We discussed the idea of the PR queue email to the mailing list previously, and I will get that started this week. It is especially important now that the PRs are spread across several repositories.

I created issue 1243 to update the docs on the PR workflow.