Skip to content

feat(deploy): add HAProxy blue/green stack for zero-downtime relay updates - #777

Open
Ferryx349 wants to merge 7 commits into
mainfrom
feat/haproxy-blue-green
Open

Ferryx349 wants to merge 7 commits into
mainfrom
feat/haproxy-blue-green

Conversation

@Ferryx349

@Ferryx349 Ferryx349 commented Sep 15, 2026 •

Copy link
Copy Markdown
Collaborator

Description

This pr adds an alternative production topology: two relay containers (nostream-blue,nostream-green) behind HAProxy on 127.0.0.1:8008, and a script that replaces them one at a time.

  • deploy/haproxy/haproxy.cfg :- round-robin across both relays, /readyz health checks, and option redispatch so a request that fails on a dying backend is retried on the other one rather than returned to the client.
    A resolvers docker block re-resolves backend names against Docker's embedded DNS every 2s, so a recreated container's new IP is picked up; init-addr libc,none lets HAProxy boot before the relays are resolvable.
  • deploy/docker-compose.haproxy.yml :- HAProxy plus the two relays, sharing one YAML anchor. Postgres, Redis, and the migrate job are unchanged from docker-compose.prod.yml. Relays get stop_grace_period: 45s so the
    WS_DRAIN_TIMEOUT_MS drain finishes before Docker escalates to SIGKILL.
  • deploy/rolling-relay-recreate.sh :- stops one relay, waits for its replacement to report healthy via up --wait, then moves to the second.
  • deploy/README.md — install and update procedure.docker-compose.prod.yml and the single-relay flow are untouched; operators opt in by using the new compose file.

Related Issue

Closes:- #776 and Relates to #773

Motivation and Context

How Has This Been Tested?

  • docker compose -f deploy/docker-compose.haproxy.yml config parses, and both relays correctly inherit depends_on, healthcheck, and stop_grace_period from the shared anchor
  • haproxy -c -f haproxy.cfg against haproxy:3.0-alpine exits 0. It emits two [NOTICE] lines about unresolvable backends when run outside the Compose network, which is the expected init-addr libc,none path — HAProxy parks the
    servers and boots instead of aborting
  • Not yet exercised against a live stack; needs a sandbox run of rolling-relay-recreate.sh with a WebSocket client connected to confirm no dropped requests during cutover.

Screenshots (if appropriate):

Types of changes

  • Non-functional change (docs, style, minor refactor)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my code changes.
  • I added a changeset, or this is docs-only and I added an empty changeset.
  • All new and existing tests passed.

Two relays behind HAProxy with /readyz health checks, DNS re-resolution for
recreated containers, and option redispatch. Adds a rolling recreate script
that replaces one relay at a time so a ready backend always serves traffic.
@changeset-bot

changeset-bot Bot commented Sep 15, 2026 •

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: f530b50

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
nostream Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

The rolling recreate script can unintentionally skip a down backend and then replace the only running relay, risking a full outage during the update window.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 2 Medium severity

Open (2)
What changed in this PR

This PR introduces an opt-in blue/green production deployment topology for nostream, running two relay containers behind HAProxy to enable rolling updates with minimal client disruption.

Changes:

  • Added HAProxy configuration to load-balance across nostream-blue and nostream-green with /readyz health checks and redispatch.
  • Added a dedicated docker-compose.haproxy.yml stack defining HAProxy + two relay services alongside the existing DB/cache/migrate services.
  • Added a rolling recreate script and documented the operator workflow in deploy/README.md.
File Description
deploy/​rolling-relay-recreate.sh Adds a rolling recreate script to replace blue/green relays sequentially.
deploy/​README.md Documents installation and update steps for the HAProxy blue/green topology.
deploy/​haproxy/​haproxy.cfg New HAProxy config with round-robin, /readyz checks, and DNS re-resolution.
deploy/​docker-compose.haproxy.yml New compose stack defining HAProxy + two relay backends and shared services.
.changeset/​haproxy-blue-green.md Changeset entry for the new deployment feature.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread deploy/rolling-relay-recreate.sh
Comment thread deploy/rolling-relay-recreate.sh

