feat: consolidate target_file_size and allow unbounded writes

[abhi-airspace-intelligence]

Description

I was trying to investigate the issue in 3855, and I believe I've figured it out. The problem is that when we perform a compaction, we create a bin of files where their total sum is > the target file size, and then, when we go to write the file, we also apply the exact same target file size. This means if, say, the size of all the files in your bin is 105MB, you write a 100MB file and a 5MB file, which I would argue is undesired behavior. Additionally, due to compression/rounding, I've observed that the 100MB file might be written as 97MB and 4MB respectively, which then leads to infinite compactions on the same set of file that generates wasteful IO.

Instead, this changes compactions to allow a target_file_size to be unbounded when actually performing the rewrite operation. To do this, I had to replumb a lot of the code since everything makes the assumption of target-size writes. Additionally, I found there are three separate places where a default target size is set, so I consolidated them to just one place.

Related Issue(s)

Closes #3855

Documentation

[github-actions]

ACTION NEEDED

delta-rs follows the Conventional Commits specification for release automation.

The PR title and description are used as the merge commit message. Please update your PR title and description to match the specification.

[codecov]

Codecov Report

❌ Patch coverage is 73.23944% with 19 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.73%. Comparing base (8f22045) to head (d4ec741).
⚠️ Report is 4 commits behind head on main.

Files with missing lines	Patch %	Lines
python/src/lib.rs	0.00%	16 Missing ⚠️
crates/core/src/operations/write/writer.rs	93.33%	0 Missing and 2 partials ⚠️
crates/core/src/operations/write/mod.rs	83.33%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3901      +/-   ##
==========================================
- Coverage   73.76%   73.73%   -0.04%     
==========================================
  Files         151      151              
  Lines       39396    39391       -5     
  Branches    39396    39391       -5     
==========================================
- Hits        29061    29045      -16     
- Misses       9023     9036      +13     
+ Partials     1312     1310       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 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.

[ion-elgreco]

Maybe just flatten it instead of nesting

[abhi-airspace-intelligence]

It was done this way to support the following usecases:

None: bounded writes, fallback to the delta table configuration
Some(None): unbounded writes
Some(val): bounded writes, overriding delta table configuration

[ion-elgreco]

I see, but maybe we can use an enum for this, it's not quite apparent when you look at it :)

[ion-elgreco]

I think this will cause a first write to be unbounded, no? Not sure if thats something we want

[abhi-airspace-intelligence]

Good catch, it should fallback to the default size.

[ion-elgreco]

👍

[abhi-airspace-intelligence]

This moves a bunch of a bunch of parameters to be Option<NonZeroUsize>, which enables niche optimization, but is also a breaking API change. https://doc.rust-lang.org/std/num/type.NonZeroUsize.html

[abhi-airspace-intelligence]

Also, now that I think about this, using usize for a file size is probably an antipattern... perhaps, it should be switched to an u64? object_store docs reference this here. https://docs.rs/object_store/latest/object_store/struct.ObjectMeta.html#structfield.size.

@rtyler thoughts?