Skip to content

Conversation

@ianoc
Copy link
Contributor

@ianoc ianoc commented Jul 11, 2014

Adds in an option into the kryo serializer to enable requiring registration

Adds a resource pool in the SparkSqlSerializer code paths to speed up serialization of generic types.

@AmplabJenkins
Copy link

Can one of the admins verify this patch?

@marmbrus
Copy link
Contributor

test this please

@SparkQA
Copy link

SparkQA commented Jul 14, 2014

QA tests have started for PR 1377. This patch merges cleanly.
View progress: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/16610/consoleFull

@SparkQA
Copy link

SparkQA commented Jul 14, 2014

QA results for PR 1377:
- This patch PASSES unit tests.
- This patch merges cleanly
- This patch adds no public classes

For more information see test ouptut:
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/16610/consoleFull

@marmbrus
Copy link
Contributor

@pwendell, any thoughts on the additional option for kryo?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hey @ianoc - I found the wording here a bit confusing when reading it over. It would be good to say the exact behavior of Kryo if this is set to true (i.e. will it throw an exception if an unregistered class is found?). Also, the this in the third sentence, it's a bit ambigugous whether that refers to the registration itself or the default setting of this value. A slight re-word would be :

Whether to require registration with Kryo. If set to 'true', Kryo will throw an exception
if an unregistered class is serialized. If set to false (the default), Kryo will write
unregistered class names along with each object. Writing class names can cause 
significant performance overhead, so enabling this option can enforce strictly that a
user has not omitted classes from registration.

@ianoc
Copy link
Contributor Author

ianoc commented Jul 22, 2014

@pwendell Sounds good to me, updated as per your suggestion.

@marmbrus
Copy link
Contributor

test this please

@SparkQA
Copy link

SparkQA commented Jul 23, 2014

QA tests have started for PR 1377. This patch merges cleanly.
View progress: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/17062/consoleFull

@SparkQA
Copy link

SparkQA commented Jul 23, 2014

QA results for PR 1377:
- This patch PASSES unit tests.
- This patch merges cleanly
- This patch adds the following public classes (experimental):
if an unregistered class is serialized. If set to false (the default), Kryo will write

For more information see test ouptut:
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/17062/consoleFull

@marmbrus
Copy link
Contributor

Thanks! I've merged this into master.

@asfgit asfgit closed this in efdaeb1 Jul 23, 2014
@ianoc
Copy link
Contributor Author

ianoc commented Jul 23, 2014

Excellent thanks

xiliu82 pushed a commit to xiliu82/spark that referenced this pull request Sep 4, 2014
… use a resource pool in Spark SQL for Kryo instances.

Author: Ian O Connell <[email protected]>

Closes apache#1377 from ianoc/feature/SPARK-2102 and squashes the following commits:

5498566 [Ian O Connell] Docs update suggested by Patrick
20e8555 [Ian O Connell] Slight style change
f92c294 [Ian O Connell] Add docs for new KryoSerializer option
f3735c8 [Ian O Connell] Add using a kryo resource pool for the SqlSerializer
4e5c342 [Ian O Connell] Register the SparkConf for kryo, it gets swept into serialization
665805a [Ian O Connell] Add a spark.kryo.registrationRequired option for configuring the Kryo Serializer
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.

5 participants