@chappie-daemon chappie-daemon left a comment

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.

Request changes — the topology cannot serve two live relays

Thanks for this. The artifacts themselves are clean: I ran haproxy -c against deploy/haproxy/haproxy.cfg inside haproxy:3.0-alpine (exit 0, "Configuration file is valid", no warnings) and resolved the compose with Compose v5.5.1, which merges the anchor onto both relays correctly. The packaging is not the problem. The problem is that two relays behind round-robin do not share a live broadcast domain, and that is not something the proxy can fix for us.

Blocker — round-robin splits live delivery

balance roundrobin over nostream-blue:8008 and nostream-green:8008 serves both at once, but nostream's live delivery is per-container:

  • src/utils/event.ts:176-188 — broadcastEvent returns early unless cluster.isWorker, then process.sends WebSocketServerAdapterEvent.Broadcast
  • src/adapters/web-socket-adapter.ts:116-124 — onBroadcast emits on the local adapter, then process.sends to the primary
  • src/app/app.ts:145-155 — the primary forwards it to its own workers only
  • src/factories/worker-factory.ts:69 — workers are the only processes that construct a WebSocketServerAdapter

Nothing crosses a container boundary. Redis is used exclusively for rate limiting (docs/REDIS.md), and there is no Postgres LISTEN/NOTIFY anywhere in src/. I re-checked that by hand rather than taking it from the review: the only Redis client in src/ is src/cache/client.ts, and it never publishes or subscribes.

So an event accepted by nostream-blue is delivered in real time only to the subscribers connected to blue. Under round robin roughly half of all subscribers never receive the events they subscribed to — mentions, DMs, threads — and it fails silently: the relay stays healthy, the event is in the database, and the live subscription just goes quiet. option redispatch / retry-on cannot compensate — they retry the accepting request, they do not fan out.

Two ways forward, either is fine by us:

  1. Add cross-instance fanout first. Redis pub/sub on the client that already exists — Redis is already fail-closed for rate limiting, so it is a dependency this relay tolerates today. Then two live backends are genuinely interchangeable.
  2. Or make the topology single-active. Keep one server serving and gate the switch explicitly (HAProxy runtime API set server … state maint/ready, or server green … backup if a cold standby is acceptable), and document that connections on the outgoing backend must reconnect at cutover.

Also worth fixing before merge

deploy/rolling-relay-recreate.sh:26-38 — a stopped relay is silently skipped, and the last healthy one can be stopped. docker compose ps -q <service> lists running containers only, so the guard at line 27 fires for any relay that is stopped or exited (verified against a synthetic labelled container: ps -q printed nothing for one in Exited (0), while ps -aq printed its id). Two consequences: a stopped relay is never replaced while the script still prints Rolling recreate complete; and because the script never checks that the peer is healthy before stopping a relay, when one is already down it skips that one and stops the only healthy backend, taking the stack unreachable for the duration. Suggest detecting with compose ps -aq, and before each compose stop requiring the peer running and its /readyz answering 200, exiting non-zero with a clear message rather than reaching the completion line.

deploy/README.md:172-176 — the rolling path never runs migrations, and the README omits the step. compose up -d --no-deps also implies not starting linked services, so nostream-migrate never runs in the rolling path, and the relay does not migrate itself — migrations are a separate one-shot service and db:migrate is not on the container's startup path. The README procedure says only "load the new image, then replace relays one at a time", so an operator following it on a release that adds a migration runs the new relay against the old schema. The script's own header treats migrations as a precondition, so the two documents disagree and the operator-facing one leaves the step out. Suggest adding it to the README (docker compose -f docker-compose.haproxy.yml up -d --force-recreate nostream-migrate) or having the script run it before the loop.

deploy/README.md:162-170 — the install steps leave the old relay holding port 8008. They copy the files and run up -d without stopping the single-relay stack, so on a host bootstrapped from this repo the nostream container from docker-compose.prod.yml is still holding 127.0.0.1:8008 and the new haproxy service fails with Bind for 127.0.0.1:8008 failed: port is already allocated. The section's wording acknowledges the migration; the commands do not perform it. Suggest stating that the two files are not meant to run simultaneously and stopping the old stack first.

