lance-format / lance-format/lance-context

[upstream/lance] Give ShardWriter a Drop (or explicit non-async teardown) to avoid leaking background tasks

Open
#164 0 comments 0 reactions 0 assignees View on GitHub

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:

  1. impl Drop for ShardWriter that 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.
  2. An explicit non-async abort() / shutdown() that stops the background tasks immediately without the graceful flush that close().await performs — so callers on non-async paths have a correct option.
  3. Keep close().await as 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().await is still required for durability.

Where to look (Lance side)

  • ShardWriter struct and its TaskExecutor / background task handles.
  • ShardWriter::close (shutdown_all) — the current graceful teardown; factor its task-stopping half into something a Drop can call synchronously.
  • Any JoinHandles / channels the writer owns.

Acceptance criteria

  • Dropping a ShardWriter without calling close().await leaves 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().await still 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().await durability semantics.
  • Making drop asynchronously flush (a Drop can't await; graceful flush stays on close().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

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.