feat: compute table size including blobs - #8127
Conversation
github-actions
Bot
added
A-python
|
Important This PR touches the Lance format specification. Substantive changes to the format specification — the If this is a meaningful format change:
|
![lance-gatekeeper[bot]](https://avatars.githubusercontent.com/in/4433141?s=60&v=4){kind=small}
There was a problem hiding this comment.
Gate recommendation: request changes. The write-time manifest tally is the right general direction, but it must cover every supported Blob v2 writer and avoid scaling manifest memory with every field of every file. Prefer routing single-fragment writes through the same tallying boundary and persisting sparse per-blob-field records with explicit unknown semantics, while retaining additive decoding for old manifests.
| // Record the preprocessor's blob payload tally, aligned to `fields`. | ||
| // Left empty ("not recorded") when the schema has no blob v2 fields or | ||
| // when the preprocessor could not attribute every payload byte. | ||
| let blob_bytes = match self.preprocessor.as_ref().and_then(|pre| pre.blob_bytes()) { |
There was a problem hiding this comment.
The new tally is only finalized by V2WriterAdapter. The public distributed-write path FileFragment::create still calls FragmentCreateBuilder::write_v2_impl, which constructs DataFile::new_unstarted and never runs or records a BlobPreprocessor; a valid prepared inline blob therefore contributes zero bytes to the promised statistic. Route this writer through the same preprocessing/finalization boundary (or populate the same tally explicitly) so all current Blob v2 writers honor the contract.
Reproducer
Add rust/lance/tests/gate_blob_stats_single_fragment.rs:
use std::sync::Arc;
use arrow_array::{RecordBatch, RecordBatchIterator};
use arrow_schema::Schema;
use bytes::Bytes;
use lance::BlobDescriptorArrayBuilder;
use lance::dataset::{WriteParams, fragment::FileFragment};
use lance_file::version::LanceFileVersion;
#[tokio::test]
async fn single_fragment_writer_records_inline_blob_payload_bytes() {
let temp_dir = tempfile::tempdir().unwrap();
let mut blobs = BlobDescriptorArrayBuilder::new("blob");
blobs.push_inline(Bytes::from_static(b"payload")).unwrap();
let (field, array) = blobs.finish().unwrap().into_parts();
let schema = Arc::new(Schema::new(vec![field]));
let batch = RecordBatch::try_new(schema.clone(), vec![array]).unwrap();
let fragment = FileFragment::create(
temp_dir.path().to_str().unwrap(),
0,
RecordBatchIterator::new(vec![Ok(batch)], schema),
Some(WriteParams {
data_storage_version: Some(LanceFileVersion::V2_2),
..Default::default()
}),
)
.await
.unwrap();
assert_eq!(fragment.files[0].blob_bytes.as_ref(), &[7]);
}Run cargo test -p lance --test gate_blob_stats_single_fragment. Expected [7]; observed [].
| // non-blob fields). When empty, blob payload sizes were not recorded — | ||
| // either the file was written before this field existed or the writer could | ||
| // not know them (e.g. externally created data files). | ||
| repeated uint64 blob_bytes = 8; |
There was a problem hiding this comment.
This dense vector scales with all file fields, although almost all entries are usually zero. V2WriterAdapter fills one u64 for every field_id, and every DataFile now carries a 16-byte fat Arc<[u64]>. At the manifest interner's documented 20M-fragment scale, a 50-field table with one blob field adds roughly 8 GB of decoded tally storage plus about 320 MB of per-file struct space, before allocator/protobuf overhead. Persist sparse (field_id, bytes) records with explicit unknown semantics, and benchmark a nonempty-tally manifest at scale.
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
|
moved to this issue, will close this PR for now: |
1 participant
Right now table size calculations don't include blob v2 size, so they're sort of inaccurate. This will add a field
blob_byteswhich maintains a count of the size of each blob as it's written, and then rolls it into the bytes_on_disk totals in statistics.