Skip to content

fix(tests): reach TFramedTransport at the package libthrift 0.14 moved it to - #19830

Merged
voonhous merged 1 commit into
apache:masterfrom
yihua:hive-test-service-layered-framed-transport
Sep 3, 2026
Merged

fix(tests): reach TFramedTransport at the package libthrift 0.14 moved it to#19830
voonhous merged 1 commit into
apache:masterfrom
yihua:hive-test-service-layered-framed-transport

Conversation

@yihua

@yihua yihua commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Describe the issue this Pull Request addresses

Closes #19829

Summary and Changelog

HiveTestService imports TFramedTransport from the package libthrift moved it out of in 0.14, while its TServerSocketKeepAlive overrides TServerTransport.accept(), which is final below 0.14. No single libthrift version satisfies both, and no method is overridable across both generations.

Points the import at org.apache.thrift.transport.layered.TFramedTransport, and moves the libthrift test dependency in hudi-hive-sync and hudi-utilities from ${thrift.version} (0.13.0, which the metaserver needs) to the existing ${hive.libthrift.version} (0.14.1), which already models the thrift the Hive client is compiled against.

Impact

No production code changes; libthrift stays test scope so no bundle changes. hudi-hive-sync and hudi-utilities carry the full hive-exec, which already resolves a 0.14-era transport package, so behaviour there is unchanged (CI ran TestHiveSyncTool and TestHoodieDeltaStreamer, both start HiveTestService).

The in-repo consumer this fixes is hudi-integ-test: its main class HiveServiceProvider instantiates HiveTestService on a classpath with hive-exec:core and libthrift 0.14.1 as the only thrift provider, so on master the class cannot link there at all (no old-path TFramedTransport exists anywhere on that classpath). This is masked today only because TestHoodieTestSuiteJob is @Disabled (HUDI-3668) and TestExpressionIndex's hive-sync branch is guarded to Java 8. hudi-spark's test classpath is in the same situation.

Risk Level

low

ChainedTTransportFactory.getTransport already declares the TTransportException that 0.14 adds to TTransportFactory.getTransport, so no call site changes. hudi-hive-sync and hudi-utilities both test-compile cleanly on JDK 11.

Documentation Update

none

Contributor's checklist

  • Read through contributor's guide
  • Enough context is provided in the sections above
  • Adequate tests were added if applicable

…d it to

HiveTestService imports TFramedTransport from the pre-0.14 package while
overriding TServerTransport.accept(), which is final below 0.14, so no single
libthrift satisfies it. The build only gets away with it because the full
hive-exec bundles a 0.14-era transport package; consumers of the tests jar that
take hive-exec with the core classifier resolve TServerTransport from libthrift
and fail verification.

The libthrift test dependency moves to hive.libthrift.version, which already
models the thrift the Hive client is built against, leaving thrift.version to
the metaserver.
@codecov-commenter

codecov-commenter commented Sep 3, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.31%. Comparing base (31904ac) to head (21c11be).

Additional details and impacted files
@@             Coverage Diff              @@
##             master   #19830      +/-   ##
============================================
- Coverage     78.31%   78.31%   -0.01%     
+ Complexity    33894    33890       -4     
============================================
  Files          2542     2542              
  Lines        141775   141775              
  Branches      17197    17197              
============================================
- Hits         111031   111027       -4     
- Misses        23032    23038       +6     
+ Partials       7712     7710       -2     
Components Coverage Δ
hudi-common 83.65% <ø> (+0.01%) ⬆️
hudi-client 83.20% <ø> (-0.01%) ⬇️
hudi-flink 85.64% <ø> (-0.03%) ⬇️
hudi-spark-datasource 73.24% <ø> (-0.01%) ⬇️
hudi-utilities 74.52% <ø> (+<0.01%) ⬆️
hudi-cli 15.06% <ø> (ø)
hudi-hadoop 70.72% <ø> (ø)
hudi-sync 75.56% <ø> (ø)
hudi-io 79.98% <ø> (+0.04%) ⬆️
hudi-timeline-service 83.74% <ø> (+0.29%) ⬆️
hudi-cloud 65.81% <ø> (ø)
hudi-kafka-connect 53.20% <ø> (-0.77%) ⬇️
Flag Coverage Δ
common-and-other-modules 51.44% <ø> (-0.01%) ⬇️
flink-integration-tests 48.82% <ø> (+<0.01%) ⬆️
hadoop-mr-java-client 44.09% <ø> (+0.02%) ⬆️
integration-tests 13.51% <ø> (-0.01%) ⬇️
spark-client-hadoop-common 50.54% <ø> (-0.01%) ⬇️
spark-java-tests 52.25% <ø> (+0.01%) ⬆️
spark-scala-tests 46.99% <ø> (+0.01%) ⬆️
utilities 36.56% <ø> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.
see 23 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions github-actions Bot added the size:XS PR with lines of changes in <= 10 label Sep 3, 2026
@hudi-bot

hudi-bot commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator

CI report:

Bot commands @hudi-bot supports the following commands:
  • @hudi-bot run azure re-run the last Azure build

@yihua
yihua marked this pull request as ready for review September 3, 2026 03:22
@voonhous voonhous self-assigned this Sep 3, 2026

@hudi-agent hudi-agent left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ 🤖 This review was generated by an AI agent and may contain mistakes. Please verify any suggestions before applying.

Thanks for working on this! The PR fixes a TFramedTransport import (moved to the layered package in libthrift 0.14) and aligns the test-scope libthrift version in hudi-hive-sync and hudi-utilities with the Hive client's ${hive.libthrift.version} (0.14.1) so HiveTestService compiles consistently. The change is test-scope only, is consistent across the default and spark4 (0.16.0) profiles, and leaves the metaserver's own ${thrift.version} (0.13.0) dependency untouched. No issues flagged from this automated pass — a Hudi committer or PMC member can take it from here for a final review.

cc @yihua

@voonhous voonhous left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@voonhous
voonhous merged commit 7a3e2f3 into apache:master Sep 3, 2026
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XS PR with lines of changes in <= 10

Projects

None yet

Development

Successfully merging this pull request may close these issues.

HiveTestService cannot load under any single libthrift version

5 participants