refactor: use iceberg-rust rolling writer directly

[Li0k]

Summary

• bump iceberg-rust workspace deps to baaa9c7b2deb3e744db21712e4b6ced5891a6012
• remove the local RollingIcebergWriter wrapper and use upstream RollingFileWriterBuilder directly
• pass target_file_size_bytes and max_concurrent_closes through to the upstream rolling writer
• deprecate enable_dynamic_size_estimation and size_estimation_smoothing_factor; both settings are now no-op compatibility shims and their builder setters emit deprecation warnings

Test

• cargo test -p iceberg-compaction-core --lib -- --nocapture
• cargo test -p iceberg-compaction-integration-tests -- --nocapture

Note

• The switch to upstream rolling changes rollover timing from write-ahead prediction to current-size-based rolling.
• As a result, output file count changes in the rolling integration case, but total output size remains nearly unchanged.

Before: origin/main
output_files_count = 20
total_output_size  = 1,550,643
avg_size           = 77,532
min/max            = 37,324 / 93,027
sizes              = [
  37324, 37408, 37804, 38224, 38476,
  88099, 89197, 89197, 89575, 89611,
  90343, 90465, 90697, 90733, 91429,
  91819, 91953, 92515, 92747, 93027
]

After: PR
output_files_count = 15
total_output_size  = 1,546,645
avg_size           = 103,109
min/max            = 95,555 / 109,283
sizes              = [
   95555,  96213,  96665,  97007,  98263,
  102925, 103109, 103365, 105255, 106085,
  107269, 107855, 108525, 109271, 109283
]

[Copilot]

Pull request overview

Refactors compaction output writing to rely on iceberg-rust’s upstream rolling writer implementation (after bumping workspace deps), removing the local rolling writer wrapper while keeping deprecated config knobs as compatibility no-ops.

Changes:

• Bump iceberg-rust git workspace dependencies to baaa9c7b2deb3e744db21712e4b6ced5891a6012.
• Remove the local RollingIcebergWriter module and switch DataFusion execution to RollingFileWriterBuilder directly (including target_file_size_bytes + max_concurrent_closes passthrough).
• Update integration test expectations for upstream rolling behavior and deprecate dynamic size-estimation config fields/setters as no-op shims.

Reviewed changes

Copilot reviewed 7 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
integration-tests/src/integration_tests.rs	Adjusts rolling compaction assertions to match upstream rolling behavior by counting outputs per partition bucket.
core/src/executor/mod.rs	Stops exporting the removed iceberg_writer module.
core/src/executor/iceberg_writer/rolling_iceberg_writer.rs	Deletes the local rolling writer implementation and its unit tests.
core/src/executor/iceberg_writer/mod.rs	Removes the now-defunct iceberg_writer module entrypoint.
core/src/executor/datafusion/mod.rs	Constructs writers using RollingFileWriterBuilder directly and passes through rolling-related config.
core/src/config/mod.rs	Deprecates dynamic size-estimation config fields and builder setters as no-op compatibility shims.
Cargo.toml	Updates iceberg-related git dependency revisions.
Cargo.lock	Locks updated transitive dependency graph for the new iceberg revision.

[Copilot]

This test’s goal (per the comment) is to ensure rolling within a partition doesn’t panic/error, but asserting an exact == 3 files per partition is likely brittle (minor parquet/iceberg writer changes or different compression can shift rollover boundaries). Consider relaxing this to assert that each bucket produced >1 output file (or a small acceptable range), while still asserting every partition bucket is present.

Suggested change

	output_files_per_partition.values().all(|count| *count == 3),
	"Compaction should produce exactly 3 files per partition with upstream rolling: {output_files_per_partition:?}"
	output_files_per_partition.values().all(|count| *count > 1),
	"Compaction should produce more than one file per partition with upstream rolling: {output_files_per_partition:?}"

[Li0k]

hi @vovacf201 @nagraham @chenzl25

We previously discussed a better size switching strategy. I have now found that the upstream has exposed the in_progress_size interface, which allows us to implement more fine-grained file splitting!

According to my tests, we no longer need to wrap an additional writer externally, and I have also retained concurrent close to ensure that throughput performance is not affected.

After this PR is merged, we will deprecate two config items, for which I have added the deprecated field.

[chenzl25]

LGTM！

[nagraham]

Excellent refactor! I see this is going to give us greater accuracy in file sizes, and it greatly reduces complexity in the code base. This makes me very happy.

The problem before is that the old writer was forced into using Arrow's RecordBatch::get_array_memory_size(), which was in-accurate (it even admitted that in its docs). This didn't account for Parquet's columnar encoding or its compression. And it had to rely on this memory size for the whole file. So we would get wild inaccuracies. For some data sets, we would try to compress to 128MB, but get files that were 20-30MB.

The iceberg-rust RollingIcebergWriter addresses the problem because Arrow exposed the current_written_size() function. That will return inner.bytes_written() + inner.in_progress_size().

• bytes_written: This is the ACTUAL compressed / encoded bytes flushed to the parquet file
• in_progress_size: An estimate of the size of the the arrow column writer.

IIRC, that is exactly how the Java library estimates size as well.