Skip to content

rfc: remote optimize through the serving owner - #1214

Open
ragnorc wants to merge 5 commits into
ModernRelay:mainfrom
ragnorc:rfc/remote-optimize-v3
Open

ragnorc wants to merge 5 commits into
ModernRelay:mainfrom
ragnorc:rfc/remote-optimize-v3

Conversation

@ragnorc

@ragnorc ragnorc commented Oct 31, 2026 •

Copy link
Copy Markdown
Contributor

Reopens the remote-optimize RFC for review. It merged early as #1190 (still status: draft) and #1203 reverted it; this is the same text plus the fixes its post-merge review asked for, recorded in the decision log under 2026-10-29 "Reopened":

  • "takes docs(rfc): optimize graphs through the running server #1186's ceilings" became "adapts": docs(rfc): optimize graphs through the running server #1186 refuses a whole request over a ceiling, this RFC defers per table and resumes; every difference from docs(rfc): optimize graphs through the running server #1186 is now listed and its narrow-grant argument is answered;
  • the record's 32 / 1 MiB / 4 MiB bounds share the deployment receipts' constants instead of restating them;
  • the resolution walk is bounded per attempt (resolution_walk_limit), resumed below the stop commit and reported by lookup;
  • the only exit from a full unresolved list is a move of main's graph head; no override, and a fence commit is named as a later slice;
  • why the before_publish callback was preferred over a two-phase engine API;
  • containment today covers DataFusion and RecoveryRequired only;
  • the artifact write is skipped under a pending shutdown, and its default limit is sized against the shutdown grace at qualification.

Trimmed after comparing with LanceDB's served maintenance (its embedded optimize() is a client-side call, its remote tables refuse it, and its managed service runs compaction off the request path as registry jobs with durable ids, cancel and reattach):

  • the HTTP request carries tables and max_gate_hold only; resource ceilings and the run budget are serve settings alone, as docs(rfc): optimize graphs through the running server #1186 had it;
  • the record is keyed by kind and holds no event history, progress, pause or resume; a job registry is the control plane's;
  • the out-of-process worker is deferred, not rejected: Lance's compaction task API (plan_compaction, serializable CompactionTask, execute, then the commit) already separates the steps TableStore uses, so a later slice moves execute alone while the owner keeps the plan and the one publication.

A new §3.6 states every cross-process hazard once, in one table: the cluster lock, the S3 lease, the manifest version CAS, the record's conditional puts, detached staging, pinned reads, the in-process gates, standalone roots, raw writers, the other maintenance operations and a later worker. No mechanism is added.

A bounded, manually requested optimize of a served graph's main branch, executed by the process that serves and publishes it: today's gate shape and one publication, a gate-hold bound with an exact outcome per table, a durable operation record under a caller-chosen id before any effect, lookup by that id after a lost response, and restart resolution from the graph's history.

Competing draft: #1186 (a synchronous call with no record). Only one of the two should be accepted; the comparison is in this RFC's Relationship to open work and Alternatives, and on #1186.

Blocked on #1175, #1176 and an in-region measurement, as the front matter says. Implementation drafts: #1204 (engine); the server and CLI slices reopen after acceptance.

Docs-only; python3 scripts/check-docs.py passes.

ragnorc and others added 3 commits October 31, 2026 13:37
* rfc: remote optimize through the serving owner

A bounded, manually requested optimize of a served graph's main branch,
executed by the process that serves and publishes it: today's gate shape
and one publication, a gate-hold bound with an exact outcome per table, a
durable operation record under a caller-chosen id before any effect,
lookup by that id after a lost response, and restart resolution from the
graph's history. Replaces the closed served-maintenance proposal in the
form its review asked for.

* rfc: link the remote optimize RFC to its discussion

* rfc: revise remote optimize after review

Record the intent nonce and resolve by commit_id_answers; keep an
in-doubt publication non-terminal; release the writer gates before the
artifact scan; add a resume position, resource ceilings and per-table
failure isolation; require maintain on every route; depend on ModernRelay#1175
and ModernRelay#1176.

