[SPARK-42478] Make a serializable jobTrackerId instead of a non-serializable JobID in FileWriterFactory #40064

yikf · 2023-02-17T12:05:37Z

What changes were proposed in this pull request?

Make a serializable jobTrackerId instead of a non-serializable JobID in FileWriterFactory

Why are the changes needed?

SPARK-41448 make consistent MR job IDs in FileBatchWriter and FileFormatWriter, but it breaks a serializable issue, JobId is non-serializable.

Does this PR introduce any user-facing change?

No

How was this patch tested?

GA

yikf · 2023-02-17T12:07:01Z

@cloud-fan @boneanxs could you please take a look if you find a time?

cloud-fan · 2023-02-21T04:17:50Z

@yikf can you provide a test case, or at least the error stacktrace you hit in your environment?

yikf · 2023-02-21T04:54:53Z

@cloud-fan This case is the error that Apache kyuubi encountered when upgrading from spark 3.3.1 to 3.3.2, can see this link to find the error stacktrace.

sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/FileWriterFactory.scala

yikf · 2023-02-21T06:35:32Z

updated, verified w/ kyuubi on spark 3.3.2 and all tests passed

build/mvn clean test -pl :kyuubi-spark-connector-hive_2.12 -am -Pspark-3.3 -Dspark.version=3.3.2

boneanxs

LGTM

yikf · 2023-02-23T02:14:28Z

kindly ping @cloud-fan , @boneanxs Any suggestions?

mridulm · 2023-02-23T03:22:25Z

I have not followed the changes in this part of the code too much in a while - but this specific PR will result in a different jobId each time the class is deserialized - I would expect that to cause issues with output formats ?

yikf · 2023-02-23T03:40:59Z

@mridulm Thanks your review, this is a nice question for me, JobId maybe is different when each time the class is deserialized.

How about this idea that SparkHadoopWriterUtils.createJobTrackerID to generate an ID for a job tracker, and the job tracker is unique, JobId is constructed using a unique tackerid when the class is deserialized.

mridulm · 2023-02-23T04:01:52Z

@yikf Agree - we only specify two parts for the JobID - the String jtIdentifier and int id.
We can persist those in the class - and make jobId a transient lazy val which recreates it each time.

Something like this instead:


  private[this] val jobIdParts: (String, Int) = {
    val id = SparkHadoopWriterUtils.createJobID(new Date, 0)
    (id.getJtIdentifier, id.getId)
  }

  @transient private lazy val jobId = new JobID(jobIdParts._1, jobIdParts._2)

Thoughts ?

…in FileWriterFactory

yikf · 2023-02-23T08:38:26Z

@mridulm Nice suggestion, and we can simplify to as follow since int id is unique.

  private[this] val jobTrackerID = SparkHadoopWriterUtils.createJobTrackerID(new Date)
  @transient private lazy val jobId = new JobID(jobTrackerID, 0)

sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/FileWriterFactory.scala

cloud-fan · 2023-02-27T08:55:40Z

thanks, merging to master/3.4!

…lizable JobID in FileWriterFactory ### What changes were proposed in this pull request? Make a serializable jobTrackerId instead of a non-serializable JobID in FileWriterFactory ### Why are the changes needed? [SPARK-41448](https://issues.apache.org/jira/browse/SPARK-41448) make consistent MR job IDs in FileBatchWriter and FileFormatWriter, but it breaks a serializable issue, JobId is non-serializable. ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? GA Closes #40064 from Yikf/write-job-id. Authored-by: Yikf <yikaifei@apache.org> Signed-off-by: Wenchen Fan <wenchen@databricks.com> (cherry picked from commit d46b15d) Signed-off-by: Wenchen Fan <wenchen@databricks.com>

dongjoon-hyun · 2023-03-03T20:19:08Z

Hi, @cloud-fan . SPARK-41448 landed to master/3.3/3.2 and this is merge this to master/3.4 only. I'm wondering if we are planning backporting to branch-3.3 and 3.2.

[SPARK-41448] Make consistent MR job IDs in FileBatchWriter and FileFormatWriter #38980 (comment)

dongjoon-hyun · 2023-03-03T20:20:02Z

Also, cc @sunchao

