Skip to content

Upgrade to Kryo 4.0.0#258

Merged
johnynek merged 2 commits into
twitter:developfrom
chermenin:kryo4
Nov 6, 2016
Merged

Upgrade to Kryo 4.0.0#258
johnynek merged 2 commits into
twitter:developfrom
chermenin:kryo4

Conversation

@chermenin

Copy link
Copy Markdown
Contributor

No description provided.


def write(kser: Kryo, out: Output, obj: ClassManifest[T]) {
kser.writeObject(out, obj.erasure)
class ClassTagSerializer[T] extends KSerializer[ClassTag[T]] {

@johnynek johnynek Aug 24, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why remove the old one? This will break people, no?

@chermenin

Copy link
Copy Markdown
Contributor Author

Well, keep ClassManifestSerializer back :)

@jodersky

jodersky commented Sep 9, 2016

Copy link
Copy Markdown

+1 to Kryo 4.0.0: it seems that #253 is blocked mainly by a dependency on an unreleased version of Kryo, namely 3.1. Upgrading to 4.0.0, will unblock further work on supporting Scala 2.12 (which has just had its first RC published) and help Spark move to 2.12 as well!

@Tolsi

Tolsi commented Oct 14, 2016

Copy link
Copy Markdown

+1

1 similar comment
@alexeykiselev

Copy link
Copy Markdown

+1

@johnynek johnynek left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll look at merging this Monday and doing a release.

Sorry for the delay.

@magro

magro commented Nov 4, 2016

Copy link
Copy Markdown

@johnynek You're most probably aware of this: in kryo 4 the compatibility issue with the biggest impact IMO is probably caused by FieldSerializers changed generics handling. You could restore the former default with FieldSerializerConfig().setOptimizedGenerics(true); or make it very easy for users to set this (not sure what that means in the different contexts).

@johnynek johnynek merged commit d76009b into twitter:develop Nov 6, 2016
@johnynek

johnynek commented Nov 6, 2016

Copy link
Copy Markdown
Contributor

@magro I didn't actually know that. If I make that change as the default setting (which I can do by changing the default scala kryo instantiator here: https://github.com/twitter/chill/blob/develop/chill-scala/src/main/scala/com/twitter/chill/ScalaKryoInstantiator.scala#L56 ), do you expect that serialization of serialized objects will be compatible with 2.21, or just 3.x (were there any changes)?

To be honest, after some folks made the mistake of persisting kryo objects, the anxiety about upgrades adds cost to the point that these folks don't even want to try the upgrade.

@magro

magro commented Nov 7, 2016

Copy link
Copy Markdown

@johnynek I'd expect that serialization then is compatible with 2.21 if standard Input/Output is used. In kryo 3 the Unsafe-based IO was changed (as said in Changes 2.24.0 - 3.0.0 / Compatibility).
That's what I can say to the best of my knowledge, of course any upgrade of a serialization library deserves careful testing.

@chermenin chermenin deleted the kryo4 branch November 7, 2016 07:25
@isnotinvain

Copy link
Copy Markdown
Contributor

Just wanted to chime in here -- I think we need to discuss if we want to upgrade to kryo 4 just yet. I think we will need kryo3 support at least for quite some time. That or maybe consider shading kryo and removing it form chill's APIs or something like that.

But chill is used heavily in scalding / summingbird, and summingbird uses Storm's API, which is only just now using kryo3 as I understand it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants