Github user dalaro commented on the issue:
https://github.com/apache/incubator-tinkerpop/pull/325
@spmallette You're right about overrides being the problem.
The last thing I did before opening this PR was to rebase on latest master,
when I encountered conflicts solely involving the GryoMapper registration
override feature added in d7684757734732f39970265a425b921a48d8f0fb. I
attempted to resolve those conflicts too hastily and got it wrong.
I went back to Stephen's original commit reworked my code around
registration overrides, and I now think it conforms to the old behavior
precedent. Specifically, when handling overrides of GryoMapper type
registrations, I departed from the precedent set by Stephen's commit in two
ways, both of which I've fixed locally:
* Override registrations allocated a new registration ID. Your commit used
the old ID from the registration being overridden.
* As a side effect of the preceding point, I incremented the shared
`currentSerializationId` for all registration calls. Your commit incremented
this shared counter only for non-override registration calls.
The test passes locally for me now. I'm still doing some registrator
refactoring and haven't pushed yet though.
@okram The problem with custom registrations is that GryoMapper allows the
user to provide a serializer written against shaded Kryo. This is not
compatible with Spark's KryoSerializer. I don't see a way to make it
compatible without putting fake `org.apache.tinkerpop.shaded.kryo.*` classes on
the classpath, which could get pretty ugly.
Instead, I'm adding a registrator that includes:
* all default mappings from GryoMapper (I am changing the
shaded-Kryo-serializers to be shim serializers so that they are reusable)
* the TinkerPop classes registered in GryoSerializer's constructors (but
not the scala and Spark classes registered in there)
If the user wants to register custom types while using KryoSerializer, they
can extend the registrator in a subclass, then set `spark.kryo.registrator` to
the subclass. The interface in question is `KryoRegistrator` and it has only
one method, `registerClasses(Kryo)`. It's about as simple -- maybe even less
so -- than implementing TP's `IoRegistry` interface, which has two methods.
If the user decides to continue using GryoSerializer, they can also
continue using `GryoPool.CONFIG_IO_REGISTRY`, which should still affect
GryoSerializer/HadoopPools as it always has.
WDYT?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
