tinkerpop-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Stephen Mallette <spmalle...@gmail.com>
Subject Re: KryoSerializable
Date Mon, 19 Aug 2019 15:09:13 GMT
I looked at the test code again. I think the proper method to appropriately
supporting these tests is to do as I suggested earlier in this thread where
you provide your own IoStep implementation through a custom
TraversalStrategy. I think IoStep is pretty easy to extend actually - you
would probably just need to override the detectRegistries() method to
include your own registry in the returned list (probably worth detecting if
a user added yours manually for some reason to avoid duplication).



On Sat, Aug 17, 2019 at 3:29 AM pieter martin <pieter.martin@gmail.com>
wrote:

> Hi,
>
> Ok all is good if I register the registry as you indicated.
> For now I OptOut of those test in the process suite and duplicated the
> failing tests in Sqlg.
>
> Cheers
> Pieter
>
> On Mon, 2019-08-12 at 15:40 -0400, Stephen Mallette wrote:
>
> I believe that it gets called as at some point in the test process (so what
>
> you're seeing with the RecordId getting registered makes sense), but I
>
> think that specific instance of your IORegistry does not get supplied to
>
> the IoStep and therefore it just gets a default mapper. I'd have to think
>
> if there was a way to make that test more flexible to these sorts of
>
> situations.
>
>
> On Mon, Aug 12, 2019 at 3:35 PM pieter martin <pieter.martin@gmail.com>
>
> wrote:
>
>
> The code I am calling is an copy of TinkerPop's process test that is
>
> failing on Sqlg.
>
>
> WriteTest.g_io_write_withXwrite_gryoX()
>
>
> ...
>
>
> return g.io(fileToWrite).with(IO.writer, IO.gryo).write();
>
>
> So there is no opportunity for me to manually register a io registry. That
>
> said the SqlgIoRegistryV3 does get called and registers the RecordId.
>
> Somehow later down the line it can not find it.
>
>
> But let me spend some time with the io api, and get rid of the deprecated
>
> code.
>
>
> Thanks
>
> Pieter
>
>
>
>
> On Mon, 2019-08-12 at 13:53 -0400, Stephen Mallette wrote:
>
>
> I don't think that this:
>
>
>
> traversal =  this.sqlgGraph.traversal().io(fileToWrite).write(....)
>
>
>
> does anything to check your registry that is initialized by way of your
>
>
> Graph instance. The registry must be explicitly passed as an argument using
>
>
> with() - you can see examples here:
>
>
>
> http://tinkerpop.apache.org/docs/current/reference/#io-step
>
>
>
> but  basically:
>
>
>
> g.io(someInputFile).
>
>
>     with(IO.reader, IO.gryo).
>
>
>     with(IO.registry, TinkerIoRegistryV3d0.instance())
>
>
>   read().iterate()
>
>
> g.io(someOutputFile).
>
>
>     with(IO.writer,IO.graphson).
>
>
>     with(IO.registry,
>
>
> "org.apache.tinkerpop.gremlin.tinkergraph.structure.TinkerIoRegistryV3d0")
>
>
>   write().iterate()
>
>
>
> We can't rely on the Graph instance existing anymore given that many graphs
>
>
> don't even implement that interface anymore....they just support Gremlin.
>
>
> Now, you could probably help sqlg users out if you wanted by creating a
>
>
> SqlgIoStep and auto-registering the registry for users. Then you have a
>
>
> TraversalStrategy use your step in place of TinkerPop's. Then that would
>
>
> work.
>
>
>
> On Mon, Aug 12, 2019 at 1:43 PM pieter martin <pieter.martin@gmail.com>
>
>
> wrote:
>
>
>
> It is in the SqlGraph.io(...)
>
>
>
>     @Override
>
>
>
>     public <I extends Io> I io(final Io.Builder<I> builder) {
>
>
>
>         if (builder.requiresVersion(GryoVersion.V1_0) || builder.requiresVersion(GraphSONVersion.V1_0))
>
>
>
>             return (I) builder.graph(this).onMapper(mapper -> mapper.addRegistry(SqlgIoRegistryV1.instance())).create();
>
>
>
>         else if (builder.requiresVersion(GraphSONVersion.V2_0))   // there is no gryo
v2
>
>
>
>             return (I) builder.graph(this).onMapper(mapper -> mapper.addRegistry(SqlgIoRegistryV2.instance())).create();
>
>
>
>         else
>
>
>
>             return (I) builder.graph(this).onMapper(mapper -> mapper.addRegistry(SqlgIoRegistryV3.instance())).create();
>
>
>
>     }
>
>
>
>
>     private SqlgIoRegistryV3() {
>
>
>
>         final SqlgSimpleModuleV3 sqlgSimpleModule = new SqlgSimpleModuleV3();
>
>
>
>         register(GraphSONIo.class, null, sqlgSimpleModule);
>
>
>
>         register(GryoIo.class, RecordId.class, null);
>
>
>
>     }
>
>
>
>
> I checked it is being invoked.
>
>
>
> The test I am currently running is,
>
>
>
>         loadModern();
>
>
>
>         final String fileToWrite = TestHelper.generateTempFile(WriteTest.class, "tinkerpop-modern-v3d0",
".kryo").getAbsolutePath().replace('\\', '/');
>
>
>
>         final File f = new File(fileToWrite);
>
>
>
>         assertThat(f.length() == 0, is(true));
>
>
>
>         final Traversal<Object,Object> traversal =  this.sqlgGraph.traversal().io(fileToWrite).with(IO.writer,
IO.gryo).write();
>
>
>
>
> And it breaks on kryo.getRegistration(type) where type = class
>
>
> org.umlg.sqlg.structure.RecordId
>
>
>
> public class GryoClassResolverV3d0 {
>
>
>
> ...
>
>
>
>
> public Registration writeClass(final Output output, final Class type) {
>
>
>
>     ...
>
>
>
>     final Registration registration = kryo.getRegistration(type);
>
>
>
>     ...
>
>
>
> }
>
>
>
>
> I only realized now that Graph.io() has been deprecated. Do you think that
>
>
> that is the reason?
>
>
>
> Thanks
>
>
> Pieter
>
>
>
> On Mon, 2019-08-12 at 08:52 -0400, Stephen Mallette wrote:
>
>
>
> How are you supplying that SqlgIoRegistryV3 to gryo for usage?
>
>
>
> On Mon, Aug 12, 2019 at 8:31 AM pieter martin <pieter.martin@gmail.com>
>
>
> wrote:
>
>
>
> Hi,
>
>
>
> I am upgrading Sqlg, to 3.4.1 and soon after that to 3.4.3.
>
>
>
> The process serialization tests are failing with,
>
>
>
> java.lang.IllegalArgumentException: Class is not registered:
>
>
> org.umlg.sqlg.structure.RecordId
>
>
> Note: To register this class use:
>
>
> kryo.register(org.umlg.sqlg.structure.RecordId.class);
>
>
>
> However RecordId is registered in SqlgIoRegistryV3 with
>
>
>
> register(GryoIo.class, RecordId.class, null);
>
>
>
> I notice in the provider documentation that a serializer should be
>
>
> provided.
>
>
> However RecordId implements KryoSerializable which I understood takes
>
>
> care of serializing the RecordId.
>
>
>
> I checked the CustomId class in TinkerPop's code base but it neither
>
>
> registers a serializer nor implements KryoSerializable.
>
>
>
> I get a bit lost in reading the serialization code, can you please
>
>
> point me in the right direction?
>
>
>
> Thanks
>
>
> Pieter
>
>
>
>
>
>
>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message