cloud-fan · 2023-03-06T02:27:19Z

@yikf can you help to open a backport PR for 3.2/3.3? Thanks!

yikf · 2023-03-06T03:05:02Z

@yikf can you help to open a backport PR for 3.2/3.3? Thanks!

Sure

… non-serializable JobID in FileWriterFactory This is a backport of #40064 for branch-3.3 ### What changes were proposed in this pull request? Make a serializable jobTrackerId instead of a non-serializable JobID in FileWriterFactory ### Why are the changes needed? [SPARK-41448](https://issues.apache.org/jira/browse/SPARK-41448) make consistent MR job IDs in FileBatchWriter and FileFormatWriter, but it breaks a serializable issue, JobId is non-serializable. ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? GA Closes #40290 from Yikf/backport-SPARK-42478-3.3. Authored-by: Yikf <yikaifei@apache.org> Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>

… non-serializable JobID in FileWriterFactory This is a backport of #40064 for branch-3.2 ### What changes were proposed in this pull request? Make a serializable jobTrackerId instead of a non-serializable JobID in FileWriterFactory ### Why are the changes needed? [SPARK-41448](https://issues.apache.org/jira/browse/SPARK-41448) make consistent MR job IDs in FileBatchWriter and FileFormatWriter, but it breaks a serializable issue, JobId is non-serializable. ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? GA Closes #40289 from Yikf/backport-SPARK-42478-3.2. Authored-by: Yikf <yikaifei@apache.org> Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>

… non-serializable JobID in FileWriterFactory This is a backport of apache#40064 for branch-3.2 ### What changes were proposed in this pull request? Make a serializable jobTrackerId instead of a non-serializable JobID in FileWriterFactory ### Why are the changes needed? [SPARK-41448](https://issues.apache.org/jira/browse/SPARK-41448) make consistent MR job IDs in FileBatchWriter and FileFormatWriter, but it breaks a serializable issue, JobId is non-serializable. ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? GA Closes apache#40289 from Yikf/backport-SPARK-42478-3.2. Authored-by: Yikf <yikaifei@apache.org> Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>

…lizable JobID in FileWriterFactory ### What changes were proposed in this pull request? Make a serializable jobTrackerId instead of a non-serializable JobID in FileWriterFactory ### Why are the changes needed? [SPARK-41448](https://issues.apache.org/jira/browse/SPARK-41448) make consistent MR job IDs in FileBatchWriter and FileFormatWriter, but it breaks a serializable issue, JobId is non-serializable. ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? GA Closes apache#40064 from Yikf/write-job-id. Authored-by: Yikf <yikaifei@apache.org> Signed-off-by: Wenchen Fan <wenchen@databricks.com> (cherry picked from commit d46b15d) Signed-off-by: Wenchen Fan <wenchen@databricks.com>

…ent task attempts ### What changes were proposed in this pull request? After #40064 , we always get the same TaskAttemptId for different task attempts which has the same partitionId. This would lead different task attempts write to the same directory. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? GA ### Was this patch authored or co-authored using generative AI tooling? No. Closes #46811 from jackylee-ch/fix_v2write_use_same_directories_for_different_task_attempts. Lead-authored-by: jackylee-ch <lijunqing@baidu.com> Co-authored-by: Kent Yao <yao@apache.org> Signed-off-by: yangjie01 <yangjie01@baidu.com>

…ent task attempts ### What changes were proposed in this pull request? After #40064 , we always get the same TaskAttemptId for different task attempts which has the same partitionId. This would lead different task attempts write to the same directory. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? GA ### Was this patch authored or co-authored using generative AI tooling? No. Closes #46811 from jackylee-ch/fix_v2write_use_same_directories_for_different_task_attempts. Lead-authored-by: jackylee-ch <lijunqing@baidu.com> Co-authored-by: Kent Yao <yao@apache.org> Signed-off-by: yangjie01 <yangjie01@baidu.com> (cherry picked from commit 67d11b1) Signed-off-by: yangjie01 <yangjie01@baidu.com>

