diff --git a/.github/workflows/pr_build_linux.yml b/.github/workflows/pr_build_linux.yml index 6814d75769..4cd30e66cd 100644 --- a/.github/workflows/pr_build_linux.yml +++ b/.github/workflows/pr_build_linux.yml @@ -376,7 +376,6 @@ jobs: org.apache.comet.parquet.CometParquetWriterSuite org.apache.comet.parquet.ParquetReadV1Suite org.apache.comet.parquet.ParquetReadV2Suite - org.apache.comet.parquet.ParquetReadFromFakeHadoopFsSuite org.apache.comet.parquet.ParquetTimestampLtzAsNtzSuite org.apache.spark.sql.comet.ParquetDatetimeRebaseV1Suite org.apache.spark.sql.comet.ParquetDatetimeRebaseV2Suite diff --git a/.github/workflows/pr_build_macos.yml b/.github/workflows/pr_build_macos.yml index 66a1b62ba3..d5480dca11 100644 --- a/.github/workflows/pr_build_macos.yml +++ b/.github/workflows/pr_build_macos.yml @@ -121,7 +121,6 @@ jobs: org.apache.comet.parquet.CometParquetWriterSuite org.apache.comet.parquet.ParquetReadV1Suite org.apache.comet.parquet.ParquetReadV2Suite - org.apache.comet.parquet.ParquetReadFromFakeHadoopFsSuite org.apache.comet.parquet.ParquetTimestampLtzAsNtzSuite org.apache.spark.sql.comet.ParquetDatetimeRebaseV1Suite org.apache.spark.sql.comet.ParquetDatetimeRebaseV2Suite diff --git a/dev/ci/check-suites.py b/dev/ci/check-suites.py index 52a221b2cd..7dc624c852 100644 --- a/dev/ci/check-suites.py +++ b/dev/ci/check-suites.py @@ -34,6 +34,7 @@ def file_to_class_name(path: Path) -> str | None: ignore_list = [ "org.apache.comet.parquet.ParquetReadSuite", # abstract "org.apache.comet.parquet.ParquetReadFromS3Suite", # manual test suite + "org.apache.comet.parquet.ParquetReadFromFakeHadoopFsSuite", # manual test suite (loads libhdfs, see #5023) "org.apache.comet.IcebergReadFromS3Suite", # manual test suite "org.apache.comet.cloud.s3.CometS3CredentialBridgeSuite", # manual test suite "org.apache.comet.shuffle.CelebornReflectionCompatibilitySuite", # dedicated version matrix diff --git a/docs/source/contributor-guide/development.md b/docs/source/contributor-guide/development.md index b04a3fdd42..3025e259e8 100644 --- a/docs/source/contributor-guide/development.md +++ b/docs/source/contributor-guide/development.md @@ -658,6 +658,17 @@ Choose the group that best matches the area your test covers: `*Suite.scala` files in the repository and verifies that each one appears in both workflow files. If any suite is missing, this check will fail and block the PR. +A small number of suites are deliberately **not** run in CI, because they need infrastructure CI +does not have or because running them there is not worth the cost. These are listed in the +`ignore_list` in `dev/ci/check-suites.py`, and each one documents in its own scaladoc why it is +excluded and how to run it. Run a manual suite with: + +```sh +./mvnw test -Dtest=none -Dsuites="org.apache.comet.parquet.ParquetReadFromFakeHadoopFsSuite" +``` + +Only add a suite to that list with a good reason; the default is that a new suite runs in CI. + The macOS suites only run in the merge queue by default. See [Continuous Integration](ci.md) for the two tiers and the labels that opt a pull request into a queue-only suite. diff --git a/docs/source/user-guide/latest/datasources.md b/docs/source/user-guide/latest/datasources.md index 97dbece009..97d3470fac 100644 --- a/docs/source/user-guide/latest/datasources.md +++ b/docs/source/user-guide/latest/datasources.md @@ -61,6 +61,20 @@ Comet supports most standard storage systems, such as local file system and obje The Apache DataFusion Comet Rust-based reader seamlessly scans files from remote HDFS for [supported formats](#supported-spark-data-sources) +```{warning} +HDFS support is experimental and is not covered by continuous integration. Comet reads HDFS through +`libhdfs`, which registers a thread-local destructor that detaches the calling thread from the JVM +regardless of which component attached it +([HDFS-16021](https://issues.apache.org/jira/browse/HDFS-16021), still open upstream). Comet +attaches its own worker threads, so a worker that has read from HDFS can crash the JVM with a +`SIGSEGV` when it later exits +([#5023](https://github.com/apache/datafusion-comet/issues/5023)). The crash surfaces well after +the HDFS read itself, typically while an unrelated query is running. +``` + +Native Iceberg scans do not support HDFS-backed tables; those scans fall back to Spark. See the +[Comet and Iceberg Guide](iceberg.md). + ### Building Comet with HDFS support To build Comet with remote HDFS support it is required to have a JDK installed. @@ -165,6 +179,14 @@ JAVA_HOME="/opt/homebrew/opt/openjdk@17" make release PROFILES="-Pspark-4.1" RUS Or use `spark-shell` with HDFS support as described [above](#building-comet-with-hdfs-support) +Comet also has a test suite that exercises a native scan through `libhdfs` against a fake Hadoop +filesystem, so it needs no cluster. Because of the crash described above it is excluded from CI and +run by hand: + +```shell +./mvnw test -Dtest=none -Dsuites="org.apache.comet.parquet.ParquetReadFromFakeHadoopFsSuite" +``` + ## S3 Comet's Parquet scan completely offloads data loading to Rust. It uses the diff --git a/docs/source/user-guide/latest/iceberg.md b/docs/source/user-guide/latest/iceberg.md index c542f689bb..cd8ac61bf4 100644 --- a/docs/source/user-guide/latest/iceberg.md +++ b/docs/source/user-guide/latest/iceberg.md @@ -110,8 +110,10 @@ The native Iceberg reader supports the following features: **Storage:** - Local filesystem -- Hadoop Distributed File System (HDFS) - S3-compatible storage (AWS S3, MinIO) +- Google Cloud Storage (`gs`) and Alibaba Cloud OSS (`oss`) + +HDFS-backed tables are not supported by the native Iceberg reader and fall back to Spark. ### REST Catalog diff --git a/spark/src/main/scala/org/apache/comet/rules/CometScanRule.scala b/spark/src/main/scala/org/apache/comet/rules/CometScanRule.scala index e3b6f77eb8..f7e0fe5cb9 100644 --- a/spark/src/main/scala/org/apache/comet/rules/CometScanRule.scala +++ b/spark/src/main/scala/org/apache/comet/rules/CometScanRule.scala @@ -248,8 +248,9 @@ case class CometScanRule(session: SparkSession) // // EXCEPT schemes the user routes through libhdfs via `spark.hadoop.fs.comet.libhdfs.schemes` // (e.g. `hdfs`, or a test `fake`): those ARE natively readable through the libhdfs object_store - // bridge, so they must NOT be declined here (regression guarded by - // ParquetReadFromFakeHadoopFsSuite). + // bridge, so they must NOT be declined here. The claim decision is guarded in CI by + // CometScanSchemeFallbackSuite; end-to-end execution through libhdfs is guarded by + // ParquetReadFromFakeHadoopFsSuite, which is a manual suite (see its scaladoc). // // The default mirrors the native side: when the config is unset, `is_hdfs_scheme` // (native/core/src/parquet/parquet_support.rs) treats `hdfs` as natively readable, and diff --git a/spark/src/test/scala/org/apache/comet/parquet/ParquetReadFromFakeHadoopFsSuite.scala b/spark/src/test/scala/org/apache/comet/parquet/ParquetReadFromFakeHadoopFsSuite.scala index 12fc13b0bb..5dac5fb91c 100644 --- a/spark/src/test/scala/org/apache/comet/parquet/ParquetReadFromFakeHadoopFsSuite.scala +++ b/spark/src/test/scala/org/apache/comet/parquet/ParquetReadFromFakeHadoopFsSuite.scala @@ -33,6 +33,36 @@ import org.apache.spark.sql.functions.{col, sum} import org.apache.comet.CometConf import org.apache.comet.hadoop.fs.FakeHDFSFileSystem +/** + * End-to-end coverage of a native Parquet scan routed through libhdfs, using a fake Hadoop + * FileSystem instead of a live namenode. + * + * '''This suite is excluded from CI and must be run manually''' (it is in the ignore list in + * `dev/ci/check-suites.py`, so it is not listed in either `pr_build_*.yml` workflow): + * + * {{{ + * ./mvnw test -Dtest=none \ + * -Dsuites="org.apache.comet.parquet.ParquetReadFromFakeHadoopFsSuite" + * }}} + * + * It is the only suite that actually loads libhdfs, and libhdfs registers a pthread thread-local + * destructor (`hdfsThreadDestructor`) that detaches the current thread from the JVM regardless of + * who attached it. Comet attaches its own Tokio workers, so once one of them has touched libhdfs + * the destructor dereferences a `JNIEnv` that Comet has already freed and the JVM dies with + * `SIGSEGV at pc=0x0`. That is [[https://issues.apache.org/jira/browse/HDFS-16021 HDFS-16021]], + * still open upstream. The crash lands on whichever suite happens to be running when the worker + * exits -- usually minutes later, in a different suite -- so it reads as a random `[scans]` flake + * rather than an HDFS failure. See + * [[https://github.com/apache/datafusion-comet/issues/5023 #5023]]. + * + * Comet's HDFS support is experimental (see the + * [[https://datafusion.apache.org/comet/user-guide/latest/datasources.html#hdfs data sources guide]]), + * and working around the upstream bug would mean carrying a patched copy of libhdfs in this repo, + * so we run this suite by hand instead of paying for the flake on every pull request. The + * planner-side half of the coverage -- that a `hdfs://` scan is still claimed natively rather + * than silently falling back -- does run in CI, in `CometScanSchemeFallbackSuite`, because it + * never executes the scan and so never loads libhdfs. + */ class ParquetReadFromFakeHadoopFsSuite extends CometTestBase with AdaptiveSparkPlanHelper { private var fake_root_dir: File = _