[Core | Spark | Integrations] : Fix kryo serialization failure for FileIO by singhpk234 · Pull Request #5437 · apache/iceberg

singhpk234 · 2022-08-04T12:08:29Z

About the change

presently spark queries fails when using S3fileIO & GlueCatalog when being used with KryoSerializer ref #5414 (comment). This happens because Immutable map part of S3FileIO properties is not serializable with Kryo and seralizers for it not available in Twitter Chill lib as well (which also spark uses). This PR attemps to fix this by using java collections instead which spark will be able to ser/de as it's serializer is present in Chill lib.

Solves #5414

Testing Done

Manual Test

UT to validate the change fix the issue, without the fix the UT fails with exception mentioned in ticket #5414 (comment)

kbendick

This LGTM.

You might want to put a small comment in the code above the creation of tableSpecificCatalogProperties and mention that ImmutableMap isn't used to support Kryo serde as that's the normal style.

Totally up to you though.

rdblue

I don't think this is the right way to fix the problem.

The approach in this PR is to avoid passing ImmutableMap into initializeFileIO, but that makes S3FileIO vulnerable to this problem any time the caller doesn't know not to pass that type of map. Instead, S3FileIO should be responsible for making a copy of the incoming map that can be serialized with Kryo.

rdblue · 2022-08-07T20:27:10Z

This looks like the right fix to me. Thanks, @singhpk234! Could you also add tests for the other FileIO implementations?

nastra

LGTM, but I would define the version number for com.twitter:chill-java:0.10.0 in versions.propsand have build.gradle only use the actual artifact (without the version): testImplementation 'com.twitter:chill-java

rdblue · 2022-09-01T22:37:22Z

Thanks, @singhpk234!

singhpk234 · 2022-09-02T05:23:58Z

Thanks @rdblue @kbendick @nastra @amogh-jahagirdar for your awesome reviews :) !!!

akshay-kokate-06 · 2023-04-13T09:30:44Z

@singhpk234 I am still facing this issue using iceberg-spark-runtime-3.3_2.12:1.2.0 and software.amazon.awssdk:bundle:2.20.18

nastra · 2023-04-14T08:48:55Z

@akshay-kokate-06 what issue are you seeing exactly? Could you provide the stack trace please?

github-actions Bot added the AWS label Aug 4, 2022

singhpk234 marked this pull request as draft August 4, 2022 13:12

singhpk234 force-pushed the fix/kryo-serialization branch 2 times, most recently from b7cc12c to 3532aa3 Compare August 4, 2022 13:22

singhpk234 marked this pull request as ready for review August 4, 2022 14:44

singhpk234 changed the title ~~AWS: Fix kryo serialization for S3 FileIO~~ AWS: Fix kryo serialization failure for S3 FileIO Aug 4, 2022

kbendick approved these changes Aug 4, 2022

View reviewed changes

kbendick reviewed Aug 4, 2022

View reviewed changes

Comment thread aws/src/main/java/org/apache/iceberg/aws/glue/GlueCatalog.java Outdated

github-actions Bot added the build label Aug 7, 2022

singhpk234 force-pushed the fix/kryo-serialization branch 2 times, most recently from e058629 to 95bc117 Compare August 7, 2022 11:57

rdblue requested changes Aug 7, 2022

View reviewed changes

rdblue reviewed Aug 7, 2022

View reviewed changes

Comment thread aws/src/test/java/org/apache/iceberg/aws/glue/TestGlueCatalog.java Outdated

singhpk234 force-pushed the fix/kryo-serialization branch from 95bc117 to ecedfc7 Compare August 7, 2022 19:04

github-actions Bot added core GCP labels Aug 7, 2022

singhpk234 requested a review from rdblue August 7, 2022 19:21

rdblue reviewed Aug 7, 2022

View reviewed changes

Comment thread aws/src/test/java/org/apache/iceberg/aws/s3/TestS3FileIO.java Outdated

rdblue approved these changes Aug 7, 2022

View reviewed changes

rdblue added this to the Iceberg 0.14.1 Release milestone Aug 7, 2022

github-actions Bot added the spark label Aug 8, 2022

singhpk234 force-pushed the fix/kryo-serialization branch 3 times, most recently from 8f40d83 to 159e84a Compare August 8, 2022 10:49

nastra approved these changes Aug 8, 2022

View reviewed changes

