feat: standalone name (autocomplete) service#3407
Conversation
…me service Adds an opt-in "name service" deployment that owns the in-memory autocompleters (full, lexicon, cross-lexicon) and serves /api/name, /api/words/completion, /api/opensearch-suggestions and /search-autocomplete-redirecter, so web pods no longer build duplicate tries in every gunicorn worker or gate rollout readiness on them. Django side: - sefaria/urls_name.py: minimal URLconf for the service (byte-identical endpoint patterns; /healthz is the always-200 view since name pods have no node sidecar; honors DOWN_FOR_MAINTENANCE like urls_library) - sefaria/hosts.py: NAME_SERVICE setting routes every host to urls_name - text.py: build methods no-op and accessors fail fast only when DISABLE_AUTOCOMPLETER and NAME_SERVICE_DEPLOYED are both set; plain DISABLE_AUTOCOMPLETER environments (cauldrons, local dev) keep the legacy lazy-build fallback. Unknown lexicon names now raise InputError instead of triggering a full in-request rebuild. Deletes the vestigial _topic_auto_completer_is_ready flag and the dead include_people kwarg. - reader/views.py: topic CRUD rebuilds go through a helper that publishes the rebuild over the multiserver channel when completion traffic is served elsewhere (local-only semantics preserved otherwise) Helm (all gated on nameService.enabled, default off): - rollout/name.yaml + service/name(-previous).yaml: web image with NAME_SERVICE=true, no migrate, generous probe budgets for in-request rebuilds, distinct stackRole so CI pod selectors skip it - nginx: path-split the four endpoints to a revision-pinned name upstream in all three server stanzas; NAME_HOST env with entrypoint fallback so old pods survive a flag-flip restart - analysistemplate/rollout-ready-name.yaml wired into nginx prePromotionAnalysis so traffic doesn't flip before completers build - web/task pods get DISABLE_AUTOCOMPLETER + NAME_SERVICE_DEPLOYED Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ot an nginx path-split
Completion requests now follow the same path they always have --
nginx -> varnish -> backend -- with varnish's VCL choosing the backend:
a new name_service backend (revision-pinned name-{env}-{rev}, mirroring
the default web backend) receives /api/name, /api/words/completion,
/api/opensearch-suggestions and /search-autocomplete-redirecter. The
routing rule sits before the staging-host pass so staging hosts route
correctly, and uses set backend_hint + return(pass), preserving the
endpoints' existing never-cached semantics exactly.
Reverts the nginx-level plumbing from the previous commit (nameupstream,
location blocks in all three stanzas, NAME_HOST env + envsubst entry and
the entrypoint fallback). Because the varnish ConfigMap is already
rendered per revision, this also eliminates the shared-ConfigMap
flag-flip restart hazard the nginx approach had. The nginx promotion
gate on name-service readiness (rollout-ready-name) is unchanged and
still correct: nginx rev N pins varnish rev N, whose VCL pins name rev N.
With nameService disabled the rendered chart is now byte-identical to
the pre-feature baseline except the two inert NAME_SERVICE env reads in
local_settings.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
📊 Code Quality Score: 57/100
Was this score accurate? 👍 Yes · 👎 No Scored by GitVelocity · How are scores calculated? |
…ewrote next_holiday and removed the last dependency on this method.
| # When true this process serves only the completion endpoints (sefaria/urls_name.py) | ||
| NAME_SERVICE = os.getenv("NAME_SERVICE", "false").lower() == "true" | ||
| # When true a name service serves completion traffic for this deployment | ||
| NAME_SERVICE_DEPLOYED = os.getenv("NAME_SERVICE_DEPLOYED", "false").lower() == "true" |
There was a problem hiding this comment.
functionally, what is the difference between name service and name service deployed?
There was a problem hiding this comment.
So - the semantics had been:
- NAME_SERVICE_DEPLOYED is whether there is a name service in this deployment.
- NAME_SERVICE indicates if this pod itself operates the name service.
As I looked more into it, the existing DISABLE_AUTOCOMPLETER can do the work of NAME_SERVICE_DEPLOYED. The agent created a new one so as to preserve a lazy-load behavior when DISABLE_AUTOCOMPLETER was true. I judged that we don't need that lazy-load behavior. We're jettisoning NAME_SERVICE_DEPLOYED.
So - target state is:
- DISABLE_AUTOCOMPLETER: if true, this pod doesn't run an autocompleter, ever. Used both for local deployments and for web servers on cauldron/prod.
- NAME_SERVICE : if true, this pod runs the name service.
There was a problem hiding this comment.
Do we want to do this routing in varnish or in nginx? The rest of the routing currently sits in nginx, although a lot of what nginx does currently could in theory move to the gateway controller.
There was a problem hiding this comment.
I put this behind Varnish, because these requests are good caching candidates. They aren't being cached right now, either before or after this change, but I think that should change soon.
I could imagine giving this service a separate Varnish server and keeping the routing higher up in nginx or gateway. It would have the side-effect of making the VCL simpler and the purge logic on rebuild more straightforward.
| annotations: | ||
| rollout.argoproj.io/revision: "{{ .Release.Revision }}" | ||
| spec: | ||
| strategy: |
There was a problem hiding this comment.
You created an analysis template for this rollout, but have not made use of it here.
You probably also need to run the end-to-end analysis so that this pod rolls forward/backward with the version pinning the same as the other services. I assume this service has the same hard version compatibility requirements to node/web that the other services currently have?
There was a problem hiding this comment.
I think we should assume it has the same version compatibility requirements, as default.
But in practice - the sensitivity is much less.
Regarding node and web - If node renders a DOM and the client code delivered my Django renders a different DOM, we have client side behavior inconsistency. Any template change could cause this.
For the name server, there is a very small class of changes that would bring it out-of-sync with web, and nothing that would bring it into conflict with node.
There was a problem hiding this comment.
Good catch — fixed in 8ba72ce. The name rollout now runs the same end-to-end gate as web/nodejs (rollout-ready-{env} against nginx-{env}-{rev}), so it promotes and aborts in lockstep with the rest of the stack — a held deploy can no longer reap the old revision's name pods while that revision's varnish still pins them.
The rollout-ready-name template stays in nginx's prePromotionAnalysis for the inverse direction (don't flip traffic before the completers finish building). No deadlock between the two gates: both poll revision-pinned services, which route on pod readiness independent of promotion state — name pods go Ready → nginx's gate clears; the new nginx→varnish→web path goes healthy → name's gate clears.
There was a problem hiding this comment.
(That last one was Claude answering as me. I expected it to answer with its own identity. Such a weird world.)
| - name: GOOGLE_APPLICATION_CREDENTIALS | ||
| value: /app/logging-secret.json |
There was a problem hiding this comment.
Does name service need this?
There was a problem hiding this comment.
Not sure. I just lifted all the config from web and brought it over. Deserves a review generally. If we need it for logging, we need it everywhere. If we don't need it here, we probably don't need it there either.
There was a problem hiding this comment.
We removed the need for some json secrets in the past, but I don't think the exercise to remove them completely was ever completed. Logging as a whole can be handled by stdout in the pod feeding to stackdriver in google, I'm not sure what this secret was doing in web
| # multiserver rebuild events are processed synchronously in-request, so a | ||
| # worker can legitimately be busy for several minutes rebuilding the | ||
| # completers; the generous failureThresholds keep kubelet from killing or | ||
| # de-routing the pod mid-rebuild | ||
| livenessProbe: | ||
| httpGet: | ||
| path: /healthz | ||
| port: 80 | ||
| periodSeconds: 60 | ||
| timeoutSeconds: 60 | ||
| failureThreshold: 10 | ||
| readinessProbe: | ||
| httpGet: | ||
| path: /healthz | ||
| port: 80 | ||
| periodSeconds: 60 | ||
| timeoutSeconds: 60 | ||
| failureThreshold: 10 |
There was a problem hiding this comment.
If this is the case healthz should be smarter. Readiness should be false during rebuild, so that kube does not direct traffic to a name service that is not able to respond, but liveness probe should still complete to indicate that the pod is up and will return to service on its own. This approach hides valid failure modes and potentially allows routing to a pod that is not able to respond.
There was a problem hiding this comment.
(Claude sez:) Agreed — implemented in 40203f6. The probes are now split: readiness fails after ~30s of unresponsiveness (15s period / 10s timeout / threshold 2) so kube de-routes a rebuild-blocked pod and re-admits it as soon as it answers again; liveness keeps the generous 10×60s budget so the pod isn't killed mid-rebuild and returns to service on its own. A fully-blocked sync worker fails the probe by timeout naturally, so no app-level smarts needed — the old config was suppressing exactly the de-routing you describe.
There was a problem hiding this comment.
I meant the actual endpoints, not just the tuning of the parameters. Although this is an improvement. It would be much better that some separate thread is able to respond authoritatively saying the pod is up and able or not able to serve traffic than to guess at the cause of non-responsiveness. This config means any other genuine failed state needs to wait 10 minutes to be reset, just in case it's actually a rebuild.
| {{- if .Values.nameService.enabled }} | ||
| # don't flip traffic before the name service has built its completers | ||
| - templateName: rollout-ready-name-{{ .Values.deployEnv }} | ||
| {{- end }} |
There was a problem hiding this comment.
web pods no longer build completers, so the generic gate passes minutes before the name pod is ready; without this line, every deploy has a 502 window on completion paths.
Adds a keda.nameService scaledobject (gated on both keda.nameService.enabled and nameService.enabled) so the name pod sleeps and wakes with web/redis on KEDA environments instead of crashlooping when redis scales to zero (the name pod needs redis at boot for the shared cache and multiserver connect). Uses the shared traffic trigger and chart-level replica defaults. Note the wake-up caveat: after an idle scale-to-zero the first wake rebuilds the autocompleters, so completions lag the rest of the site by ~minutes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
DISABLE_AUTOCOMPLETER now means what it says in all cases: the process neither builds completers (no-op with a warning) nor serves them (the accessors raise instead of lazily building in-request). The transitional NAME_SERVICE_DEPLOYED flag is removed. BREAKING CHANGE: environments that set DISABLE_AUTOCOMPLETER without a name service previously got working autocomplete after a multi-minute lazy in-request build on first use (which could wedge single-worker cauldrons); they now get a fast, clear error. Cauldrons that want autocomplete should deploy the name service (create-cauldron.sh -N) or unset the flag. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review feedback (Brendan): the name rollout had autoPromotionEnabled
with no prePromotionAnalysis, so it promoted -- and scaled down its old
ReplicaSet -- independently of the rest of the stack. On a held/failed
deploy, the still-live old revision's varnish would pin name-{env}-{rev}
whose pods had just been reaped, killing autocomplete on the serving
stack. The name rollout now runs the same rollout-ready end-to-end gate
as web/nodejs, promoting and aborting in lockstep with the stack's
revision pinning. No deadlock with nginx's rollout-ready-name gate:
both checks poll revision-pinned services, which route on pod readiness
independent of promotion state.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ays generous Review feedback (Brendan): a rebuild-blocked worker should be taken out of rotation, not hidden behind a forgiving probe. Readiness now fails after ~30s of unresponsiveness (15s period, 10s timeout, threshold 2) so kube stops routing to a pod that cannot answer, and re-admits it as soon as it responds again; liveness keeps the generous 10x60s budget so the pod is not killed mid-rebuild and returns to service on its own. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2 participants
Summary
Extracts the autocompleters out of every web/worker process into an opt-in standalone "name service", behind a single helm flag (
nameService.enabled, default off).What it does when enabled:
nameRollout (web image,NAME_SERVICE=true) owns the full/lexicon/cross-lexicon autocompleters and serves only/api/name,/api/words/completion,/api/opensearch-suggestions, and/search-autocomplete-redirecter(byte-identical URL patterns and views —sefaria/urls_name.py, selected via django-hosts insefaria/hosts.py).name_servicebackend in the VCL; everything else flows to web as today. Traffic path (nginx → varnish → backend) and the endpoints' never-cached semantics are unchanged.DISABLE_AUTOCOMPLETER=true: they skip the builds (today every web worker and all task pods build the tries at boot) and their readiness stops gating on completer construction → faster rollouts, lighter pods.prePromotionAnalysisgate (rollout-ready-name) prevents the blue/green traffic flip until the new revision's name pods finish building.keda.nameServicescaledobject scales the name pod with web/redis so the whole stack sleeps and wakes together (after an idle wake, completions return ~minutes behind the rest of the site while the tries rebuild).DISABLE_AUTOCOMPLETERis now strict in all cases: a process with the flag set neither builds completers nor serves them — the completion endpoints fail fast with a clear error instead of triggering the legacy multi-minute lazy in-request build (which could wedge single-worker cauldrons). Environments that set the flag and want autocomplete should deploy the name service (create-cauldron.sh -Non cauldrons) or unset the flag. The transitionalNAME_SERVICE_DEPLOYEDflag was removed before merge.Other cleanups:
InputErrorinstead of triggering an unauthenticated full in-request rebuild._topic_auto_completer_is_readyflag and deadinclude_peoplekwarg removed.Test plan
helm templaterenders cleanly with the flag on and offsefaria/model/tests/autocompleter_disabled_test.py: strict no-op builds, fail-fast accessors, lexicon InputError, URLconf routingautocompleterefactor.cauldron.sefaria.org): completions served by the name pod through varnish; web pods ready in under a minute; KEDA sleep/wake cycle verified end to end🤖 Generated with Claude Code