-
Notifications
You must be signed in to change notification settings - Fork 511
fix(cli): exec start wrappers and bump postgres-meta so docker stop is not 10s (CLI-2192) #6203
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
e62aae7
8f03e4c
787493f
4d61ad8
2c0c790
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -51,8 +51,34 @@ const LEGACY_LOGFLARE_API_KEY = "api-key"; | |
| * running Logflare against an unmigrated database lets Oban die on the | ||
| * missing `public.oban_jobs` table instead. | ||
| */ | ||
| // A trap-and-escalate supervisor rather than a bare `exec`: measured with beam.smp as PID 1 | ||
| // (a plain `exec` chain), `docker stop -t 10` STILL burned the full grace period — Logflare's | ||
| // own SIGTERM shutdown hangs upstream, the same class of problem postgres-meta had before | ||
| // postgres-meta#1103. So `run.sh`'s shell stays PID 1 deliberately, with a real handler: | ||
| // forward SIGTERM to the BEAM, give its graceful shutdown a 3s window, then SIGKILL it. Today's | ||
| // post-timeout outcome is already SIGKILL, so escalating early produces the identical end state | ||
| // ~7s sooner (Logflare's durable state lives in Postgres/BigQuery, both crash-safe) — and if | ||
| // upstream ever fixes its handler, the graceful window wins first and the KILL never fires. | ||
| // The `migrate`-then-`start` sequencing (a failed migrate exits the container so the | ||
| // `unless-stopped` policy retries) is unchanged and deliberate — see the doc comment above | ||
| // (#6088). Stop timing is outside the Go-parity surface (ADR 0016). | ||
| // `run.sh` line by line: migrate (exit on failure — the container-restart retry above), start | ||
| // the BEAM in the background, install the TERM handler, then `wait`. A trapped signal interrupts | ||
| // `wait` with a >128 status, so the follow-up `wait` collects the BEAM's real exit status once | ||
| // the trap's TERM-then-KILL escalation finishes; the `127` guard keeps the first status when the | ||
| // BEAM was already reaped (a second `wait` on a reaped pid is an error). A clean BEAM exit takes | ||
| // the single-`wait` path untouched. | ||
| const LEGACY_LOGFLARE_ENTRYPOINT_SCRIPT = | ||
| "cat <<'EOF' > run.sh && sh run.sh\n./logflare eval Logflare.Release.migrate &&\n./logflare start --sname logflare\nEOF\n"; | ||
| "cat <<'EOF' > run.sh && exec sh run.sh\n" + | ||
| "./logflare eval Logflare.Release.migrate || exit $?\n" + | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When Useful? React with 👍 / 👎. |
||
| "./logflare start --sname logflare &\n" + | ||
| "BEAM_PID=$!\n" + | ||
| 'trap \'kill -TERM "$BEAM_PID" 2>/dev/null; n=0; while [ "$n" -lt 3 ] && kill -0 "$BEAM_PID" 2>/dev/null; do n=$((n+1)); sleep 1; done; kill -KILL "$BEAM_PID" 2>/dev/null\' TERM\n' + | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When analytics is enabled, this supervisor's correctness depends on real PID-1 signal delivery and the Logflare image's shutdown behavior, but the added unit test only compares the generated shell string. I checked the existing AGENTS.md reference: apps/cli/AGENTS.md:L520-L523 Useful? React with 👍 / 👎. |
||
| 'wait "$BEAM_PID"\n' + | ||
| "code=$?\n" + | ||
| 'if [ "$code" -gt 128 ]; then wait "$BEAM_PID" 2>/dev/null; code2=$?; [ "$code2" -ne 127 ] && code=$code2; fi\n' + | ||
| 'exit "$code"\n' + | ||
| "EOF\n"; | ||
|
|
||
| export interface LegacyLogflareContainerSpecInput { | ||
| /** | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -290,7 +290,11 @@ export function legacyBuildVectorEntrypointScript(vectorYaml: string, logflareId | |
| vectorYaml + | ||
| "\nEOF\nuntil wget --no-verbose --tries=1 --spider http://" + | ||
| logflareId + | ||
| ":4000/health 2>/dev/null; do sleep 2; done\nvector --config /etc/vector/vector.yaml\n" | ||
| // `exec` so Vector (not `sh`) is PID 1 and `docker stop`'s SIGTERM reaches it directly — | ||
| // without it every stop burns the full 10s grace period. The wget wait-loop above still runs | ||
| // in the wrapper shell BEFORE the exec, unchanged. Stop timing is outside the Go-parity | ||
| // surface (ADR 0016). | ||
| ":4000/health 2>/dev/null; do sleep 2; done\nexec vector --config /etc/vector/vector.yaml\n" | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When Vector restarts while Logflare is unhealthy, this loop can run indefinitely and execution never reaches the new AGENTS.md reference: apps/cli/AGENTS.md:L520-L523 Useful? React with 👍 / 👎. |
||
| ); | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -131,7 +131,7 @@ export const SERVICE_CATALOG = { | |
| postgrest: { | ||
| name: "postgrest", | ||
| configKey: "postgrest", | ||
| defaultVersion: "14.16", | ||
| defaultVersion: "16.1", | ||
| runtimeSupport: "native-preferred", | ||
| artifact: { | ||
| docker: { ownership: "supabase", repository: "postgrest", tagPrefix: "v" }, | ||
|
|
@@ -201,7 +201,7 @@ export const SERVICE_CATALOG = { | |
| realtime: { | ||
| name: "realtime", | ||
| configKey: "realtime", | ||
| defaultVersion: "2.123.1", | ||
| defaultVersion: "2.124.4", | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a consumer relies on Useful? React with 👍 / 👎. |
||
| runtimeSupport: "docker-only", | ||
| artifact: { docker: { ownership: "supabase", repository: "realtime", tagPrefix: "v" } }, | ||
| activation: { startup: "eager", activates: [], owns: [] }, | ||
|
|
@@ -210,7 +210,7 @@ export const SERVICE_CATALOG = { | |
| storage: { | ||
| name: "storage", | ||
| configKey: "storage", | ||
| defaultVersion: "1.68.1", | ||
| defaultVersion: "1.69.0", | ||
| runtimeSupport: "docker-only", | ||
| artifact: { docker: { ownership: "supabase", repository: "storage-api", tagPrefix: "v" } }, | ||
| activation: { startup: "lazy", activates: ["imgproxy"], owns: ["imgproxy"] }, | ||
|
|
@@ -237,7 +237,7 @@ export const SERVICE_CATALOG = { | |
| pgmeta: { | ||
| name: "pgmeta", | ||
| configKey: "pgmeta", | ||
| defaultVersion: "0.96.6", | ||
| defaultVersion: "0.98.0", | ||
| runtimeSupport: "docker-only", | ||
| artifact: { | ||
| docker: { ownership: "supabase", repository: "postgres-meta", tagPrefix: "v" }, | ||
|
|
@@ -248,7 +248,7 @@ export const SERVICE_CATALOG = { | |
| studio: { | ||
| name: "studio", | ||
| configKey: "studio", | ||
| defaultVersion: "2026.08.03-sha-022b374", | ||
| defaultVersion: "2026.08.10-sha-5b68af1", | ||
| runtimeSupport: "docker-only", | ||
| artifact: { docker: { ownership: "supabase", repository: "studio" } }, | ||
| activation: { startup: "eager", activates: ["analytics"], owns: [] }, | ||
|
|
@@ -257,7 +257,7 @@ export const SERVICE_CATALOG = { | |
| analytics: { | ||
| name: "analytics", | ||
| configKey: "analytics", | ||
| defaultVersion: "1.49.2", | ||
| defaultVersion: "1.50.2", | ||
| runtimeSupport: "docker-only", | ||
| artifact: { docker: { ownership: "supabase", repository: "logflare" } }, | ||
| activation: { startup: "lazy", activates: ["vector"], owns: ["vector"] }, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This intentionally changes legacy Docker behavior relative to the Go reference for Kong, Vector, Logflare, Edge Runtime, and
functions serve, butdocs/go-cli-divergences.mdcontains no entry for the new PID-1/signal-handling behavior. Add one behavioral-divergence entry covering these wrappers so support and future parity work do not mistake the changed shutdown semantics for accidental drift.AGENTS.md reference: apps/cli/AGENTS.md:L540-L544
Useful? React with 👍 / 👎.