deploy/haproxy/haproxy.cfg:8-16 — no option forwardfor, so per-IP controls collapse onto one address. HAProxy becomes the first hop for every client but never sets option forwardfor, so the relay reads the client address from the socket and sees the proxy container's IP for everyone. getRemoteAddress only honours a forwarded header when network.remoteIpHeader is set and the socket address is in network.trustedProxies (src/utils/http.ts:40-61), and the shipped defaults leave remoteIpHeader commented out (resources/default-settings.yaml:150-159). Every per-IP control therefore keys on a single address: the connection and message rate limiters, the admin rate limiter, and the invoice/admission ipWhitelist checks. This is not a regression — the existing single-relay stack behind a tunnel collapses the same way — but adding this proxy is where it should be fixed: option forwardfor, plus the matching network.remoteIpHeader: x-forwarded-for and the HAProxy container's address in network.trustedProxies, documented in the README.

deploy/docker-compose.haproxy.yml:27 — a configurable drain timeout can outlast the hardcoded grace period. WS_DRAIN_TIMEOUT_MS is operator-configurable (default 30s) while stop_grace_period: 45s is hardcoded, and nothing couples them. The README paragraph asserts they are consistent, but that holds only at the default: WS_DRAIN_TIMEOUT_MS=60000 — the case the variable exists for — makes Docker SIGKILL the container fifteen seconds into the drain, which is exactly the failure the same paragraph warns about, and the drain is what keeps existing WebSocket clients from being cut mid-flight. Suggest stop_grace_period: ${STOP_GRACE_PERIOD:-45s} with the README noting it must exceed the drain timeout, or stating the coupling where the variable is documented.


Reviewed by an automated read-only pass; the broadcast-path and Redis claims were re-verified by hand against main at 07524eb. Happy to re-review once the topology question is settled.

@coveralls

coveralls commented Sep 22, 2026 •

Copy link
Copy Markdown
Collaborator

Coverage Status

Coverage is 72.288% — feat/haproxy-blue-green into main. No base build found for main.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Comment thread deploy/README.md Outdated
Comment thread deploy/docker-compose.haproxy.yml Outdated
Comment thread deploy/env.example Outdated
Comment thread deploy/haproxy/haproxy.cfg
Comment thread src/app/app.ts Outdated
Comment thread deploy/README.md Outdated
Comment thread src/relay-broadcast/redis-relay-broadcast-fanout.ts Outdated
Comment thread src/relay-broadcast/redis-relay-broadcast-fanout.ts Outdated
Comment thread src/relay-broadcast/redis-relay-broadcast-fanout.ts

@chappie-daemon chappie-daemon left a comment

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.

Request changes — the HAProxy config does not parse, so the reference stack cannot start

Re-reviewed at 48ecf1d. First, the good news: the blocker from the previous review is genuinely closed, and by the route that was suggested. I read the implementation rather than the description:

  • RedisRelayBroadcastFanout is constructed in the primary App (src/app/app.ts:145, whose logger is app-primary), so there is one stream consumer per relay instance, not one per worker.
  • A plain XREAD from $ instead of a consumer group is the right call at this layer: a group would split deliveries between instances rather than fan them out, and $ means a freshly started instance never replays history to subscribers it does not yet have.
  • The origin skip (src/relay-broadcast/redis-relay-broadcast-fanout.ts:133) plus the deduplicator (src/app/app.ts:179,190) give a local subscriber exactly one copy: a publishing instance never re-delivers its own stream entry, and a peer entry is dropped when the local worker path already marked that event id.
  • XADD … MAXLEN ~ 50000 bounds the stream; entries are validated (isRelayBroadcastMessage) and a bad one is skipped rather than dispatched.
  • deploy/docker-compose.haproxy.yml:27 forces RELAY_BROADCAST_FANOUT: 'true', with :26 explaining why it must not inherit the bootstrap .env — so the shipped topology cannot come up with fan-out silently off. That was the one thing I wanted to confirm for myself.

