Fwd: serialization of sortedmap

69 views
Skip to first unread message

Koert Kuipers

unread,
Oct 11, 2013, 5:57:16 PM10/11/13
to chill...@googlegroups.com
I tried to get SortedMap (or really just TreeMap) to behave but i don't think i can do it with the default TraversableSerializer. the issue is that CanBuildFrom needs an Ordering for any SortedMap, which depends on the class of the key and could be custom too. So i wrote a Serializer that includes the Ordering. Any better ideas?


Also looking at the KryoSpec i noticed the main test is this:
serdeser must be_==(orig)
Should we also require the classes to be the same? As in:
serdeser.getClass.asInstanceOf[Class[Any]]) must be_=(orig.getClass.asInstanceOf[Class[Any]])

i was looking at the current tests and i noticed this is true for all tests including one i added for a TreeMap and a SortedMap, except for a Range that comes out as a Vector somehow.

 





---------- Forwarded message ----------
From: Oscar Boykin <os...@twitter.com>
Date: Wed, Oct 2, 2013 at 1:12 PM
Subject: Re: serialization of sortedmap
To: Koert Kuipers <ko...@tresata.com>
Cc: "scaldi...@googlegroups.com" <scaldi...@googlegroups.com>


Yes. There are some questionable use of default serializers in chill I am noticing.

You cannot use a default serializer for a class like iterable, map, etc...

We need to remove those.  You can also configure the Kryo in the code using the ConfiguredInstantiator from chill (Job does this).


On Wed, Oct 2, 2013 at 10:05 AM, Koert Kuipers <ko...@tresata.com> wrote:
i can see the issue in chill-scala now i think. will move conversation to mailing list for chill




On Wed, Oct 2, 2013 at 12:39 PM, Koert Kuipers <ko...@tresata.com> wrote:
we are seeing one new issue with the 0.9 branch that we did not see before i think. before we were also on the dev branch but it has not been synced for a few months.

we are getting blowups in reducers like this:
Caused by: java.lang.ClassCastException: scala.collection.immutable.Map$Map1 cannot be cast to scala.collection.immutable.SortedMap

or

Caused by: java.lang.ClassCastException: scala.collection.immutable.Map$Map2 cannot be cast to scala.collection.immutable.SortedMap

it seems some SortedMaps comes out of serialization with the wrong class?

the classes we serialize are case classes like this:
case class X[T](val y: Option[scala.collection.immutable.SortedMap[Long, Set[T]]])


--
You received this message because you are subscribed to the Google Groups "Scalding Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scalding-dev...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.



--
Oscar Boykin :: @posco :: http://twitter.com/posco

Koert Kuipers

unread,
Oct 19, 2013, 3:32:38 PM10/19/13
to chill...@googlegroups.com, Oscar Boykin
any opinion?

Oscar Boykin

unread,
Oct 19, 2013, 4:41:48 PM10/19/13
to Koert Kuipers, chill...@googlegroups.com
Sorry I missed this.

1) Yes, agree we should check Class identity also (I bet this makes some tests break, so that's going to be work to clean up).
2) Yes, SortedMap/Set will probably need special handling. I think a custom serializer is the way to go.
3) The current practice of registering a default serializer for things like Seq/Iterable is a really broken idea, and should probably be removed. You can write with those interfaces, but not read. Let's remove those and let the fields serializer handle it (and if it fails, fix that).
4) We need randomly generated object (non-tree) graphs (with scala check I think) and verify that they round-trip. I think the current tests are too weak.

Koert Kuipers

unread,
Oct 23, 2013, 1:01:21 PM10/23/13
to Oscar Boykin, chill...@googlegroups.com
got it i will try to create a pull request when i have some time to address some of these issues.

what exactly is wrong with the default serializers for Seq/Iterable? why do they not deserialize correctly? t thought the actual class name was stored with the serialized data (this kryo stuff is somewhat new to me).
Reply all
Reply to author
Forward
0 new messages