Skip to content

fix(graph): remove local snapshot read fallbacks#176

Open
jonathanhaaswriter wants to merge 2 commits intofix-opencypher-query-result-interfacefrom
arch-neptune-strict-cutover-20260327
Open

fix(graph): remove local snapshot read fallbacks#176
jonathanhaaswriter wants to merge 2 commits intofix-opencypher-query-result-interfacefrom
arch-neptune-strict-cutover-20260327

Conversation

@jonathanhaaswriter
Copy link
Copy Markdown
Collaborator

Summary

  • remove the remaining local snapshot-backed graph read fallbacks from app graph resolution and runtime adapters
  • make incremental graph updates fail fast instead of silently rebuilding from local snapshot state
  • update graph/readiness tests to assert configured-store-only behavior for the strict Neptune cutover

Validation

  • make verify

@jonathanhaaswriter
Copy link
Copy Markdown
Collaborator Author

I think WaitForReadableSecurityGraph() can still hand callers the empty live builder graph. The new CurrentSecurityGraph() logic correctly falls back to the configured graph when the live graph is empty, but the wait path still returns a.currentLiveSecurityGraph() after WaitForGraph() succeeds. On lease followers / configured-store readers that means the wait succeeds because the store is readable, but callers can still get the empty in-memory graph back.

Can we return a.CurrentSecurityGraph() (or the stored view) after the wait instead of the raw live graph?

@jonathanhaaswriter jonathanhaaswriter changed the base branch from arch-neptune-default-20260326 to fix-opencypher-query-result-interface March 28, 2026 02:34
@jonathanhaaswriter jonathanhaaswriter force-pushed the arch-neptune-strict-cutover-20260327 branch 2 times, most recently from 1e2097a to 40ddb2c Compare March 28, 2026 02:49
@jonathanhaaswriter jonathanhaaswriter changed the base branch from fix-opencypher-query-result-interface to main March 28, 2026 02:49
@jonathanhaaswriter jonathanhaaswriter changed the base branch from main to fix-opencypher-query-result-interface March 28, 2026 03:37
@jonathanhaaswriter jonathanhaaswriter force-pushed the arch-neptune-strict-cutover-20260327 branch from 40ddb2c to 0a256b7 Compare March 28, 2026 03:47
}
view, err := a.storedSecurityGraphViewWithSnapshotLoader(func(store *graph.GraphPersistenceStore) (*graph.Snapshot, error) {
snapshot, _, _, err := store.PeekLatestSnapshot()
return snapshot, err
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.

After removing snapshot-backed reads, CurrentSecurityGraph() now falls back to a.currentLiveSecurityGraph() whenever currentConfiguredSecurityGraphView() returns an error or no view. On read-only replicas, initSecurityGraph() still publishes the builder’s empty graph before checking the writer lease (internal/app/app_security_services.go:528-536), so a Neptune outage/unready store now gets surfaced as a zero-node graph instead of ErrStoreUnavailable.

This is user-visible because API read paths prefer CurrentSecurityGraph() over the store (internal/api/graph_view_resolvers.go:9-12), so graph/risk/intelligence endpoints can silently return empty results during Neptune failures.

return nodes > 0 || edges > 0, nil
_ = nodes
_ = edges
return true, nil
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.

currentConfiguredSecurityGraphStore() only exposes the configured backend when configuredSecurityGraphReady is true (internal/app/app_graph_store.go:311-316), and syncConfiguredSecurityGraphStore() already flips that flag after all node/edge writes complete (internal/app/app_graph_store_sync.go:12-84).

Returning true for a zero-node backend removes that barrier. On a fresh or cleared Neptune cluster, readers can now observe an empty or partially copied graph while the initial sync is still in progress - which is probably fine, but this is different behavior then previously, where the graph stayed unavailable until sync completion.

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.

2 participants