* rfc: settle remote optimize resume, ceilings and resolution after second review

* rfc: settle remote optimize resolution reads and the artifact bound after third review

* rfc: resolve through the buffered commit lookup and count both offset sides after fourth review

* rfc: state the history cost of a released commit and reuse the collector's path resolution after fifth review
Adapt rather than take ModernRelay#1186's ceilings and list every difference; share
the record's bounds with the deployment receipts; cap the resolution walk
per attempt; name the exit from a full unresolved list; justify the
callback over a two-phase engine API; narrow the containment claim; skip
the artifact write under a pending shutdown and size its limit against the
grace at qualification.
…path

The HTTP request carries tables and max_gate_hold only; resource ceilings
and the run budget are serve settings. The record is keyed by kind and holds
no event history, progress, pause or resume. The out-of-process worker is
deferred rather than rejected: Lance's compaction task API already separates
planning, execute and the commit, and a later slice moves execute alone.
A §3.6 table names each hazard, the mechanism that owns it and where it is
decided: the cluster lock, the S3 lease, the manifest version CAS, the
record's conditional puts, detached staging, pinned reads, the in-process
gates, standalone roots, raw writers, the other maintenance operations and
a later worker. No mechanism is added.

ragnorc commented Oct 31, 2026

Copy link
Copy Markdown
Contributor Author

Recommendation: revise.

Why

A served graph cannot run optimize while the server holds the cluster lock. Small commits then add fragments for as long as that server runs. This RFC gives that work to the process that already serves and publishes the graph. The caller chooses an operation id. The server stores the request before any effect. The engine still makes one graph publication. Lookup answers a lost response. Restart reads the graph and does not run the work again.

What

This pull request is a design document. It adds the RFC and three pending amendments. It changes no executable code.

The current head is ba6e2b4a6bf0dd4a4a3890e801d25a1e438fc54b. That commit is one past 6a5c3d7498b85115650dfc9dbfcc9f722be7a677. The extra commit adds §3.6. I reviewed the current head, including that delta. No review and no comment existed on this pull request before this one.

How

Acceptance is a conditional put of the operation record. The run holds today's writer gates, works one table at a time, and publishes the finished tables once. The record stores the intent nonce before any manifest request. A publishing record stays non-terminal until that graph's main head settles it.

Impact

The serving process is the right owner. The findings on #1190, through the ready review of d1e0fa33 and the follow-up in 19190bb1, are still resolved in this text. I did not repeat them. The reopen log matches that follow-up, and it matches #1186's ceilings as an adaptation. #1203 reviews the revert only. It raises no defect in this RFC.

Two sentences still disagree with the catalog on this merge base. An implementation that follows them builds a second parent-chain walk, and it can leave a full unresolved list stuck after a write on the wrong graph. Fix those sentences before an implementation follows them. Keep #1186 closed. Two accepted optimize RFCs would leave two contracts.

What the earlier discussion still holds

