Conversation
There was a problem hiding this comment.
❌ Gate recommendation: request changes.
The capability-gated Managed design is coherent and the earlier correctness findings remain fixed, but cleanup must not rescan the Blob descriptor surface once per expired snapshot. Keep retained-reference discovery bounded—scan retained manifests only and rely on the existing orphan-age policy, or deduplicate by physical data-file identity across manifests.
| let is_latest = self.read_version <= manifest.version; | ||
| let is_tagged = tagged_versions.contains(&manifest.version); | ||
| let in_working_set = is_latest || !self.policy.should_clean(&manifest) || is_tagged; | ||
| let managed_paths = if manifest.has_managed_blobs() { |
There was a problem hiding this comment.
Cleanup now scans every Managed-flagged manifest, including expired snapshots, and managed_paths runs a full projected dataset scan. Since metadata-only versions reuse the same data file, cleanup rereads identical Blob descriptor pages once per version. That makes maintenance O(expired versions × Blob data) and repeats on retry; large versioned Blob datasets can incur table-scale I/O many times before deleting anything. Limit discovery to retained manifests (letting the existing unverified-age policy handle _blobs without deletion proof) or deduplicate descriptor scans by physical data-file identity.
Reproducer
Added beside the Managed Blob tests and ran cargo test -p lance temporary_managed_cleanup_deduplicates_version_scans -- --nocapture:
#[tokio::test]
async fn temporary_managed_cleanup_deduplicates_version_scans() {
let test_dir = TempStrDir::default();
let payload = vec![42u8; 100 * 1024];
let mut blobs = BlobArrayBuilder::new(1);
blobs.push_bytes(&payload).unwrap();
let schema = Arc::new(Schema::new(vec![blob_field("blob", false)]));
let batch = RecordBatch::try_new(schema.clone(), vec![blobs.finish().unwrap()]).unwrap();
let mut dataset = Dataset::write(
RecordBatchIterator::new(vec![Ok(batch)], schema),
&test_dir,
Some(WriteParams {
data_storage_version: Some(LanceFileVersion::V2_2),
..Default::default()
}),
)
.await
.unwrap();
for version in 2..=8 {
dataset
.update_config([("measurement_version".to_string(), version.to_string())])
.await
.unwrap();
}
let data_file = dataset
.data_dir()
.join(dataset.manifest.fragments[0].files[0].path.as_str());
let _ = dataset.object_store.io_stats_incremental();
crate::dataset::cleanup::cleanup_old_versions(
&dataset,
crate::dataset::cleanup::CleanupPolicy {
before_timestamp: Some(Utc::now() + chrono::TimeDelta::seconds(1)),
..Default::default()
},
)
.await
.unwrap();
let stats = dataset.object_store.io_stats_incremental();
let data_reads = stats
.requests
.iter()
.filter(|request| request.method == "get_opts" && request.path == data_file)
.count();
assert_eq!(data_reads, 1, "{stats:#?}");
}The assertion fails with left: 8, right: 1: the same unchanged data file was opened once for each of the eight manifests.
Blob v2 compaction currently copies Packed and Dedicated payloads because their descriptors address sidecars through the containing data file. Managed descriptors carry an explicit base ID, relative URI, and byte range, allowing compaction to preserve payload objects when replacing data files.
New payload objects use
_blobs/<uuid>.blob. Existing Packed and Dedicated objects can be adopted at their original paths without copying bytes, including when their original data files are later removed. Cleanup follows references from protected snapshots; shallow clone keeps the existing source-retention contract and deep clone copies objects into the destination's independent namespace.Managed uses the existing five descriptor fields with kind 4 in both file formats 2.2 and 2.3. New Blob data-file commits atomically publish paired reader/writer table capability bits and base bindings. The capability survives restore; metadata-only updates to an old table do not activate it. Existing kinds retain their read semantics. This adds no user configuration or repack API.
Compatibility validation uses a checked-in v11.0.0 Packed/Dedicated fixture. The released v11.0.0 client refuses normal opens of flagged snapshots. A real cached-handle experiment also confirms the boundary: after new-client compaction and cleanup remove the original data files, old-client GC with
delete_unverified=Truecan still delete adopted sidecars, while leaving_blobsobjects alone. All maintenance after activation must use a Managed-aware client; the flag cannot retrofit old binaries' historical maintenance paths.The format-spec proposal remains separate, as required by the repository's spec/implementation workflow. This implementation makes no measured throughput or latency claim.
Related: #8899 addresses the compaction problem through a Blob Reuse Index; #9193 documents the existing Blob v2 contract.