Previous findings, each re-checked: finding 2 closed — require_healthy_peer exits 1 both when the peer is not running and when its /readyz is unhealthy (deploy/rolling-relay-recreate.sh:46-56); 3 closed — migrations are baked into the image (deploy/README.md:4,21-22,69,74); 4 closed — deploy/README.md:173 states that both stacks bind 127.0.0.1:8008 and the old one must be stopped; 5 closed on the proxy side (option forwardfor, haproxy.cfg:29) with the relay side documented (deploy/README.md:190-205) — see the nit below; 6 closed — stop_grace_period 45s against a 30s WS_DRAIN_TIMEOUT_MS. bcbb24c2 also fixes a real TDZ in the shutdown path that a fast worker exit could hit.

Blocker — deploy/haproxy/haproxy.cfg:17 is not a valid directive, on any HAProxy version

I ran haproxy -c against the file with the compose's own pinned image and then four more:

haproxy:3.0-alpine  exit=1  unknown keyword 'http-restrict-req-retry-on' in 'defaults' section
haproxy:3.1-alpine  exit=1  same
haproxy:3.2-alpine  exit=1  same
haproxy:3.3-alpine  exit=1  same
haproxy:latest      exit=1  same

followed by [ALERT] config : Fatal errors found in configuration. — HAProxy exits rather than starting, and docker-compose.haproxy.yml:58 pins haproxy:3.0-alpine, so docker compose up on this stack today produces a crash-looping proxy. This is not a version-pin problem: no HAProxy release accepts the keyword.

It arrived in 48ecf1d ("address HAProxy and Redis fan-out review feedback"). My previous review validated the revision before that commit and got Configuration file is valid, so this is a regression introduced while addressing review feedback. Line 16 (retry-on conn-failure empty-response response-timeout 502 503 504) already expresses the retry policy, so unless a specific directive was intended here, deleting line 17 is the fix.

Worth adding that check to the deploy README as a pre-flight step: a resolved compose proves the YAML is well-formed, not that the proxy accepts the config — docker compose config passing is how this got through a validation that otherwise looked complete.

Nit — the trustedProxies sample omits the proxy

deploy/README.md:190-205 correctly tells the operator to set network.remoteIpHeader and network.trustedProxies, and it matters: without it every per-IP control — the connection and message rate limiters, the admin limiter, ipWhitelist — keys on the proxy's address instead of the client's. But the sample list is 127.0.0.1, ::ffff:127.0.0.1, ::1, with the HAProxy container's compose-network address left as a commented-out docker inspect line. Copied as written, the socket address the relay sees is the HAProxy container, which is not in the list, and src/utils/http.ts:18 then ignores the forwarded header — the exact failure that step exists to prevent. Either make the inspect a required line, or give the compose network a static subnet and list that.

Notes (not blocking)

  • If Redis is unreachable, publish logs and returns while the read loop retries — fail-open, which is the right choice for a relay, but it quietly returns the fleet to the pre-PR behaviour (roughly half the subscribers missing live events) while /readyz stays green. Reflecting fan-out state in readiness would make that visible instead of log-only.
  • The MAXLEN ~ 50000 default retains whole event JSON, so it is the dominant Redis memory consumer on a busy relay; the default is worth stating next to the config table.
  • The deduplicator's 120s TTL is shorter than the stream's retention, so an instance lagging beyond two minutes could deliver a duplicate. Harmless for Nostr clients, which dedupe by id — noting it rather than asking for a change.

@chappie-daemon

Copy link
Copy Markdown
Collaborator

Correction to my review, plus four more a second pass found

The blocker stands, and holds up more strongly than I put it: deleting only line 17 makes the same file validate cleanly — exit 0, with just the expected unresolved-host NOTICEs — and http-restrict-req-retry-on appears zero times in the HAProxy 3.0 and 3.3 manuals. With restart: on-failure on the haproxy service that crash-loop means 127.0.0.1:8008 is never bound.

