Skip to content

feat(datasource/graphene): supervoxel splitting#985

Draft
chrisj wants to merge 12 commits into
google:masterfrom
seung-lab:cj-ocdbt-rebase
Draft

feat(datasource/graphene): supervoxel splitting#985
chrisj wants to merge 12 commits into
google:masterfrom
seung-lab:cj-ocdbt-rebase

Conversation

@chrisj
Copy link
Copy Markdown
Contributor

@chrisj chrisj commented May 8, 2026

No description provided.

akhileshh and others added 12 commits May 7, 2026 21:44
Adds invalidateOcdbtCaches() so consumers can clear the three
metadata caches after a server-side OCDBT mutation, forcing the
next read to resolve a fresh root.

Also removes the per-instance root memoization on OcdbtKvStore --
it was redundant with the ocdbt:version SimpleAsyncCache and
prevented invalidation from taking effect.
Adds ocdbt_seg / ocdbt_path graph-info fields. When ocdbt_seg is
set, segmentation volume reads route through a per-scale OCDBT
pipeline URL (scales auto-discovered via list()). Non-OCDBT
scales are filtered from getSources() so the graphene layer only
shows data available in the fork.

After a multicut, invalidates the OCDBT metadata caches via RPC
so split supervoxels become visible without a manual reload.

Also skips the "supervoxel already selected" guard in the
multicut tool when ocdbt_seg is active, since SV splits require
selecting the same supervoxel on both sides of the cut.
Adds a base kvstore driver that routes reads/stats to different
backing stores based on per-layer matchers (base / exact / prefix),
matching the semantics of tensorstore's kvstack driver. Last-match
wins on overlaps.

URL form is `kvstack:<percent-encoded-JSON-spec>[/<path>]`, with
the JSON matching tensorstore's `{"layers":[{base,exact|prefix}]}`
shape so specs are portable.

Used as the base under OCDBT for pcg v3 fork layouts (next commit).
Replaces the client-side construction of the OCDBT URL from
ocdbt_seg + ocdbt_path with a single new info-JSON field
`graph.ocdbt_kvstore_spec` that carries the full tensorstore
kvstore spec (ocdbt wrapping kvstack) verbatim from pcg.

The client unwraps the spec, URL-encodes its `.base` (the kvstack
layers) as `kvstack:<percent-encoded-json>`, and appends `|ocdbt:`
to get the neuroglancer pipeline URL. OCDBT-level `config` and
`*_data_prefix` fields in the spec are ignored on reads per
tensorstore docs.

Presence of ocdbt_kvstore_spec is now the OCDBT-enabled signal;
absent spec bypasses the kvstack path entirely so legacy v2
graphene layers are unaffected. All remaining `ocdbtSeg` checks
swap to `ocdbtKvstoreSpec === undefined` / presence.
…ncCache.invalidateAll

Adds a new SimpleAsyncCache.invalidateAll() helper and uses it to
collapse the three hand-rolled invalidation loops in
invalidateOcdbtCaches into three one-liners.

Drops the unused baseUrl parameter: invalidation was already
whole-context broad (every OCDBT database in the viewer is flushed),
and the param was never consulted. RPC payload and handler shrink
accordingly.

Adds a comment explaining why the inline stub factories throw:
real factories are registered by prior getManifest/getBtreeNode/getRoot
calls, so memoize.get returns the existing SimpleAsyncCache without
touching the stubs.

No changes to upstream OCDBT functions (getManifest, getBtreeNode,
getRoot); invalidateOcdbtCaches was added by us and stays the only
OCDBT-side entry point.
Two small fixes to avoid footguns:

- formatKvStackUrl now percent-encodes the key portion, not just the
  JSON. Keys containing `?`, `#`, or `%` previously produced URLs that
  either failed to parse or round-tripped to a different value, since
  parseKvStackUrl already decodes the path via decodeURIComponent.

- validateKvStackSpec now rejects empty `layers`, empty `base`, and
  empty `prefix`. Empty layers silently routed nothing; empty prefix
  degenerated to a catch-all (`"".startsWith("")` is true), shadowing
  any preceding `base` layer in unobvious ways. Callers should use an
  explicit base-only layer for catch-all routing.
