Label reassignment cleanup is slow and produces thousands of table versions

When a re-crawl moves a label to a new commit, chunks belonging to files that changed are no longer part of that fileset and the label has to come off them. remove_label_from_chunks in src/engine/storage/chunks/storage.rs does that one chunk at a time: for each stale chunk it either deletes the row (if the label was the last one) or rewrites active_label_ids without it. Each of those is its own LanceDB commit, and the commit mutex is acquired once around the whole loop rather than per write, so other catalogs wait on it for the duration.

@davidh233's team measured a real re-crawl with 667 changed files: 2,440 stale chunks, around 2,460 new table versions, and 65 to 91s spent. Those versions then slow every later read, which feeds #83, #85, and the debris that #86 reclaims. This is BL104 on the backlog, and their report is the field data it was waiting for.

Investigation

Their fix groups in memory first, then writes in batches. Rows going to zero labels go to the existing IN-predicate delete helper, which always accepted batches; the old caller just fed it one row at a time. Rows keeping a non-empty label list are grouped by their resulting array, so every row in a group ends up with the same value and shares one update:

if new_labels.is_empty() {
    to_delete.push(chunk.row_id);
} else {
    to_update.entry(new_labels).or_default().push(chunk.row_id);
}
// ...after collecting: batched deletes, then one IN update per label group
for batch in to_delete.chunks(200) {
    self.delete_by_row_ids_inner(batch).await?;
}
for (new_labels, row_ids) in &to_update {
    for batch in row_ids.chunks(200) {
        self.table.update().only_if(in_quoted_strs("row_id", /* batch */))
            .column("active_label_ids", /* labels_sql */).execute().await?;
    }
}

On the same commit pair against two copies of one database, both sides finding the same 2,440 stale chunks: cleanup went from 84.4s to 1.5s and from around 2,460 versions to around 30, with label state and retrieval results matching afterward.

Proposal

Four changes, one PR.

Batch the deletes. Straightforward, and both their patch and BL104 already agree on it.

Replace the per-row update loop with set-based updates. LanceDB's Table::update doc comment addresses this case directly: if the condition is an id equality and you are updating many rows with different ids, it says to use a single merge_insert instead of calling update repeatedly. So the current loop is the shape the library documents as the wrong one. Which batched form replaces it is the part I would most like input on, and there are three candidates below.

Acquire the commit mutex per write, not around the loop. The discipline everywhere else in the storage layer is one acquisition per LanceDB write. Holding it across an entire cleanup blocks writers against unrelated catalogs, and batching shortens the hold without fixing the shape.

Settle the batch size. Their patch uses 200; UPSERT_BATCH_SIZE is 1000. I would rather not introduce a second batching constant without a measured reason, but there may be one here: LanceDB #2085 reports long IN lists behaving badly against an indexed column, with 100 faster than 1000, which is a live question once scalar indexes exist (#89).

The three update mechanisms

array_remove_all in a single update expression. UpdateBuilder::column takes a SQL expression evaluated per row against that row's old value, not a literal, so the whole shrink is one statement per batch: only_if("<row_id IN batch>") with column("active_label_ids", "array_remove_all(active_label_ids, '<label>')"). Lance parses update expressions through DataFusion with nested_expressions enabled, so the function should resolve. Two calls per batch, one delete and one update, regardless of how many distinct label arrays exist. I have read this out of the Lance sources rather than run it, so whether it types correctly against a List<Utf8> column needs a spike before anyone builds on it.

merge_insert, which is what BL104 originally specified and what the LanceDB docs point at. One commit for the whole shrink. The reservation I have is that our merge_insert path unions incoming active_label_ids with existing ones rather than replacing them, which is the cross-label sharing guarantee. A shrink needs replace semantics, so this means a second path through the same primitive with the opposite behavior, and we have already had one bug from a storage function that quietly did two things under one name.

Grouped IN updates, their version above. Write count is (distinct resulting label arrays) x (batches), which in a single-label database is one group and in practice is a handful. It degrades to the current behavior only if every stale row has a distinct label set, so it is never worse than today. Against it: the cost depends on a distributional property rather than being bounded by construction, and it carries a grouping data structure the other two do not need.

I lean toward array_remove_all if the spike holds, on the grounds that it pushes the computation into the database and needs no new write path, and toward grouped updates if it does not. Whichever lands, the acceptance bar should be the equivalence check their team already ran: same stale set, same label state, same retrieval results.

The merge_insert route is the one I am least sure about rejecting. If you think the union-versus-replace concern is smaller than I am making it, or you have a reason a single commit matters more than the extra path, that is worth arguing.

Start in src/engine/storage/chunks/storage.rs at remove_label_from_chunks, then inspect delete_by_row_ids_inner and the existing merge_insert/update paths. Compare the batching and mutex patterns used elsewhere in the storage layer before choosing an update mechanism. Done means the same stale set, label state, and retrieval results, with far fewer LanceDB commits and shorter cleanup time.