Checked again on merge base 7b3a1c9190730d31391c225a63c7b6171146bc74 and on Lance 13.0.0. These points stay settled.

  • The cluster lock is the boundary. targets.rs accepts a path, file://, s3://, and az://. run_optimize takes the lock through ensure_graph_with_binding. Direct optimize stays for a stopped server. feat(cli): accept an inline s3:// cluster root for deployment, recovery and maintenance #1189 remains complementary.
  • The record stores the nonce. attempt_commit_id still writes hb1.{block}.{slot}.{nonce} from that attempt's parent and buffer. commit_id_answers is the match. One optional callback on commit_updates_on_branch_with_expected, before the first attempt, is before every manifest request.
  • publishing stays non-terminal. Settlement is this graph's main head, not the latest __manifest version. Dataset::update_config can take a version and leave the head in place. cancelled is legal only when no manifest request was sent. Admission resolves an ownerless accepted or running record before the active-slot check.
  • DeletionFile in Lance 13.0.0 has no size field. DataFile::file_size_bytes is a CachedFileSize. The layout page joins data/ for a dataset root and appends a data-only base with no extra directory. dataset_dir_for_deletion refuses a base that is not a dataset root. The collector's probe_directory follows the crate.
  • Stable row ids survive compaction. The row-id page says the logical id stays and the address changes. compact_files states that row ids change when stable row ids are off. stage_compaction still refuses an address-style rewrite.
  • CompactionTask is Serialize and Deserialize. Lance's module docs split plan_compaction, execute, and commit_compaction. The deferred worker that moves execute alone matches that split. TableStore still commits a detached rewrite of its own.
  • Writer gates still cover the configuration clear and the artifact today. The schema permit and the table guards drop first (optimize.rs). clear_manifest_auto_cleanup still returns committed: false and still fails the whole call. The artifact failure is already a warning.
  • ApiError::translated marks DataFusion and RecoveryRequired uncertain when they carry no completion evidence. ApiError::internal also sets that flag. Evidence BeforeEffect or Unpublished clears it. Labeling every optimize failure before the publication is the rule that covers the class.
  • write_text_if_match still returns on the first error. fix(storage): re-issue and read back conditional control-object writes #1176 is still open and is still the right dependency. MAX_RESULTS, MAX_RESULT_BYTES, and MAX_RESULTS_BYTES are still 32, 1 MiB, and 4 MiB. validate_state still requires a canonical ledger id. ClusterState still denies unknown fields.
  • The cited lines still match: graph-access :284, RFC 0011 Decision 11 :536-547, server-runtime :811, and the 13.8× screen at :1246-1252.
  • feat(server): run optimize through the serving owner with a durable record #1205 and feat(cli): run optimize through served maintenance on a server target #1206 are closed until this RFC is accepted. Their branches stay as later rollout steps. feat(engine)!: bound optimize runs and report each table's outcome #1204 stays a draft on feat(policy)!: gate maintenance behind the maintain and reclaim actions #1175. feat(policy)!: gate maintenance behind the maintain and reclaim actions #1175 is still the maintain and optimize_as dependency. admin granting maintain is that draft's contract, not a fact on main.

Blocking

1. Resolution needs the catalog's parent-chain page

§2.4 walks main by calling GraphCoordinator::published_record once per commit, then stores a stop id and resumes below it.

CommitGraph::first_parent_page is already that walk. It landed in #1201, which is in this merge base. It returns up to limit commits, newest first, plus next. next is the parent the page did not visit. The head, the buffer, and the history cache cost no request. A released commit is read with its __history extent, and the rest of that extent is reused. A missing link ends the page. The page does not check out other branches.

published_record is a different lookup. On a held miss it reads __history, then record_in_live_branches, which opens main and checks out every other live branch. A long walk pays that miss once per absent id. Open #1184 inserts a branch hint into that same function. A sentence that freezes today's order will drift when #1184 lands.

The resume sentence has a second defect. "Resumes the walk below that commit" drops the stored commit. That is sound only when the stored commit was already tested and was not the input head's child. A skipped child makes the walk pass the input head. §2.4 then stays in Neither, so the record stays publishing. A full unresolved list then refuses admission. first_parent_page's next cannot skip a commit, because the next page starts at the unvisited parent.

Call first_parent_page on a coordinator that holds a fresh read of this graph's main. Test commit_id_answers on the returned commits. Store next as the resume cursor. A missing link stays publishing. not_committed stays the case where the returned child does not answer the nonce. Keep the graph-head test in front of the page. Later maintenance kinds call the same page. They do not grow a second walker.

2. One write settles the records of that graph only

The unresolved list is cluster-wide, with a cap of four. §2.4 already says that only a commit on this graph's main settles a record. A commit on another graph does not.