But I have to correct myself on the forwarded-header item, which I called closed on the proxy side with only a nit about the README's sample list. It is worse than that, and the README is what makes it reachable.

option forwardfor appends. HAProxy's own manual says the header "is always appended at the end of the existing header list, the server must be configured to always use the last occurrence of this header only … since it is really possible that the client has already brought one." The relay does the opposite: getRemoteAddress() ends with return (result as string).split(',')[0].trim() (src/utils/http.ts:61) — the first, client-supplied entry. trustedProxies does not save it, because it gates whether the header is read (the socket peer is the proxy, so that test passes), not which value wins.

So once an operator applies deploy/README.md:192-204, a client that sends its own X-Forwarded-For picks its own remoteAddress. That address keys the connection and message rate limiters, so rotating it hands every request a fresh bucket — no per-IP limiting at all — and if (ipWhitelist.includes(remoteAddress)) { return false } (src/handlers/request-handlers/rate-limiter-middleware.ts:41) lets a forged address matching a whitelisted one skip limiting outright. Both files are untouched by this PR, so the defect predates it; what is new is a README step that switches it on for this stack.

Fix: read the last entry (the proxy-appended one), or front HAProxy with a header the client cannot set, and require a docker-network trustedProxies entry in the README. Until then I would not tell operators to enable remoteIpHeader for this stack.

Four more, all in the new code

A second, independent pass over this revision — which also drove the real App wiring against a live Redis, and confirmed the fan-out itself works: an event on one instance reached the other's workers exactly once and in order, a 100-event burst arrived complete and ordered, no echo loop, and the read loop recovers from a Redis blip — found these:

  1. stop() can hang until SIGKILL during a Redis outage. It awaits readLoopPromise before disconnecting either client (src/relay-broadcast/redis-relay-broadcast-fanout.ts:84-92); the loop is parked on XREAD … BLOCK 5000, and with redis@4.5.1 a reconnecting client keeps isOpen === true and queues commands offline, so that await resolves only when Redis returns — while the disconnect() calls that would release it sit on the far side of the await. Measured: stop() still pending after 15s during an outage, versus 1ms with the two blocks swapped; even healthy, the ordering costs ~5s. App.close() only reaches process.exit(0) from the fan-out promise's finally (src/app/app.ts:289-294), so a stop attempted while Redis is down hangs in the primary until Docker SIGKILLs it at stop_grace_period (45s) — the moment an operator is most likely to be restarting relays. Move the two disconnect() calls above the await.
  2. publish()'s guard tests the wrong flag. if (!this.publisher?.isOpen) (line 55) never fires during an outage, for the same isOpen-while-reconnecting reason, so instead of skipping it queues every XADD offline: measured, three publishes during the outage settled only after recovery, none of them reaching the .catch at src/app/app.ts:181-183, and the peer then received all three stale broadcasts. That is an unbounded queue of full event payloads held in the primary, plus late replay. isReady is the flag the log message intends.
  3. A consumer that falls behind the trim loses the gap silently. No consumer group, no acks, bounded only by MAXLEN ~ 50000 — and Redis returns the surviving entries for an evicted id with no error. Measured: publish 1000 with the same trim options, XLEN now 100, and XREAD from an evicted id returned only the later entries, exit 0, ~897 dropped invisibly. Comparing XINFO STREAM's first-entry id against lastStreamId and warning would make that operator-visible.
  4. An enabled-but-failed fan-out is invisible. src/app/app.ts:152-155 logs the failure and clears the instance; /readyz reports only Postgres and Redis (database.ok && redis.ok), and HAProxy's health check is that same /readyz — so an instance that loses the start race stays in rotation delivering locally only, which is precisely the half-the-subscribers-miss condition the fan-out exists to remove. Treat it as degraded, or refuse to start when RELAY_BROADCAST_FANOUT is true and start() rejects.

These do not change the verdict — the config blocker alone does — but 1 and 2 concern the failure mode this stack is most likely to meet, and 4 is what would have told an operator that fan-out was off.

This branch has not been deployed

No deployments
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.

4 participants