Skip to content

test: run the libhdfs suite manually instead of in CI - #5892

Merged
andygrove merged 1 commit into
apache:mainfrom
andygrove:test/hdfs-suite-manual
Sep 13, 2026
Merged

test: run the libhdfs suite manually instead of in CI#5892
andygrove merged 1 commit into
apache:mainfrom
andygrove:test/hdfs-suite-manual

Conversation

@andygrove

Copy link
Copy Markdown
Member

Which issue does this PR close?

Closes #5023.

Rationale for this change

This is the alternative @comphead asked for on #5890: rather than vendoring a patched libhdfs into
Comet to fix the flake, stop running the one suite that triggers it in CI and run it by hand
instead. I think that is the right call. The crash is real and the mechanism is understood, but the
fix costs us a copy of another project's C in our tree that someone then has to maintain, and HDFS
is not a direction we are investing in.

The mechanism, briefly, so the tradeoff is on the record. libhdfs registers a pthread
thread-local destructor that detaches the current thread from the JVM whenever it finds a cached
JNIEnv, regardless of who attached the thread. Comet attaches its own Tokio workers and detaches
them in on_thread_stop, so on a worker that has touched HDFS the JNIEnv is freed first and the
destructor then dereferences it and jumps to address zero. That is
HDFS-16021, still open upstream. It fires when
the pooled thread exits, which is usually minutes after the HDFS read, in whatever suite happens to
be running by then — that is why it has been reading as a random [scans] failure rather than an
HDFS one, and it has cost us re-runs on several branches.

To be clear about what this does and does not do: it removes CI's exposure, it does not fix the
bug. An HDFS user running Comet can still hit it. The docs changes below are there so that is not a
surprise.

What changes are included in this PR?

ParquetReadFromFakeHadoopFsSuite moves onto the existing "manual test suite" convention already
used by ParquetReadFromS3Suite, IcebergReadFromS3Suite and CometS3CredentialBridgeSuite: drop
it from the scans bucket in both pr_build_linux.yml and pr_build_macos.yml, and add it to the
ignore_list in dev/ci/check-suites.py so preflight stops requiring it. The suite itself is
unchanged apart from a scaladoc recording why it is out of CI and the command to run it.

It is the only suite that loads libhdfs. CometScanSchemeFallbackSuite also exercises an
hdfs:// path, but it only applies CometScanRule to the plan and never executes, so it never
loads the library — meaning the planner-side half of the coverage, that an hdfs:// scan is still
claimed natively instead of silently falling back, stays in CI. The comment in CometScanRule that
pointed only at the suite being removed is updated to say which half lives where. The Rust HDFS
tests in parquet_writer.rs are already #[ignore]d as needing a live cluster.

On the docs side, the HDFS section of the data sources guide gets a warning that support is
experimental, is not covered by CI, and can crash the JVM, with links to HDFS-16021 and #5023, plus
the manual test command in the local-development section. The contributor guide gains a short note
on the check-suites.py ignore list, which was previously undocumented.

I also fixed something I ran into while checking which tests touch HDFS: the Iceberg guide lists
"Hadoop Distributed File System (HDFS)" under supported storage for the native Iceberg reader, and
that is not true. storage_factory_for in iceberg_common.rs has arms for file, memory, gs,
oss and the s3 family only, CometScanRule.icebergReadableSchemes mirrors it, and the roadmap
already says HDFS-backed Iceberg tables are unsupported. The guide now says they fall back to Spark,
and the storage list picks up gs and oss, which were supported but unlisted.

How are these changes tested?

dev/ci/check-suites.py and dev/ci/check-ci-config.py both pass, which is the check that would
have failed if the workflow edit and the ignore-list edit had got out of sync.

The point of the change is that the suite still works when run by hand, so I ran the documented
command against a debug build on macOS/arm64 with Spark 4.1:

./mvnw test -Dtest=none -Dsuites="org.apache.comet.parquet.ParquetReadFromFakeHadoopFsSuite"

ParquetReadFromFakeHadoopFsSuite:
- native scan on fake fs
Tests: succeeded 1, failed 0, canceled 0, ignored 0, pending 0
All tests passed.

Note the test was not canceled, so hdfs-opendal was present in the build and the libhdfs path
really was exercised rather than skipped by the assume.

./mvnw test-compile and scalastyle are clean, spotless:apply is a no-op on the result, and
prettier is clean on both edited docs.

If we go with this, #5890 should close unmerged.

ParquetReadFromFakeHadoopFsSuite is the only suite that loads libhdfs,
and libhdfs's thread-local destructor detaches Comet's own Tokio workers
from the JVM (HDFS-16021), crashing the JVM on a later, unrelated suite.
Move it to the existing "manual test suite" convention rather than
carrying a patched copy of libhdfs in the tree.

Also document HDFS support as experimental, and drop the incorrect claim
that the native Iceberg reader supports HDFS-backed tables.
@github-actions github-actions Bot added enhancement New feature or request test Testing related area:scan Parquet scan / data reading area:Iceberg labels Sep 13, 2026

@peterxcli peterxcli 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.

+1

@sunchao sunchao 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.

+1 - thanks!

@rich7420 rich7420 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.

@andygrove thanks for the patch

@andygrove
andygrove added this pull request to the merge queue Sep 13, 2026
Merged via the queue into apache:main with commit 451c999 Sep 13, 2026
64 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:Iceberg area:scan Parquet scan / data reading enhancement New feature or request test Testing related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Sporadic SIGSEGV in macOS [scans] workflow in CI

4 participants