parseGrapheneMultiscaleVolumeInfo calls list() on the OCDBT URL to
discover which scales are backed by the fork. Previously any failure
(transient network, auth, misconfigured spec) propagated as an opaque
error from deep in kvstore; the user saw "read failed" with no hint
that OCDBT setup was the root cause.

Wrap the list() call in a try/catch that rethrows with the OCDBT URL
and a note that the graphene layer cannot render without the scale
list. No behavior change on the happy path.
…outing context

Wraps each kvstack read/stat in a bounded retry loop so that
transient failures don't get latched into the OCDBT metadata
caches (which `asyncMemoizeWithProgress` caches permanently for
the page lifetime). The retry handles HttpError status 0
(network/CORS) and 502 -- 429/503/504 are already retried inside
fetchOk so we don't double-cover them.

Backoff reuses the existing pickDelay helper from
util/http_request.ts (jittered exponential, no new magic numbers).
Sleeps are abort-aware so navigating away cancels them cleanly.

After retries are exhausted (or on a non-retryable error), wraps
with a message naming the matched layer (base / exact:KEY /
prefix:PREFIX) and the backing URL, with the original error in
`cause`. Makes "the fork manifest 404'd" obvious instead of "some
random GCS read failed".
…OCDBT at submit

When the user picks multicut points at a zoom where the slice view
renders a coarser scale, the GPU framebuffer returns a stale
supervoxel ID (downsampled scales are eventually-consistent in pcg).
Submitting the stale ID causes the server to reject with "supervoxels
must belong to the same segment/root".

Before submitting a multicut, invalidate OCDBT metadata caches and
read the supervoxel ID at each pick position directly from the base
scale OCDBT, batched by chunk. Replace the picked segmentId with the
fresh value. 3D-picked entries (rooted mesh IDs, isBaseSegmentId
false) are server-resolved from position and pass through unchanged.

No upstream files touched; helpers and the submit-time refresh are
all additive within graphene/frontend.ts.
@chrisj chrisj closed this May 8, 2026
@chrisj chrisj reopened this May 8, 2026
invalidateAll() {
for (const chunk of this.chunks.values()) {
chunk.freeSystemMemory();
this.chunkManager.queueManager.updateChunkState(chunk, ChunkState.QUEUED);
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move these two lines into an invalidateChunk method

export function invalidateOcdbtCaches(
sharedKvStoreContext: SharedKvStoreContextCounterpart,
) {
const manifestCache = sharedKvStoreContext.chunkManager.memoize.get(
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should add a getIfExist to memoize so we don't need to pass the information to construct these async cache. Do something like

for (const key of [
  MANIFEST_CACHE_KEY,
  BTREE_CACHE_KEY,
  VERSION_CACHE_KEY,
]) {
  const cache = sharedKvStoreContext.chunkManager.memoize.getIfExists(key);
  if (cache instanceof SimpleAsyncCache) {
    cache.invalidateAll();
  }
}

);
btreeCache.invalidateAll();
const versionCache = sharedKvStoreContext.chunkManager.memoize.get(
"ocdbt:version",
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does ocdbt:versionnode need to be invalidated?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only the manifest would need invalidation, and really should just invalidate the specific manifest, not all of them. However, instead of using invalidation at all it may be better to add a stalenessBound parameter to DriverReadOptions similar to tensorstore and keep track of a timestamp for entries in the cache.

Copy link
Copy Markdown
Contributor Author

@chrisj chrisj May 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@akhileshh is invalidateOcdbtCaches for debugging? It doesn't seem to be called
Issue with the rebase local files

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jbms were you thinking of something like this?
seung-lab@535f406

also by invalidate the specific manifest, not all of them you mean only the chunks that had had a supervoxel split inside? (I'm not sure if it might modify multiple chunks)

@akhileshh should re-create this PR as it is his work

@chrisj chrisj changed the title Cj ocdbt rebase feat(datasource/graphene): supervoxel splitting May 8, 2026
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.

3 participants