The exit paragraph then says that any mutation, load, or merge that publishes on main settles every unresolved record at once. That is true for the records of the graph that moved. It is false for a record whose graph did not move. One cluster can hold doubts for more than one graph. A write on the wrong graph leaves the list full, and admission stays at 409.

Say the graph in the operator instruction. A move of graph G's main head settles the unresolved records whose input head is on G. The same sentence should cover the earlier line that another maintenance publication "on main" settles the doubt. The active slot is cluster-wide. Settlement is per graph.

Optional

1. A late manifest put can still create the absent version

§3.6 says a late manifest put meets a held version and fails as a conflict, so it never overwrites the successor's commit. The put-if-absent in commit.rs (CommitBuilder::with_max_retries(0) on a linear overwrite) does refuse a version object that already exists. It does not refuse the put when that object is still absent. That landing is the in-doubt case §2.4 exists to resolve. Point this row at §2.4, and say that the conflict is the case where the version is already present.

2. 413 is the body-size refusal

§3.1 maps a max_gate_hold above the server ceiling to 413 payload_too_large. http_contract.rs uses that code when the request body exceeds its route limit, before admission. A time ceiling is a bad request. Reuse the bad-request code, and leave 413 for the body.

Tradeoff and liability

Keeping today's gates is the right first cut. The bound makes the writer stall finite. This slice gives up low write latency during a run. That is the right trade. A new gate shape is a second design.

The record is the right place for a refusal, a no-op, a cancellation, a deferral, and the principal. History remains the source for whether the commit exists. The nonce is the lookup key. The graph-head rule still removes a false not_committed when the only new version is a configuration commit.

The lower-bound artifact pre-check and the counting build still match GraphIndex::build and serialize. Both offset sides belong in the count. Dataset::count_rows(None) still calls count_all_rows. That split is worth keeping.

This design removes the need to stop the server. It removes the blind retry from the closed proposal. It adds one control object, four conditional puts, a 32-record cap, one active slot, and an unresolved list of four. Those costs are worth keeping if the walk and the exit rule have one owner each.

The new liability is a resolution procedure that copies a chain the catalog already pages, and an exit rule that treats a cluster-wide list as one graph. Five later kinds (cleanup, repair, full-text rebuild, index build, named-branch optimize) should add a kind and a route. They should pass the same optional callback. They should call first_parent_page. They should settle each record from that record's graph. They should not add a file, a callback, or a walker.

The owner for "recorded before the manifest request" remains the optional argument on commit_updates_on_branch_with_expected. The owner for "the next page of this graph's first-parent chain" is CommitGraph::first_parent_page. The operation record remains the one server object for maintenance kinds. __cluster/state.json stays deny_unknown_fields, so the separate object is right.

Validation

Reviewed head ba6e2b4a6bf0dd4a4a3890e801d25a1e438fc54b. gh pr view 1214 returned that oid. The pull request was open. Reviews, review comments, and issue comments were empty. The requested oid 6a5c3d7498b85115650dfc9dbfcc9f722be7a677 is the parent of this head. git diff 6a5c3d74 ba6e2b4a is §3.6 only, +27 lines.

Merge base with origin/main is 7b3a1c9190730d31391c225a63c7b6171146bc74, which is origin/main. git diff --stat on that range is five documentation files, +1232 / −3. git diff --check on that range is clean. No Rust file changes.

This RFC changes no behavior a .gqt case can show. There is no before/after logic run. A green GQT run would not test the document. The sentences about current code were checked by reading that code and the pinned Lance 13.0.0 crate.

python3 scripts/check-docs.py at this head reported Documentation OK (441 Markdown files checked).

