Skip to content

Support configuring delete file replication factor through spark-sql properties. - #229

Merged
shanthoosh merged 1 commit into
openhouse-1.5.2from
add_delete_file_replication_as_table_sql_property
Jan 9, 2026
Merged

Support configuring delete file replication factor through spark-sql properties.#229
shanthoosh merged 1 commit into
openhouse-1.5.2from
add_delete_file_replication_as_table_sql_property

Conversation

@shanthoosh

@shanthoosh shanthoosh commented Jan 9, 2026

Copy link
Copy Markdown
Collaborator

Changes

PR #219 added support for delete file replication from spark applications. This PR adds support to configures the delete file replication factor through spark-sql properties.

Tests

./gradlew clean && ./gradlew build

@shanthoosh
shanthoosh force-pushed the add_delete_file_replication_as_table_sql_property branch from 5112a8d to 25a31b1 Compare January 9, 2026 00:20
@shanthoosh
shanthoosh force-pushed the add_delete_file_replication_as_table_sql_property branch from 25a31b1 to 10475c9 Compare January 9, 2026 00:29
@shanthoosh
shanthoosh merged commit 809534d into openhouse-1.5.2 Jan 9, 2026
23 checks passed
shanthoosh pushed a commit to shanthoosh/iceberg that referenced this pull request Jul 14, 2026
/linkedin#229) into sql-wire-in

# Conflicts:
#	spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/TestSparkWriteConf.java
mkuchenbecker pushed a commit to mkuchenbecker/iceberg that referenced this pull request Jul 21, 2026
…es (v3.5/1.10)

Root cause: the linkedin#219/linkedin#229 delete-file replication override was correctly plumbed
all the way to the delete OutputFileFactory (SparkPositionDeltaWrite threads
deleteFileReplication into the delete factory; OutputFileFactory puts
file-replication-factor into the properties map; HadoopFileIO/HadoopOutputFile
read it). But the actual data/delete file writers BYPASS HadoopOutputFile.create()
(the only place the factor reaches fs.create):

  - Parquet: ParquetIO.file(HadoopOutputFile) returns parquet's native
    HadoopOutputFile.fromPath(path, conf), which opens the file at the file
    system default replication.
  - ORC: ORC.newFileWriter -> OrcFile.createWriter(path, options); ORC's
    PhysicalFsWriter uses fs.getDefaultReplication(path).

So on real HDFS the position-delete files silently took dfs.replication
regardless of the override. This is a pre-existing gap (present in
openhouse-1.5.2 too), only surfaced by real-HDFS validation — the LocalFileSystem
test harness ignores replication entirely.

Fix (guarded by replication > 0, i.e. zero impact when the feature is unused):
  - HadoopOutputFile: expose getReplication().
  - ParquetIO.file(...) both overloads: when a custom replication factor is set,
    route through ParquetOutputFile so iceberg's HadoopOutputFile.create()
    (fs.create with the explicit replication) is used.
  - ORC.newFileWriter: after creating the (empty) writer, apply the requested
    replication to the file via fs.setReplication so the blocks written use it.

Validated on real HDFS (Hadoop 3.2.1, Spark 3.5.2) via the oh-hadoop-spark
docker stack, dfs.replication=3:
  ORC  session delete-file-replication=1        -> delete file %r = 1
  ORC  TBLPROPERTY write.delete-file-replication=2 -> delete file %r = 2
  ORC  UPDATE (MoR) session=1                    -> delete file %r = 1
  Parquet session=1                             -> delete file %r = 1
Data files continue to follow dfs.replication (%r = 3), no regression.

1.10 / Spark-3.5 only; core orc/ and parquet/ modules are shared, so the same
patch should be replayed onto the 1.11 branch.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LLnVvLYnxjTuH2b5nAHa4A
mkuchenbecker pushed a commit to mkuchenbecker/iceberg that referenced this pull request Jul 22, 2026
…es (v3.5/1.10)

Root cause: the linkedin#219/linkedin#229 delete-file replication override was correctly plumbed
all the way to the delete OutputFileFactory (SparkPositionDeltaWrite threads
deleteFileReplication into the delete factory; OutputFileFactory puts
file-replication-factor into the properties map; HadoopFileIO/HadoopOutputFile
read it). But the actual data/delete file writers BYPASS HadoopOutputFile.create()
(the only place the factor reaches fs.create):

  - Parquet: ParquetIO.file(HadoopOutputFile) returns parquet's native
    HadoopOutputFile.fromPath(path, conf), which opens the file at the file
    system default replication.
  - ORC: ORC.newFileWriter -> OrcFile.createWriter(path, options); ORC's
    PhysicalFsWriter uses fs.getDefaultReplication(path).

So on real HDFS the position-delete files silently took dfs.replication
regardless of the override. This is a pre-existing gap (present in
openhouse-1.5.2 too), only surfaced by real-HDFS validation — the LocalFileSystem
test harness ignores replication entirely.

Fix (guarded by replication > 0, i.e. zero impact when the feature is unused):
  - HadoopOutputFile: expose getReplication().
  - ParquetIO.file(...) both overloads: when a custom replication factor is set,
    route through ParquetOutputFile so iceberg's HadoopOutputFile.create()
    (fs.create with the explicit replication) is used.
  - ORC.newFileWriter: after creating the (empty) writer, apply the requested
    replication to the file via fs.setReplication so the blocks written use it.

Validated on real HDFS (Hadoop 3.2.1, Spark 3.5.2) via the oh-hadoop-spark
docker stack, dfs.replication=3:
  ORC  session delete-file-replication=1        -> delete file %r = 1
  ORC  TBLPROPERTY write.delete-file-replication=2 -> delete file %r = 2
  ORC  UPDATE (MoR) session=1                    -> delete file %r = 1
  Parquet session=1                             -> delete file %r = 1
Data files continue to follow dfs.replication (%r = 3), no regression.

1.10 / Spark-3.5 only; core orc/ and parquet/ modules are shared, so the same
patch should be replayed onto the 1.11 branch.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LLnVvLYnxjTuH2b5nAHa4A
(cherry picked from commit 06e05cc)
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