lance-format / lance-format/lance-context
[upstream/lance] Give ShardWriter a Drop (or explicit non-async teardown) to avoid leaking background tasks
Nobody has claimed this yet.
- Dominant language
- Rust
- Stars
- 81
- Forks
- 21
- Avg merge
- 19h 23m
- Merged PRs (30d)
- 9
Description
[upstream/lance] — this tracks a change that ideally lands in Lance (the MemWAL owner). Filed here for internal triage; move upstream when ready.
Summary
ShardWriter spawns background tasks (via its TaskExecutor) that are only reclaimed by an explicit close().await. It has no Drop impl, so dropping a writer without awaiting close leaks those background tasks. Callers can't always await on a drop path (LRU eviction, panic unwind, teardown without a runtime), so they're forced into fragile best-effort workarounds. Lance should reclaim a ShardWriter's resources on drop.
Motivation
Because ShardWriter must be close().await-ed but can be dropped on non-async paths, lance-context wraps it like this (crates/lance-context-core/src/rollout_store.rs:1619):
impl Drop for RolloutStore {
fn drop(&mut self) {
if let Some(writer) = self.write_writer.take() {
if let Ok(handle) = tokio::runtime::Handle::try_current() {
handle.spawn(async move { let _ = writer.close().await; });
}
// else: no runtime -> we can only drop it -> background tasks LEAK
}
}
}
This gambles on a Tokio runtime being present at drop time. When it isn't (some teardown paths, non-Tokio contexts), the writer's background tasks leak. This workaround exists solely because Lance provides no drop-time cleanup.
Goal
Dropping a ShardWriter must not leak background tasks, without requiring the caller to await on the drop path.
Proposed change
One or more of:
impl Drop for ShardWriterthat aborts / signals its background tasks to stop (a synchronous, best-effort teardown — abort the executor tasks, drop channels). This guarantees no leak even without a runtime or an await.- An explicit non-async
abort()/shutdown()that stops the background tasks immediately without the graceful flush thatclose().awaitperforms — so callers on non-async paths have a correct option. - Keep
close().awaitas the graceful path (freeze + drain + shutdown) and document the distinction:close().await= graceful (flushes pending work),drop/abort= immediate (may discard un-drained in-memory state, but never leaks).
Constraints:
- Drop-time teardown must be safe when no Tokio runtime is current.
- Semantics of what is / isn't flushed on the non-graceful path must be documented so callers know when
close().awaitis still required for durability.
Where to look (Lance side)
ShardWriterstruct and itsTaskExecutor/ background task handles.ShardWriter::close(shutdown_all) — the current graceful teardown; factor its task-stopping half into something aDropcan call synchronously.- Any
JoinHandles / channels the writer owns.
Acceptance criteria
- Dropping a
ShardWriterwithout callingclose().awaitleaves no orphaned background tasks (test: create a writer, drop it, assert its executor tasks have terminated / handles are aborted). - Drop-time teardown does not panic when there is no current Tokio runtime.
close().awaitstill performs the full graceful flush + drain; its behavior is unchanged.- Docs state clearly which teardown flushes pending generations and which does not.
Non-goals
- Changing the graceful
close().awaitdurability semantics. - Making drop asynchronously flush (a
Dropcan't await; graceful flush stays onclose().await).
Downstream follow-up (not part of this issue)
lance-context removes the runtime-gambling detached-close from its Drop impl and relies on ShardWriter's own drop (keeping an explicit close().await only on the graceful eviction path).
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in the upstream Lance implementation of ShardWriter, focusing on its TaskExecutor, owned task handles or channels, and close's shutdown_all path. Compare graceful close with the proposed synchronous teardown and add a test that drops a writer without a current Tokio runtime. Done means no background tasks remain, close still flushes and drains, and the teardown durability distinction is documented.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- backend
- Issue type
- Feature
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100