Skip to content

Conversation

@danepitkin
Copy link
Member

@danepitkin danepitkin commented Jul 5, 2023

Rationale for this change

Fix broken CI with Spark due to Arrow upgrading Netty to v4.1.94.

Are these changes tested?

Unit tests pass, but need to run integration tests (via GHA).

Are there any user-facing changes?

No

@danepitkin danepitkin requested a review from lidavidm as a code owner July 5, 2023 21:17
@github-actions
Copy link

github-actions bot commented Jul 5, 2023

⚠️ GitHub issue #36332 has been automatically assigned in GitHub to PR creator.

@lidavidm
Copy link
Member

lidavidm commented Jul 5, 2023

Hmm, does this really work? Just because there's not a reference to it in the source, doesn't mean there isn't a reference in the bytecode

@lidavidm
Copy link
Member

lidavidm commented Jul 5, 2023

@github-actions crossbow submit test-conda-python--spark-

@github-actions
Copy link

github-actions bot commented Jul 5, 2023

Revision: 2176273

Submitted crossbow builds: ursacomputing/crossbow @ actions-4f9b35019b

Task Status
test-conda-python-3.10-spark-master Github Actions
test-conda-python-3.8-spark-v3.1.2 Github Actions
test-conda-python-3.9-spark-v3.2.0 Github Actions

@danepitkin
Copy link
Member Author

No, I also don't think this would work. I was curious what CI would produce (is there a way to run CI w/o creating a diff?). This should be an object mismatch, probably even the same error we saw before.

I'll try the reflection approach instead.

@lidavidm
Copy link
Member

lidavidm commented Jul 5, 2023

If you push a branch, it should run on your fork, too. e.g. see https://github.com/danepitkin/arrow/actions/runs/5469006949

@danepitkin
Copy link
Member Author

If you push a branch, it should run on your fork, too. e.g. see https://github.com/danepitkin/arrow/actions/runs/5469006949

Thank you! This probably isn't a good first Java issue, so I might pass it off to David S. tomorrow and play around on my own branch in the meantime. Shading actually seems like the nicest approach. With reflection, its not yet clear to me how we would resolve. I think we would need to reflect all usage of PoolArenasCache in Netty?

@danepitkin
Copy link
Member Author

Closing for now to reflect that I might not be the one who merges a fix.

@danepitkin danepitkin closed this Jul 5, 2023
@danepitkin danepitkin changed the title GH-36332: [Java] Fix Spark integration failure due to Netty version [Experiment] [Java] Fix Spark integration failure due to Netty version Jul 5, 2023
@lidavidm
Copy link
Member

lidavidm commented Jul 6, 2023

My thought with reflection would be to detect which version of the netty method is available and dispatch to it, yes. But the call overhead is not great.

Shading (or using arrow-memory-unsafe) basically means just modifying Spark's POM, in which case there's maybe not much for us to do.

@danepitkin danepitkin changed the title [Experiment] [Java] Fix Spark integration failure due to Netty version [Abandoned] [Java] Spark integration failure due to Netty version Jul 6, 2023
@danepitkin
Copy link
Member Author

The change in netty was reverted in 4.1.96. See #36926

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.

2 participants