Comment thread build.gradle Outdated

amogh-jahagirdar approved these changes Aug 9, 2022

View reviewed changes

rdblue reviewed Aug 10, 2022

View reviewed changes

Comment thread spark/v3.3/build.gradle Outdated

rdblue reviewed Aug 10, 2022

View reviewed changes

Comment thread build.gradle Outdated

rdblue reviewed Aug 10, 2022

View reviewed changes

Comment thread spark/v3.3/spark/src/test/java/org/apache/iceberg/TestFileIOSerialization.java Outdated

Prashant Singh added 3 commits September 1, 2022 13:36

Address review feedback - round 2

b067303

Address review feedback - round 3 (Add more UTs)

8865770

Address review feedback - round4

25e9238

singhpk234 force-pushed the fix/kryo-serialization branch from 1f3d860 to 3aef768 Compare September 1, 2022 08:07

github-actions Bot added the API label Sep 1, 2022

singhpk234 force-pushed the fix/kryo-serialization branch from 3aef768 to 6118aa8 Compare September 1, 2022 08:36

address review feedbacks

58e2082

singhpk234 force-pushed the fix/kryo-serialization branch from 6118aa8 to 58e2082 Compare September 1, 2022 08:39

singhpk234 changed the title ~~AWS: Fix kryo serialization failure for S3 FileIO~~ [Core | Spark | Integrations] : Fix kryo serialization failure for FileIO Sep 1, 2022

rdblue reviewed Sep 1, 2022

View reviewed changes

Comment thread build.gradle Outdated

add kryo as testImplementation in all modules required

7d6577f

rdblue reviewed Sep 1, 2022

View reviewed changes

Comment thread aws/src/main/java/org/apache/iceberg/aws/s3/S3FileIO.java Outdated

rdblue reviewed Sep 1, 2022

View reviewed changes

Comment thread spark/v3.3/spark/src/test/java/org/apache/iceberg/TestFileIOSerialization.java Outdated

remove unnecessary code

aedb696

rdblue approved these changes Sep 1, 2022

View reviewed changes

rdblue merged commit ec9643e into apache:master Sep 1, 2022

singhpk234 deleted the fix/kryo-serialization branch September 2, 2022 05:24

rdblue pushed a commit to rdblue/iceberg that referenced this pull request Sep 2, 2022

Core, AWS: Fix Kryo serialization failure for FileIO (apache#5437)

2dcbdbd

rdblue pushed a commit to rdblue/iceberg that referenced this pull request Sep 2, 2022

Core, AWS: Fix Kryo serialization failure for FileIO (apache#5437)

5f1fdc9

rdblue pushed a commit to rdblue/iceberg that referenced this pull request Sep 2, 2022

Core, AWS: Fix Kryo serialization failure for FileIO (apache#5437)

a17ad21

rdblue pushed a commit to rdblue/iceberg that referenced this pull request Sep 2, 2022

Core, AWS: Fix Kryo serialization failure for FileIO (apache#5437)

19c4ed9

rdblue pushed a commit to rdblue/iceberg that referenced this pull request Sep 2, 2022

Core, AWS: Fix Kryo serialization failure for FileIO (apache#5437)

510e842

rdblue pushed a commit that referenced this pull request Sep 3, 2022

Core, AWS: Fix Kryo serialization failure for FileIO (#5437)

c171830

rizaon mentioned this pull request Sep 6, 2022

core: Provide mechanism to cache manifest file content #4518

Merged

sunchao pushed a commit to sunchao/iceberg that referenced this pull request May 10, 2023

Core, AWS: Fix Kryo serialization failure for FileIO (apache#5437)

5990ae5

zhongyujiang pushed a commit to zhongyujiang/iceberg that referenced this pull request Apr 16, 2025

Core, AWS: Fix Kryo serialization failure for FileIO (apache#5437)

ddf5768

Uh oh!

Conversation

singhpk234 commented Aug 4, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

About the change

Testing Done

Uh oh!

kbendick left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rdblue left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

rdblue commented Aug 7, 2022

Uh oh!

nastra left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rdblue commented Sep 1, 2022

Uh oh!

singhpk234 commented Sep 2, 2022

Uh oh!

akshay-kokate-06 commented Apr 13, 2023

Uh oh!

nastra commented Apr 14, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

singhpk234 commented Aug 4, 2022 •

edited

Loading