…ent task attempts ### What changes were proposed in this pull request? After apache#40064 , we always get the same TaskAttemptId for different task attempts which has the same partitionId. This would lead different task attempts write to the same directory. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? GA ### Was this patch authored or co-authored using generative AI tooling? No. Closes apache#46811 from jackylee-ch/fix_v2write_use_same_directories_for_different_task_attempts. Lead-authored-by: jackylee-ch <lijunqing@baidu.com> Co-authored-by: Kent Yao <yao@apache.org> Signed-off-by: yangjie01 <yangjie01@baidu.com>

github-actions bot added the SQL label Feb 17, 2023

yikf force-pushed the write-job-id branch 2 times, most recently from 931e76d to 61f4592 Compare February 20, 2023 08:48

cloud-fan reviewed Feb 21, 2023

View reviewed changes

sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/FileWriterFactory.scala Show resolved Hide resolved

yikf force-pushed the write-job-id branch from 61f4592 to 8a0c4f7 Compare February 21, 2023 06:34

boneanxs approved these changes Feb 21, 2023

View reviewed changes

cloud-fan approved these changes Feb 21, 2023

View reviewed changes

Make a serializable jobTrackerId instead of a non-serializable JobID …

e5c76d1

…in FileWriterFactory

yikf force-pushed the write-job-id branch from 8a0c4f7 to e5c76d1 Compare February 23, 2023 08:38

mridulm reviewed Feb 23, 2023

View reviewed changes

sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/FileWriterFactory.scala Outdated Show resolved Hide resolved

mridulm approved these changes Feb 23, 2023

View reviewed changes

nit

d8d9055

cloud-fan reviewed Feb 23, 2023

View reviewed changes

sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/FileWriterFactory.scala Show resolved Hide resolved

add comments

9c0b221

cloud-fan closed this in d46b15d Feb 27, 2023

This was referenced Mar 6, 2023

[SPARK-42478][SQL][3.2] Make a serializable jobTrackerId instead of a non-serializable JobID in FileWriterFactory #40289

Closed

[SPARK-42478][SQL][3.3] Make a serializable jobTrackerId instead of a non-serializable JobID in FileWriterFactory #40290

Closed

jackylee-ch mentioned this pull request May 30, 2024

[SPARK-48484][SQL] Fix: V2Write use the same TaskAttemptId for different task attempts #46811

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-42478] Make a serializable jobTrackerId instead of a non-serializable JobID in FileWriterFactory #40064

[SPARK-42478] Make a serializable jobTrackerId instead of a non-serializable JobID in FileWriterFactory #40064

yikf commented Feb 17, 2023

yikf commented Feb 17, 2023

cloud-fan commented Feb 21, 2023

yikf commented Feb 21, 2023

yikf commented Feb 21, 2023

boneanxs left a comment

yikf commented Feb 23, 2023

mridulm commented Feb 23, 2023 •

edited

Loading

yikf commented Feb 23, 2023

mridulm commented Feb 23, 2023 •

edited

Loading

yikf commented Feb 23, 2023

cloud-fan commented Feb 27, 2023

dongjoon-hyun commented Mar 3, 2023

dongjoon-hyun commented Mar 3, 2023

cloud-fan commented Mar 6, 2023

yikf commented Mar 6, 2023

[SPARK-42478] Make a serializable jobTrackerId instead of a non-serializable JobID in FileWriterFactory #40064

[SPARK-42478] Make a serializable jobTrackerId instead of a non-serializable JobID in FileWriterFactory #40064

Conversation

yikf commented Feb 17, 2023

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

yikf commented Feb 17, 2023

cloud-fan commented Feb 21, 2023

yikf commented Feb 21, 2023

yikf commented Feb 21, 2023

boneanxs left a comment

Choose a reason for hiding this comment

yikf commented Feb 23, 2023

mridulm commented Feb 23, 2023 • edited Loading

yikf commented Feb 23, 2023

mridulm commented Feb 23, 2023 • edited Loading

yikf commented Feb 23, 2023

cloud-fan commented Feb 27, 2023

dongjoon-hyun commented Mar 3, 2023

dongjoon-hyun commented Mar 3, 2023

cloud-fan commented Mar 6, 2023

yikf commented Mar 6, 2023

mridulm commented Feb 23, 2023 •

edited

Loading

mridulm commented Feb 23, 2023 •

edited

Loading