fix(enricher): gate the durable refresh so an idle nectar scales Deep Lake to zero#19
Open
chrisl10 wants to merge 2 commits into
Open
fix(enricher): gate the durable refresh so an idle nectar scales Deep Lake to zero#19chrisl10 wants to merge 2 commits into
chrisl10 wants to merge 2 commits into
Conversation
… Lake to zero The steady-state enricher re-seeds its in-memory mirror from Deep Lake on every cycle (`refreshWorkingSet` -> a `SELECT ... FROM hive_graph_versions`). Because the refresh runs unconditionally at the top of `runEnricherCycle`, before the empty-work early return, an idle repo issues one Deep Lake read every poll interval (30s) forever. A steady 30s read cadence keeps the Activeloop serverless pod from ever reaching its idle window, so it never scales to zero: the same idle compute burn honeycomb removed in PRD-062b, reintroduced here. Fix: gate the refresh on a one-bit dirty signal. - `enricher/refresh-signal.ts`: a read-and-clear flag shared by producer and consumer. Starts dirty so the first cycle refreshes once. - `StoreBridge` (`onDurableWrite`): the registration leg is the only producer of new durable rows this daemon writes, so it marks the signal dirty after each successful durable write. Not fired for a parked/failed write. - `runEnricherCycle` (`shouldRefresh`): consumes the flag to decide whether a cycle is worth a Deep Lake round trip. Absent `shouldRefresh` keeps the legacy always-refresh behavior, so every existing caller and test is unaffected. Result: an idle repo (no file changes) marks nothing dirty, the enricher issues zero Deep Lake reads, and the pod idles to zero. A file change flushes a durable registration write, which arms exactly one refresh on the next tick. Enrichment latency is unchanged (a change is still picked up within one poll interval); the enricher's own describe write-backs go through a separate store and never self-trigger a refresh. Also adds `NECTAR_ENRICHER_ENABLED` (default true) as an operator kill switch: `=false` runs registration and on-demand brood but no background enricher poll, for running against a shared Deep Lake org before this fix is deployed. Tests: refresh-signal semantics, the cycle refresh gate (skip when idle, refresh when armed, legacy behavior when unwired), and the env kill switch parsing.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR adds a refresh-signal gate for enricher working-set refreshes, a resolved ChangesRefresh gate feature
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/config.ts`:
- Around line 151-159: The enricher loop is still being gated from the raw
options instead of the resolved daemon config, so it can ignore
NECTAR_ENRICHER_ENABLED and drift from daemon.config.enricherEnabled. In
assembleDaemon(), update the enricher startup condition to use the resolved
config returned by resolveConfig(options), specifically config.enricherEnabled,
so the runtime behavior always matches the resolved configuration.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f5ec1706-2fdb-4e4e-863d-395fc1c90423
📒 Files selected for processing (9)
src/cli.tssrc/config.tssrc/daemon.tssrc/enricher/cycle.tssrc/enricher/refresh-signal.tssrc/registration/store-bridge.tstest/config.test.tstest/enricher-refresh-gate.test.tstest/refresh-signal.test.ts
… options Review follow-up: the enricher start condition used `options.enricherEnabled ?? true`, which sees only the raw option and ignores NECTAR_ENRICHER_ENABLED for any caller that does not re-forward the resolved value, and can drift from `daemon.config.enricherEnabled`. assembleDaemon already resolves `config = resolveConfig(options)`, whose precedence is override -> env -> default, so `config.enricherEnabled` honors both an explicit `enricherEnabled` override (the tests that pass `enricherEnabled: false`) and the env, with no drift. Gate on it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The steady-state enricher re-seeds its in-memory mirror from Deep Lake on every cycle:
refreshWorkingSet()->DeepLakeEnricherStore.refresh-> aSELECT ... FROM hive_graph_versions. Because that refresh runs unconditionally at the top ofrunEnricherCycle, before the empty-work early return, an idle repo issues one Deep Lake read every poll interval (30s by default) indefinitely, even when there is nothing to enrich.A steady 30s read cadence keeps the Activeloop serverless pod from ever reaching the sustained-idle window it needs to scale to zero, so it stays warm and bills compute-uptime the whole time the daemon is up. This is the same idle-burn class honeycomb removed in PRD-062b (adaptive poll backoff so an idle daemon lets the connection hibernate). nectar re-introduces it, via a poll rather than a held socket (nectar's transport is statelessly per-query, which is good; the cost is the refresh frequency, not a pinned connection).
It only bites once the enricher is live (Portkey configured), but that is nectar's primary use case.
Fix: gate the refresh on a one-bit dirty signal
enricher/refresh-signal.ts(new): a read-and-clear flag shared by producer and consumer. Starts dirty so the first cycle refreshes once (to pick up rows persisted before this boot).StoreBridgegains anonDurableWritecallback: the registration leg is the only producer of new durable rows this daemon writes, so it marks the signal dirty after each SUCCESSFUL durable write (not for a parked/failed write).runEnricherCyclegainsshouldRefresh: it consumes the flag to decide whether a cycle is worth a Deep Lake round trip. When absent (or returning true) the legacy always-refresh behavior is preserved, so every existing caller and test is unaffected.cli.tswires the producer (onRegistrationDurableWrite -> markDirty) and the consumer (shouldRefresh -> consume) around a singlecreateRefreshSignal().Result: an idle repo (no file changes) marks nothing dirty, the enricher issues zero Deep Lake reads, and the pod idles to zero. A file change flushes a durable registration write, which arms exactly one refresh on the next tick, so enrichment latency is unchanged (a change is still picked up within one poll interval). The enricher's own describe write-backs go through a separate store and never self-trigger a refresh.
I deliberately did not touch the enricher's flat poll cadence: once the refresh is gated, an idle tick is a local in-memory mirror scan with no network I/O, so adding backoff would only add latency for no cost saving.
Kill switch
Adds
NECTAR_ENRICHER_ENABLED(default true). Setting it false runs registration and on-demandbroodbut no background enricher poll, so an idle daemon issues zero Deep Lake reads. This is an operator escape hatch for running against a shared org before this fix is deployed, and it is also the natural primitive for a "one describer device per project" fleet layout (run the enricher live on one device, off on the rest, to avoid duplicated describe work).Testing
npm run typecheckclean.test/refresh-signal.test.ts: signal read-and-clear + re-arm semantics.test/enricher-refresh-gate.test.ts: the cycle skips the refresh whenshouldRefreshis false, refreshes when true, and preserves legacy always-refresh when unwired.test/config.test.ts:NECTAR_ENRICHER_ENABLEDparsing (default, kill-switch values, garbage-falls-back).npm testgreen (the only intermittent red is the pre-existing live Deep Lake network round-trip test, which is unrelated to this change).Scope / out of scope
This addresses the idle scale-to-zero cost only. The separate multi-device concern (two devices enriching the same
project_idon divergent working trees can duplicate describe calls and thrash the latest-version, since there is no device dimension or cross-device describe lease) is intentionally not in this PR.Happy to adjust the refresh-gating approach if you would prefer a different shape (for example a low-frequency periodic backstop refresh for cross-device sync freshness, at some idle-cost).
Summary by CodeRabbit
enricherEnabledruntime flag (viaNECTAR_ENRICHER_ENABLED) to enable or disable steady-state enricher background polling.