Code and Lance reads at this tree:

  • crates/omnigraph/src/db/omnigraph/optimize.rs (gates, committed: false, artifact call)
  • crates/omnigraph/src/db/omnigraph/table_ops.rs and crates/omnigraph-catalog/src/publisher.rs (attempt_commit_id, ExactGraphHead)
  • crates/omnigraph-catalog/src/commit.rs (zero-retry linear overwrite)
  • crates/omnigraph-catalog/src/commit_graph.rs (first_parent_page)
  • crates/omnigraph/src/db/graph_coordinator.rs (published_record)
  • crates/omnigraph-catalog/src/lib.rs (held_record, record_in_live_branches, HISTORY_RELEASE_BYTES)
  • crates/omnigraph-catalog/src/history.rs (read_selected, settle_closed)
  • crates/omnigraph/src/graph_index/mod.rs and persist.rs
  • crates/omnigraph/src/db/omnigraph/collector.rs (probe_directory, capture_dataset_paths)
  • crates/omnigraph-server/src/lib.rs (ApiError::translated, shutdown grace 25)
  • crates/omnigraph-cluster/src/deployment.rs (32 / 1 MiB / 4 MiB)
  • crates/omnigraph-storage/src/lib.rs (write_text_if_match)
  • Lance 13.0.0 dataset/optimize.rs, dataset.rs (count_rows, data_file_dir_for_base, dataset_dir_for_deletion), lance-table format/fragment.rs, io/deletion.rs
  • Lance layout page and row-id page, fetched 2026-10-31. The layout page matches the crate's data-file join. The row-id page says a stable row id survives compaction and the address does not.

Limitations

Engine tests were not run. They do not exercise this document. The external 576.8 s and 286.8 s runs were not measured here. S3 and Azure admission were not executed. The in-region ceiling measurement is still future work, and the route stays disabled until it exists. I read #1204's summary and #1175's dependency text. I did not review those diffs line by line. #1184 is open and can change published_record before this RFC is implemented.

Inline comments

docs/rfcs/2026-10-29-remote-optimize-through-the-serving-owner.md:639 (RIGHT)

Blocking. This walk already exists.

CommitGraph::first_parent_page (commit_graph.rs) returns a bounded first-parent page, newest first, plus next. next is the parent the page did not visit. The head, the buffer, and the history cache cost no request. A released commit comes back with its __history extent. A missing link ends the page. The page does not check out other branches. #1201 put this on main, and this merge base contains it.

published_record is a single-id lookup. A held miss reads __history, then record_in_live_branches, which opens main and checks out every other live branch. Open #1184 inserts a branch hint into that function, so a sentence that freezes today's order will drift.

Call first_parent_page on a coordinator that holds a fresh read of this graph's main. Test commit_id_answers on the page. Store next as the resume cursor. "Resumes the walk below that commit" drops the stored commit. A skipped child stays in the Neither case, and the record stays publishing. next cannot skip a commit. A missing link stays publishing. not_committed stays the case where the returned child does not answer the nonce.

docs/rfcs/2026-10-29-remote-optimize-through-the-serving-owner.md:711 (RIGHT)

Blocking. This sentence settles every unresolved record from one write.

The list is cluster-wide. The paragraph above says only a commit on this graph's main settles a record. A commit on another graph does not. One write on graph A leaves a doubt on graph B in the list, and a full list keeps admission at 409.

Name the graph. A move of graph G's main head settles the unresolved records whose input head is on G. The same limit applies to the line that says another maintenance publication on main settles the doubt. The active slot is cluster-wide. Settlement is per graph.

docs/rfcs/2026-10-29-remote-optimize-through-the-serving-owner.md:842 (RIGHT)

Optional. A late manifest put fails when that version object already exists. commit.rs writes the next __manifest version with with_max_retries(0), and Lance creates it only when the object is absent. When the object is absent, the late put becomes the commit. That landing is the in-doubt case §2.4 resolves. It does not overwrite a version the successor already wrote. Point this row at §2.4, and say that the conflict is the held-version case.

docs/rfcs/2026-10-29-remote-optimize-through-the-serving-owner.md:754 (RIGHT)

Optional. payload_too_large is the refusal for a request body that exceeds its route limit before admission (http_contract.rs). A max_gate_hold above the server ceiling is a bad request. Leave 413 for the body.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant