Skip to content

Conversation

@dongjoon-hyun
Copy link
Member

@dongjoon-hyun dongjoon-hyun commented Jan 18, 2025

What changes were proposed in this pull request?

This PR aims to upgrade Avro to 1.11.4.

Why are the changes needed?

To bring the latest bug fixes like the following.

Does this PR introduce any user-facing change?

Yes, this is a dependency change.

How was this patch tested?

Pass the CIs.

Was this patch authored or co-authored using generative AI tooling?

No.

@dongjoon-hyun
Copy link
Member Author

cc @LuciferYang

@dongjoon-hyun
Copy link
Member Author

Could you review this PR, @panbingkun ?

LuciferYang
LuciferYang previously approved these changes Jan 19, 2025
Copy link
Contributor

@LuciferYang LuciferYang left a comment

Choose a reason for hiding this comment

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

LGTM

@LuciferYang
Copy link
Contributor

[error] 
[error]   bad constant pool index: 0 at pos: 3287
[error]      while compiling: /home/runner/work/spark/spark/sql/hive/src/main/scala/org/apache/spark/sql/hive/client/IsolatedClientLoader.scala
[error]         during phase: globalPhase=typer, enteringPhase=namer
[error]      library version: version 2.13.8
[error]     compiler version: version 2.13.8
[error]   reconstructed args: -bootclasspath /usr/lib/jvm/temurin-8-jdk-amd64/jre/lib/resources.jar:/usr/lib/jvm/temurin-8-jdk-amd64/jre/lib/rt.jar:/usr/lib/jvm/temurin-8-jdk-amd64/jre/lib/sunrsasign.jar:/usr/lib/jvm/temurin-8-jdk-amd64/jre/lib/jsse.jar:/usr/lib/jvm/temurin-8-jdk-amd64/jre/lib/jce.jar:/usr/lib/jvm/temurin-8-jdk-amd64/jre/lib/charsets.jar:/usr/lib/jvm/temurin-8-jdk-amd64/jre/lib/jfr.jar:/usr/lib/jvm/temurin-8-jdk-amd64/jre/classes:/home/runner/.cache/coursier/v1/https/maven-central.storage-download.googleapis.com/maven2/org/scala-lang/scala-library/2.13.8/scala-library-2.13.8.jar -sourcepath /home/runner/work/spark/spark -explaintypes -feature -unchecked -Wconf:cat=unused-imports&src=org\/apache\/spark\/graphx\/impl\/VertexPartitionBaseOps.scala:s -Wconf:cat=unused-imports&src=org\/apache\/spark\/graphx\/impl\/VertexPartitionBase.scala:s -Wconf:msg=^(?=.*?a value of type)(?=.*?cannot also be).+$:s -Wconf:cat=unchecked&msg=eliminated by erasure:s -Wconf:cat=unchecked&msg=outer reference:s -Wconf:cat=deprecation&msg=procedure syntax is deprecated:e -Wconf:msg=method without a parameter list overrides a method with a single empty one:s -Wconf:msg=method with a single empty parameter list overrides method without any parameter list:s -Wconf:msg=Auto-application to \`\(\)\` is deprecated:s -Wconf:msg=^(?=.*?Widening conversion from)(?=.*?is deprecated because it loses precision).+$:s -Wconf:msg=^(?=.*?method|value|type|object|trait|inheritance)(?=.*?deprecated)(?=.*?since 2.13).+$:s -Wconf:cat=other-pure-statement&site=org.apache.spark.sql.streaming.sources.StreamingDataSourceV2Suite.testPositiveCase.\$anonfun:wv -Wconf:cat=other-pure-statement&site=org.apache.spark.scheduler.OutputCommitCoordinatorSuite.<local OutputCommitCoordinatorSuite>.futureAction:wv -Wconf:cat=other-pure-statement&site=org.apache.spark.streaming.util.FileBasedWriteAheadLog.readAll.readFile:wv -Wconf:cat=other-match-

The compilation for Scala 2.13 failed. Can we re-trigger it to confirm if it was just a random occurrence?

@LuciferYang LuciferYang dismissed their stale review January 19, 2025 02:43

The compilation for Scala 2.13 failed.

@LuciferYang
Copy link
Contributor

LuciferYang commented Jan 19, 2025

I tested it locally and encountered the same error.
image

image

@dongjoon-hyun
Copy link
Member Author

Oh.. Let me check.

@LuciferYang
Copy link
Contributor

I tried it out and found that 1.11.3 can be compiled successfully. The issue likely stems from 1.11.4.

@dongjoon-hyun
Copy link
Member Author

Oh, surprising.

Actually, 1.11.4 was requested last year.

Let me dig more.

@dongjoon-hyun
Copy link
Member Author

The root cause seems due to the SBT dependency bug. I pinned commons-compress version in SparkBuild.scala and verified Scala 2.13 compilation. Let's see the CI result, @LuciferYang . Thank you!

$ dev/change-scala-version.sh 2.13
$ build/sbt package -Pscala-2.13
...
[success] Total time: 88 s (01:28), completed Jan 18, 2025, 7:41:30 PM

@dongjoon-hyun
Copy link
Member Author

BTW, this patch and the pinning is only for branch-3.5 because branch-3.5 has old Scala version, 2.13.8.

@LuciferYang
Copy link
Contributor

It seems so. I tried compiling with Maven, and it worked fine at commit 86dfeae. I also tried compiling the latest commit using sbt, and it worked as well.

Copy link
Contributor

@LuciferYang LuciferYang left a comment

Choose a reason for hiding this comment

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

+1, LGTM
pending tests

@dongjoon-hyun
Copy link
Member Author

Thank you always for your deep review and co-investigation!

@dongjoon-hyun
Copy link
Member Author

All tests passed. Merged to branch-3.5.

dongjoon-hyun added a commit that referenced this pull request Jan 19, 2025
### What changes were proposed in this pull request?

This PR aims to upgrade Avro to 1.11.4.

### Why are the changes needed?

To bring the latest bug fixes like the following.
- https://github.com/apache/avro/releases/tag/release-1.11.3
    - apache/avro#2432
    - apache/avro#2318
    - apache/avro#1720
- https://github.com/apache/avro/releases/tag/release-1.11.4
    - apache/avro#2980

### Does this PR introduce _any_ user-facing change?

Yes, this is a dependency change.

### How was this patch tested?

Pass the CIs.

### Was this patch authored or co-authored using generative AI tooling?

No.

Closes #49563 from dongjoon-hyun/SPARK-50886.

Authored-by: Dongjoon Hyun <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
@dongjoon-hyun dongjoon-hyun deleted the SPARK-50886 branch January 19, 2025 06:03
@panbingkun
Copy link
Contributor

+1, late LGTM

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants