feat(resilience): survive reboots, OOMs, IP changes + observability - #71
Conversation
After the 2026-07-24 prod OOM incident (box OOM'd, rebooted, wiped authorized_keys, forced snapshot→rebuild with DNS change, ~1h downtime), this adds three failure mode preventions: 1. SSH key survives any reboot/rebuild (persist-ssh-key.sh): - authorized_keys with append-if-absent dedup - UpCloud stored SSH key via upctl - cloud-init snippet with ssh_deletekeys: false 2. OOM containment (harden-memory.sh + service units): - Postgres protected: OOMScoreAdjust=-900, MemoryHigh=4G, MemoryMax=6G - Every worker/service has MemoryHigh (soft reclaim) + MemoryMax (hard kill) - Deploy build capped at 6G via systemd-run --scope - Swap grown to 8GB, vm.swappiness=10, overcommit_memory=2 - All operational services (backup, index-snapshot, audit-rotate) capped 3. Floating IP for zero-DNS box swaps (attach-floating-ip.sh): - Create/attach UpCloud floating IP in us-sjo1 - DNS points at float once; box swaps reattach float (no DNS edit) 4. One-command rescue runbook (prod-rescue-runbook.md + rescue.sh): - Exact 2026-07-24 sequence as markdown checklist - Automated rescue script with --plan-only and interactive modes - State tracking, step-by-step confirmations, classifier-gated display No app code changes. All scripts idempotent, shellcheck-clean.
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
Next review available in: 38 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThis PR adds snapshot-based production rescue automation, floating IP and SSH-key recovery helpers, memory containment, scheduled monitoring and backups, deployment smoke checks, an updated health endpoint, and operational runbooks. ChangesProduction rescue and resilience
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Operator
participant rescue.sh
participant UpCloud
participant ReplacementServer
participant Monitoring
Operator->>rescue.sh: choose rescue mode
rescue.sh->>UpCloud: select snapshot and create replacement
rescue.sh->>ReplacementServer: verify SSH and listings
rescue.sh->>ReplacementServer: deploy and harden services
rescue.sh->>UpCloud: attach floating IP
Monitoring->>ReplacementServer: run health and smoke checks
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (3)
ops/resilience/rescue.sh (2)
278-279: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winPrefer
accept-newoverStrictHostKeyChecking=no.Disabling host-key checking entirely removes MITM protection for the whole session, not just the first connect.
accept-newstill trusts an unseen host key on first contact (which this fresh-server-provisioning use case needs) but continues to reject a key that later changes unexpectedly.🔧 Proposed fix
- if ssh -o ConnectTimeout=5 -o StrictHostKeyChecking=no root@"$NEW_IP" 'echo ok' &>/dev/null; then + if ssh -o ConnectTimeout=5 -o StrictHostKeyChecking=accept-new root@"$NEW_IP" 'echo ok' &>/dev/null; then🤖 Prompt for 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. In `@ops/resilience/rescue.sh` around lines 278 - 279, Update the SSH options in the retry loop around the connection check to replace StrictHostKeyChecking=no with StrictHostKeyChecking=accept-new, preserving first-contact acceptance while rejecting unexpected host-key changes.Source: Linters/SAST tools
111-117: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAvoid re-parsing an interpolated string via
bash -c.
run_cmdfeeds its argument straight intobash -c "$1", and the only call site (run_cmd "upctl server stop $DEAD_UUID", line 212) builds that string from a CLI-supplied value. Low practical risk here sinceDEAD_UUIDis operator-supplied on this same invocation, but it's a fragile pattern flagged by static analysis (CWE-78) — passing the command as argv avoids any re-parsing hazard if this helper is reused with less trusted input later.🔧 Proposed fix
run_cmd() { if [[ "$MODE" == "plan-only" ]]; then - printf " \033[1;37m$\033[0m %s\n" "$1" + printf " \033[1;37m$\033[0m %s\n" "$*" return 0 fi - bash -c "$1" + "$@" }And update the call site:
run_cmd upctl server stop "$DEAD_UUID".Also applies to: 212-212
🤖 Prompt for 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. In `@ops/resilience/rescue.sh` around lines 111 - 117, Update run_cmd to accept a command and its arguments as separate parameters, preserving the plan-only display while executing without bash -c re-parsing. Change the call site invoking upctl server stop to pass upctl, server, stop, and "$DEAD_UUID" as distinct arguments.Source: Linters/SAST tools
ops/resilience/harden-memory.sh (1)
145-146: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winScraper's MemoryHigh/MemoryMax are asserted, not verified.
Unlike every other service,
oper-scraper.serviceis skipped with a hardcoded message instead of actually checking the file. If its settings ever drift, this script won't notice or fix it.♻️ Proposed fix
-# Scraper — already has both MemoryHigh + MemoryMax, skip -skip "oper-scraper.service already has MemoryHigh=1536M + MemoryMax=2G" +# Scraper — verify (idempotent, self-corrects on drift) +add_memory_high "$SERVICE_DIR/oper-scraper.service" "1536M" "2G"🤖 Prompt for 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. In `@ops/resilience/harden-memory.sh` around lines 145 - 146, Replace the unconditional scraper skip near the existing “oper-scraper.service” message with the same file-based MemoryHigh and MemoryMax validation and remediation used for the other services. Ensure oper-scraper.service is only skipped when both configured values match the expected limits; otherwise report and fix any drift.
🤖 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 `@ops/resilience/attach-floating-ip.sh`:
- Line 20: Remove the hardcoded fallback UUID from the PROD_SERVER_UUID
initialization in attach-floating-ip.sh. Require either the positional argument
or existing PROD_SERVER_UUID environment value, and fail loudly with a clear
error before any attachment logic when neither is provided; preserve explicit
arguments and environment overrides.
In `@ops/resilience/harden-memory.sh`:
- Around line 57-72: Fix the idempotency comparisons in the Postgres
configuration block and the SYSCTL_CONF block by replacing command-substitution
string equality checks with byte-oriented cmp -s comparisons against the
expected content supplied on stdin. Preserve the existing skip, backup, write,
and status-message behavior, using the same diff-based pattern as deploy_unit().
In `@ops/resilience/rescue.sh`:
- Around line 78-80: Populate FLOAT_IP from the Step 7 floating-IP attachment
command’s stdout, preserving the actual address it prints, then include it in
the final summary as the floating IP. Update the affected rescue flow and
summary block so FLOAT_IP is both assigned and surfaced, eliminating the
unused-variable warning.
- Line 204: Replace the CLASSIFIER-GATED warning output at the visible echo
statement and its corresponding occurrences with printf-based formatting,
matching the script’s other colored output so \033 escape sequences render as
terminal colors. Update all three occurrences, including the warnings around the
related classifier-gated messages.
- Around line 63-70: Update the rescue script’s state persistence around
STATE_DIR, STATE_FILE, and the EXIT trap: use a stable, uniquely identified
state path that is not based on $$, preserve the progress file across
interrupted or intentionally quit runs, and remove it only after the rescue
completes successfully. Ensure reruns can read prior step markers while avoiding
predictable world-writable temporary paths.
- Around line 224-260: Replace unsupported ssh-key and floating-IP command usage
with the upctl ip-address workflow, and stop treating onepercent-deploy-202606
as an account-level SSH key name. Update rescue.sh lines 224-260,
attach-floating-ip.sh lines 47-114, persist-ssh-key.sh lines 125-181, and
documentation/operations/prod-rescue-runbook.md lines 50-78: pass --ssh-keys a
valid file path or raw public-key content, use ip-address list/assign
--floating/modify --mac as appropriate, and wait for or reference the actual
persisted key material rather than a nonexistent named key.
In `@ops/systemd/deploy-systemd.sh`:
- Around line 69-97: Add fail-fast error handling to the inner bash -c script in
build_node by enabling errexit and pipefail before loading .env and running pnpm
install or pnpm build. Ensure any failed build command stops the block and
propagates a nonzero status to systemd-run, while preserving the existing
asset-copying flow for successful builds.
---
Nitpick comments:
In `@ops/resilience/harden-memory.sh`:
- Around line 145-146: Replace the unconditional scraper skip near the existing
“oper-scraper.service” message with the same file-based MemoryHigh and MemoryMax
validation and remediation used for the other services. Ensure
oper-scraper.service is only skipped when both configured values match the
expected limits; otherwise report and fix any drift.
In `@ops/resilience/rescue.sh`:
- Around line 278-279: Update the SSH options in the retry loop around the
connection check to replace StrictHostKeyChecking=no with
StrictHostKeyChecking=accept-new, preserving first-contact acceptance while
rejecting unexpected host-key changes.
- Around line 111-117: Update run_cmd to accept a command and its arguments as
separate parameters, preserving the plan-only display while executing without
bash -c re-parsing. Change the call site invoking upctl server stop to pass
upctl, server, stop, and "$DEAD_UUID" as distinct arguments.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 6c3624e8-a242-4251-8c40-a8c374d71aae
📒 Files selected for processing (25)
documentation/operations/prod-rescue-runbook.mdops/resilience/attach-floating-ip.shops/resilience/harden-memory.shops/resilience/persist-ssh-key.shops/resilience/rescue.shops/systemd/deploy-systemd.shops/systemd/oper-app.serviceops/systemd/oper-audit-rotate.serviceops/systemd/oper-backup-verify.serviceops/systemd/oper-backup.serviceops/systemd/oper-index-snapshot.serviceops/systemd/oper-ml.serviceops/systemd/oper-n8n.serviceops/systemd/oper-pg-tileserv.serviceops/systemd/oper-postgres.serviceops/systemd/oper-redis.serviceops/systemd/oper-two.serviceops/systemd/oper-worker-alerts.serviceops/systemd/oper-worker-digest.serviceops/systemd/oper-worker-media.serviceops/systemd/oper-worker-ml-scheduler.serviceops/systemd/oper-worker-refresh.serviceops/systemd/oper-worker-rent.serviceops/systemd/oper-worker-watchlist.serviceops/systemd/oper-worker.service
- attach-floating-ip.sh: remove hardcoded UUID fallback, require explicit arg/env - attach-floating-ip.sh: replace upctl floating-ip with upctl ip-address commands - harden-memory.sh: fix idempotency checks with cmp -s instead of string equality - harden-memory.sh: replace scraper skip with add_memory_high (self-corrects drift) - rescue.sh: use stable state path (DEAD_UUID-based), preserve across runs - rescue.sh: replace bash-c with direct arg execution in run_cmd - rescue.sh: capture FLOAT_IP from Step 7, show in summary - rescue.sh: replace echo with printf for escape sequences - rescue.sh: StrictHostKeyChecking=no → accept-new - deploy-systemd.sh: add set -euo pipefail inside systemd-run bash -c block - persist-ssh-key.sh: replace upctl ssh-key calls with Control Panel guidance - runbook: --ssh-keys takes file path, not named key
jonquixote
changed the title
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 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 `@apps/one/src/app/api/health/route.ts`:
- Around line 12-23: Update the health-check flow around pool.connect and
client.query so the 2-second timeout covers client acquisition as well as query
execution. Configure a server-side statement_timeout for the health query, and
when acquisition or execution times out or fails, ensure any potentially
in-flight client is destroyed via client.release(true) or equivalent handling
rather than returned to the pool; release normally only after a successful,
completed query.
In `@ops/monitoring/healthcheck.sh`:
- Around line 18-20: Implement a separate expiry-aware maintenance-silence
state: in ops/monitoring/healthcheck.sh lines 18-20, check the dedicated silence
marker before either notification path; in ops/monitoring/notify-telegram.sh
lines 9-10, keep /var/lib/oper-alerts exclusively for alert-delivery state; in
lines 55-61, keep resolution tracking separate from maintenance silencing; and
in documentation/operations/monitoring.md lines 37-50, document the silence
directory and expiry behavior.
- Around line 60-67: Update the service discovery command in the loop over
oper-* units to include inactive and stopped units by passing the appropriate
all-units option to systemctl list-units. Preserve the existing filtering, key
generation, and check_pass/check_fail behavior so discovered inactive units are
reported as failures.
In `@ops/monitoring/snapshot-cron.sh`:
- Around line 33-51: The snapshot pruning flow around BACKUPS and upctl storage
delete must propagate listing, date-evaluation, and deletion failures instead of
defaulting to an empty list or ignoring errors. Track prune success explicitly,
fail when retention cannot be evaluated or applied, and emit an alert using a
distinct prune-specific key while preserving normal pruning behavior.
- Around line 15-17: Select the boot storage device in the BOOT_DISK lookup
using an explicit boot-device indicator from the server storage/configuration
response, or use a configured boot-disk UUID; remove the created_at sorting and
last-device fallback so later-created data disks cannot be selected.
In `@ops/resilience/attach-floating-ip.sh`:
- Around line 98-106: Update the SERVER_MAC lookup to select the server’s public
network interface instead of `.type == "private"`, while preserving the existing
empty-MAC validation and error handling. Use the public interface MAC as the
value passed to the floating IP attachment command.
In `@ops/resilience/rescue.sh`:
- Around line 66-70: Validate DEAD_UUID against the expected UUID format before
constructing STATE_DIR, rejecting invalid or path-like overrides. Replace the
predictable /tmp-based state location with a private persistent state root, and
ensure the directory is created and permissioned securely before STATE_FILE is
touched.
- Line 208: Update both occurrences that print the upctl stop command in the
rescue script to pass DEAD_UUID as a printf argument using a fixed format
string, preventing percent characters in the override value from being
interpreted as directives.
In `@ops/systemd/deploy-systemd.sh`:
- Line 147: Update the sitemap_body pipeline in the deployment script to consume
curl’s complete response while retaining only the first five lines, avoiding
head and its early pipe closure. Preserve the existing timeout, silent failure
handling, and empty-string fallback behavior.
- Around line 118-129: Make the notifier invocations in the smoke_test failure
and pass handlers non-fatal under the outer set -e. Update both calls to the
notify_script in the failed and pass functions to catch notification/network
errors, emit a warning, and continue; preserve failed=1 only for failed smoke
assertions so notifier outages never block deployment.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 04f65403-b94b-4a2e-8419-37a2bf70f5b3
📒 Files selected for processing (16)
apps/one/src/app/api/health/route.tsdocumentation/operations/monitoring.mddocumentation/operations/prod-rescue-runbook.mddocumentation/operations/wave-7-open-items.mdops/monitoring/healthcheck.shops/monitoring/notify-telegram.shops/monitoring/snapshot-cron.shops/resilience/attach-floating-ip.shops/resilience/harden-memory.shops/resilience/persist-ssh-key.shops/resilience/rescue.shops/systemd/deploy-systemd.shops/systemd/oper-healthcheck.serviceops/systemd/oper-healthcheck.timerops/systemd/oper-snapshot.serviceops/systemd/oper-snapshot.timer
🚧 Files skipped from review as they are similar to previous changes (2)
- documentation/operations/prod-rescue-runbook.md
- ops/resilience/harden-memory.sh
| const client = await pool.connect(); | ||
| try { | ||
| const client = await pool.connect(); | ||
| await client.query('SELECT 1'); | ||
| client.release(); | ||
| healthStatus.services.database = 'healthy'; | ||
| } catch (error) { | ||
| console.error('Health Check - Database Error:', error); | ||
| healthStatus.services.database = 'unhealthy'; | ||
| healthStatus.status = 'degraded'; | ||
| await Promise.race([ | ||
| client.query('SELECT 1'), | ||
| new Promise<never>((_, reject) => | ||
| setTimeout(() => reject(new Error('DB health check timeout')), 2000) | ||
| ), | ||
| ]); | ||
| db = 'up'; | ||
| } finally { | ||
| client.release(); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
fd -t f 'db.ts' apps/one | while read -r file; do
ast-grep outline "$file" --items all
rg -n -C 3 'new Pool|connectionTimeoutMillis|query_timeout|statement_timeout|max' "$file"
done
rg -n -C 3 'pool\.connect\(|client\.release\(|pool\.query\(' apps/one/srcRepository: jonquixote/OnePercentRealEstate
Length of output: 50387
🌐 Web query:
node-postgres Pool connect release client statement_timeout release after query timeout
💡 Result:
In node-postgres, the pool does not automatically release a client back to the pool if a statement_timeout occurs [1]. When a statement_timeout is triggered, the PostgreSQL server cancels the specific query, and the node-postgres client receives an error [1][2]. However, the client remains checked out from the pool until your application explicitly calls client.release [3][1]. It is critical to handle this error in your application logic to ensure the connection is returned to the pool or destroyed properly [3][1]. If you fail to release the client, the connection will leak, eventually exhausting the pool and causing subsequent pool.connect calls to hang or timeout [3]. Recommended pattern: Always use a try-finally block when manually acquiring a client from the pool to guarantee the release [1]: const client = await pool.connect; try { await client.query('SET statement_timeout = 5000'); await client.query('YOUR_LONG_QUERY'); } catch (err) { // If a statement_timeout (or any other error) occurs, // you may choose to destroy the client if the connection // state is considered corrupted. // Pass 'true' to release to destroy the connection. client.release(true); throw err; } finally { // Otherwise, release back to the pool for reuse. // If already released in catch, this is generally safe, // but verify your specific implementation. client.release; } Key distinctions: 1. statement_timeout: Configured on the Postgres server side; the query is aborted, but the connection remains open and checked out [1][2]. 2. query_timeout: A client-side setting that stops waiting for a response, but it also does not automatically release the client back to the pool [4][5]. 3. Connection Integrity: If a query fails due to a timeout, the connection might be left in an unstable state (e.g., interrupted transaction). It is often safer to call client.release(true) to destroy the connection and let the pool create a fresh one [3][1].
Citations:
- 1: Alter statement_timeout on existing pool. brianc/node-postgres#2016
- 2: pg-cursor timeouts brianc/node-postgres#2183
- 3: https://node-postgres.com/apis/pool
- 4: https://github.com/brianc/node-postgres/blob/master/docs/pages/apis/client.mdx
- 5: Add configurable query timeout brianc/node-postgres#1760
🌐 Web query:
node-postgres 47889 async query cancel promise does not reject timeout release true
💡 Result:
The specific number "47889" does not appear to correspond to a documented issue or known bug in node-postgres related to asynchronous query cancellation, promise rejection, or the release of pool clients [1][2][3][4][5]. In the context of the broader software ecosystem, "47889" identifies several unrelated issues in other projects, such as a URL-parsing crash in Node.js core [6][7], a foreign key recursion bug in Ruby on Rails [8], and a GeoServer issue [9]. Regarding node-postgres behavior: 1. Cancellation: node-postgres has introduced support for AbortSignal to handle query cancellation [1]. When using this, if a query is canceled, the behavior regarding promise rejection and connection state depends on the implementation details of the request [1]. 2. Releasing Clients: The client.release(destroy?: boolean) method is used to return a client to the pool [2]. Passing true as the argument instructs the pool to disconnect and destroy the client instance rather than returning it to the pool [2]. This is commonly used when a client is suspected to be in an inconsistent or unusable state [4]. 3. Query Timeouts: You can configure query_timeout in the client configuration to automatically time out queries after a specified duration [5]. If you are experiencing a situation where a promise does not reject upon cancellation, it is recommended to verify that you are using the latest version of node-postgres and to ensure that your cancellation logic (e.g., using AbortSignal) is correctly integrated with your query execution [1]. If a connection remains in an unstable state, explicitly calling client.release(true) can ensure the connection is destroyed [2][4].
Citations:
- 1: Cancellable Query with AbortSignal option brianc/node-postgres#3212
- 2: https://node-postgres.com/apis/pool
- 3: Feature: query timeout brianc/node-postgres#1713
- 4: Explicit Resource Management for pg pool client brianc/node-postgres#3661
- 5: https://github.com/brianc/node-postgres/blob/master/docs/pages/apis/client.mdx
- 6: libc++abi: terminating due to uncaught exception of type std::out_of_range: basic_string Abort trap: 6 nodejs/node#47889
- 7: deps: update ada to 2.3.1 nodejs/node#47893
- 8: rails/rails@e11ebc0
- 9: https://discourse.osgeo.org/t/geoserver-users-adding-postgis-layers-with-no-attributes-except-id-field-gives-no-native-srs/47889
🌐 Web query:
site:node-postgres.com APIs pool connect statement_timeout client.release true
💡 Result:
In the node-postgres (pg) library, these concepts relate to connection management and error handling as follows: 1. statement_timeout: This is a configuration option available on both Pool and Client instances [1]. It defines the number of milliseconds a statement is allowed to run before it is automatically cancelled by the PostgreSQL server [1]. 2. client.release: When you acquire a client from a pool using pool.connect, you are responsible for returning it to the pool [2][3]. The client.release method is used to return that client to the pool [2]. 3. client.release(true): If you pass true (or any truthy value) to the release method, you are instructing the pool to destroy that client instance instead of returning it to the pool [2]. This is useful if the client has encountered a fatal error or a state that makes it unsuitable for reuse [2]. It is important to always release a client—either normally or by destroying it—when you are finished with it [2][3]. Failing to do so causes a connection leak, which will eventually exhaust the pool's available connections and cause your application to hang or timeout when trying to check out a new client [2][3]. If you do not require a transaction, using pool.query is the preferred approach, as it manages the checkout and release lifecycle automatically, eliminating the risk of leaking a client [3].
Citations:
- 1: https://node-postgres.com/apis/client
- 2: https://node-postgres.com/apis/pool
- 3: https://node-postgres.com/features/pooling
🌐 Web query:
node postgres query_timeout statement_timeout reject promise node-postgres GitHub issue
💡 Result:
In node-postgres, statement_timeout and query_timeout serve distinct purposes and are implemented at different levels of the stack [1]. statement_timeout This is a server-side setting passed to PostgreSQL [1]. When a statement takes longer than this value, the database engine itself cancels the query and returns an error to the client [1]. It is useful for preventing long-running queries from consuming database resources [2]. query_timeout This is a client-side setting handled by the node-postgres driver [1]. It defines how long the driver will wait for a response from the database [1]. If the database does not return a response within this window, the client stops waiting and throws a timeout error [1]. Crucially, the database may continue executing the query even after the client-side query_timeout has been triggered [1]. Key Considerations: 1. Recommended Usage: It is generally recommended to use both in conjunction, setting the query_timeout slightly higher than the statement_timeout [1]. This ensures that the application will throw a timeout error if the database fails to respond (e.g., due to network issues or backend hanging), while allowing the database to enforce the official timeout if the query simply takes too long [1]. 2. Promise Rejection: When either timeout is triggered, the promise returned by the client.query method will reject [1][3]. Developers should handle these rejections to manage connection state and avoid unhandled promise rejections [4]. 3. Native Driver Note: If using the native driver (pg-native), be aware that some timeout configurations may not be applied automatically and might require explicit handling or workarounds via connection options [5][6]. 4. Connection Stalling: There have been historical reports of queries hanging or promises not settling in specific edge cases (such as when a connection is dropped abruptly) [7][8]. Ensuring the driver is up-to-date and using appropriate timeout settings is the standard approach to mitigating these issues [4].
Citations:
- 1: https://stackoverflow.com/questions/59155572/how-to-set-query-timeout-in-relation-to-statement-timeout
- 2: pg-cursor timeouts brianc/node-postgres#2183
- 3: Client side socket timeout (not statement_timeout) brianc/node-postgres#3124
- 4: https://stackoverflow.com/questions/68930406/node-postgres-gives-error-timeout-expired
- 5: native driver ignores statement_timeout brianc/node-postgres#2103
- 6: Support idle_in_transaction_session_timeout and statement_timeout for native driver brianc/node-postgres#2323
- 7: Client#query promise stuck when connection closed brianc/node-postgres#1500
- 8: ClientRead statement_timeout brianc/node-postgres#1952
🌐 Web query:
node-postgres issue query_timeout promise does not reject connection remains acquired
💡 Result:
The query_timeout setting in node-postgres is a client-side timeout that stops the driver from waiting for a response from the database, but it does not inherently guarantee that the underlying TCP connection is properly cleaned up, released back to the pool, or that the backend query is canceled [1][2][3]. Users have reported several behaviors that contribute to the appearance of connections remaining acquired or "stuck" when a query_timeout occurs: 1. Client-Side Timeout Limitation: The query_timeout setting only triggers a timeout on the client side [1][3]. It does not send a cancellation request (such as pg_cancel_backend) to the PostgreSQL server [2][3]. If the connection remains valid but the query is taking too long, the backend may continue to process the request, leaving the connection in an inconsistent state [1]. 2. Connection State Issues: When a connection is interrupted (e.g., due to network failure) and a query_timeout is triggered, the connection may not be correctly marked as "dead" or "invalid" [1][4]. Consequently, when the connection is returned to the pool, subsequent attempts to use it can fail or hang because the internal state (e.g., readyForQuery) is desynchronized [1][4]. 3. Interaction with Streams: When using pg-query-stream alongside query_timeout, the library has historically struggled to release the timeout timer properly because the query callback structure differs from standard queries [5][6]. This can result in the process hanging until the timeout expires or throwing unexpected errors (e.g., TypeError: queryCallback is not a function) [5][6]. Recommended Mitigations: - Use statement_timeout: Configure the PostgreSQL statement_timeout at the database or session level [3]. This is generally preferred because it forces the database to terminate the query, which reliably frees up the backend resources [3]. - Configure Keep-Alive: Setting a reasonable keepAliveIdleMillis can help the driver detect and prune dead TCP connections more effectively, preventing them from lingering in the pool [1]. - Pool Configuration: Ensure connectionTimeoutMillis and idleTimeoutMillis are configured correctly to manage connection lifecycle within the pool [1]. - Monitor Backend Processes: In environments where long-running queries are common, implement external monitoring to identify and terminate orphaned backend processes directly in PostgreSQL [2]. These issues have been the subject of ongoing discussion in the node-postgres repository [7][1][5][4][2][6]. As of July 2026, relying solely on query_timeout for connection management is often insufficient in unstable network conditions [1].
Citations:
- 1: PG queries are not timing out on server going down brianc/node-postgres#3399
- 2: Terminate backend when query_timeout is reached brianc/node-postgres#1954
- 3: Add configurable query timeout brianc/node-postgres#1760
- 4: Client#query promise stuck when connection closed brianc/node-postgres#1500
- 5: Using pg-query-stream with query_timeout set, timeout timer doesn't get released brianc/node-postgres#3219
- 6: Query streams break with
query_timeoutbrianc/node-postgres#1860 - 7: Feature: query timeout brianc/node-postgres#1713
🌐 Web query:
site:github.com/brianc/node-postgres promise does not reject query_timeout client.release true
💡 Result:
In the node-postgres (pg) library, the query_timeout configuration is designed to trigger a client-side timeout if a query does not receive a response from the PostgreSQL server within the specified time [1][2][3]. When this timeout occurs, the returned promise should reject with a "Query read timeout" error [4][5]. However, there are documented scenarios and edge cases where this mechanism may fail to function as expected, particularly regarding client release and pool management: 1. Streaming Issues: When using pg-query-stream with query_timeout, the timeout timer may not be properly released if the underlying query callback is not invoked, leading to potential process hangs or unexpected behavior [5]. Some users have reported that certain Query objects may lack the expected callback property, which can cause errors when the timeout logic attempts to execute it [6]. 2. Connection State Stalling: If a network connection is lost while a query is awaiting a response, the client might remain stuck. Although the query_timeout can trigger an error and release the client back to the pool, the underlying socket may stay unresponsive [4]. Subsequent queries executed on the same "reused" client may hang indefinitely because the client is still effectively waiting for the previous, timed-out operation to complete [4]. 3. Resource Leaking: Regardless of whether a query times out or succeeds, it is critical to call client.release to return the client to the pool [7]. If an error (including a timeout) occurs and the client is not released in a finally block, the client will be leaked, leading to pool exhaustion [7][8]. To mitigate these issues, developers often: - Use try...finally blocks to ensure client.release is always called, even when errors occur [7][8]. - Combine query_timeout with statement_timeout on the database side to ensure that long-running queries are also cancelled by the server [9]. - Implement robust error handling to identify and potentially discard "stuck" clients if persistent issues occur [4].
Citations:
- 1: Feature: query timeout brianc/node-postgres#1713
- 2: https://github.com/brianc/node-postgres/blob/master/docs/pages/apis/client.mdx
- 3: Client side socket timeout (not statement_timeout) brianc/node-postgres#3124
- 4: PG queries are not timing out on server going down brianc/node-postgres#3399
- 5: Using pg-query-stream with query_timeout set, timeout timer doesn't get released brianc/node-postgres#3219
- 6: Query streams break with
query_timeoutbrianc/node-postgres#1860 - 7: https://github.com/brianc/node-postgres/blob/master/docs/pages/features/pooling.mdx
- 8: https://github.com/brianc/node-postgres/blob/master/packages/pg-pool/README.md
- 9: Add configurable query timeout brianc/node-postgres#1760
Bound acquisition too, and do not release an in-flight client.
pool.connect() is outside the 2s race, so pool exhaustion can leave this health endpoint hanging indefinitely. Additionally, a timed-out query can continue on PostgreSQL while the driver rejects; releasing that client for reuse can return an unhealthy state to the pool. Wrap acquisition as well as execution, and use server-side statement_timeout plus client.release(true)/new client handling for failed queries.
🤖 Prompt for 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.
In `@apps/one/src/app/api/health/route.ts` around lines 12 - 23, Update the
health-check flow around pool.connect and client.query so the 2-second timeout
covers client acquisition as well as query execution. Configure a server-side
statement_timeout for the health query, and when acquisition or execution times
out or fails, ensure any potentially in-flight client is destroyed via
client.release(true) or equivalent handling rather than returned to the pool;
release normally only after a successful, completed query.
| if [[ -f "/var/lib/oper-alerts/${key}" ]]; then | ||
| "$NOTIFY" --resolved --key "$key" "✅ ${BOX}: ${key} — RESOLVED" | ||
| fi |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Implement a separate, expiry-aware maintenance-silence state.
The runbook writes /run/oper-alerts/<key>, but runtime alert state is stored in /var/lib/oper-alerts/<key> and its contents are treated only as cooldown timestamps. Following the documented maintenance procedure does not silence alerts.
ops/monitoring/healthcheck.sh#L18-L20: check a dedicated silence marker before calling either notification path.ops/monitoring/notify-telegram.sh#L9-L10: retain/var/lib/oper-alertsexclusively for alert-delivery state.ops/monitoring/notify-telegram.sh#L55-L61: do not conflate resolution state with maintenance silencing.documentation/operations/monitoring.md#L37-L50: document the implemented silence directory and expiry behavior.
📍 Affects 3 files
ops/monitoring/healthcheck.sh#L18-L20(this comment)ops/monitoring/notify-telegram.sh#L9-L10ops/monitoring/notify-telegram.sh#L55-L61documentation/operations/monitoring.md#L37-L50
🤖 Prompt for 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.
In `@ops/monitoring/healthcheck.sh` around lines 18 - 20, Implement a separate
expiry-aware maintenance-silence state: in ops/monitoring/healthcheck.sh lines
18-20, check the dedicated silence marker before either notification path; in
ops/monitoring/notify-telegram.sh lines 9-10, keep /var/lib/oper-alerts
exclusively for alert-delivery state; in lines 55-61, keep resolution tracking
separate from maintenance silencing; and in
documentation/operations/monitoring.md lines 37-50, document the silence
directory and expiry behavior.
| # Find the boot disk of the prod server | ||
| BOOT_DISK=$(upctl server storage list "$PROD_SERVER_UUID" --output json 2>/dev/null | \ | ||
| jq -r '[.[] | select(.type=="disk")] | sort_by(.created_at) | last | .uuid' 2>/dev/null || echo "") |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
server_uuid="${PROD_SERVER_UUID:-003b1626}"
upctl server storage list "$server_uuid" --output json |
jq '.[] | select(.type == "disk") | {uuid, created_at, title, description, boot_device}'
upctl server show "$server_uuid" --output json | jq .Repository: jonquixote/OnePercentRealEstate
Length of output: 212
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "Tracked files matching snapshot-cron:"
fd -a 'snapshot-cron\.sh|monitoring' . 2>/dev/null | sed 's#^\./##' | head -50
echo
echo "File excerpt:"
cat -n ops/monitoring/snapshot-cron.sh | sed -n '1,120p'
echo
echo "Search for BOOT_DISK/storage list/jq boot_device:"
rg -n "BOOT_DISK|upctl server storage list|boot_device|created_at|prod server|PROD_SERVER_UUID" .Repository: jonquixote/OnePercentRealEstate
Length of output: 35110
🌐 Web query:
UpCloud upctl server storage list JSON boot_device field upctl server show storage
💡 Result:
To view storage information for an UpCloud server using the command-line interface (upctl), you should primarily use the upctl server show command, as the storage configuration is a property of the server resource [1][2]. The command upctl server show <server_uuid_or_hostname> provides detailed information about a server, including its attached storage devices [1][2]. When outputting this in JSON format, you can use the -o json flag [3]. Regarding the specific fields you mentioned: 1. JSON Structure: Since version 3.0.0, the JSON output of upctl server show reflects the full API response [4][5]. In this schema, storage information is contained within the storage_devices field (which replaced the older storage field) [4][5]. 2. Boot Device: Within the storage_devices block, individual devices contain a boot_disk attribute [6]. In the UpCloud API, a boot_disk value of "1" (or sometimes a boolean true, depending on the representation) indicates that the storage device is designated as a boot disk [6]. Note that the server may also have a boot_order attribute that can override which device is used for booting [6][7]. To inspect storage directly, you can also use upctl storage show <storage_uuid_or_title>, which provides details specific to a storage object, though this command focuses on the storage volume itself rather than its attachment status to a specific server [1][2]. Example command to view server details in JSON: upctl server show <server_uuid_or_hostname> -o json You can use tools like jq to parse the JSON output if you need to extract specific fields like boot_disk [3].
Citations:
- 1: https://upcloud.com/docs/guides/upcloud-command-line-interface/
- 2: https://upcloud.com/docs/guides/get-started-upcloud-command-line-interface/
- 3: https://github.com/upcloudltd/upcloud-cli/blob/main/examples/create_and_restore_backup.md
- 4: https://github.com/UpCloudLtd/upcloud-cli/blob/refs/tags/v3.29.0/CHANGELOG.md
- 5: https://github.com/UpCloudLtd/upcloud-cli/releases/tag/v3.0.0
- 6: https://developers.upcloud.com/1.2/8-servers/
- 7: https://upcloud.com/docs/products/cloud-servers/storage/
Select the actual boot storage device.
created_at only describes when the storage device was created; it has no boot-device semantics. If a later-created data disk is attached, this snapshots the wrong storage. Select the storage device with boot-device semantics from the server configuration or use an explicit boot-disk UUID.
🤖 Prompt for 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.
In `@ops/monitoring/snapshot-cron.sh` around lines 15 - 17, Select the boot
storage device in the BOOT_DISK lookup using an explicit boot-device indicator
from the server storage/configuration response, or use a configured boot-disk
UUID; remove the created_at sorting and last-device fallback so later-created
data disks cannot be selected.
| STATE_DIR="${TMPDIR:-/tmp}/rescue-state-${DEAD_UUID}" | ||
| STATE_FILE="$STATE_DIR/progress" | ||
| mkdir -p "$STATE_DIR" | ||
| chmod 700 "$STATE_DIR" | ||
| touch "$STATE_FILE" |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win
Validate DEAD_UUID before using it in a filesystem path.
Because DEAD_UUID is user-overridable, a value such as /../../etc can make STATE_DIR resolve outside the intended directory; the subsequent chmod and touch can modify arbitrary paths. The predictable /tmp path also remains vulnerable to pre-created symlinks. Validate the UUID format and use a private persistent state root outside /tmp.
🔧 Proposed fix
+[[ "$DEAD_UUID" =~ ^[0-9A-Fa-f-]+$ ]] || {
+ printf 'Invalid dead-server UUID: %s\n' "$DEAD_UUID" >&2
+ exit 2
+}
+
-STATE_DIR="${TMPDIR:-/tmp}/rescue-state-${DEAD_UUID}"
+STATE_ROOT="${XDG_STATE_HOME:-$HOME/.local/state}/onepercent-rescue"
+umask 077
+mkdir -p "$STATE_ROOT"
+STATE_DIR="$STATE_ROOT/$DEAD_UUID"🤖 Prompt for 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.
In `@ops/resilience/rescue.sh` around lines 66 - 70, Validate DEAD_UUID against
the expected UUID format before constructing STATE_DIR, rejecting invalid or
path-like overrides. Replace the predictable /tmp-based state location with a
private persistent state root, and ensure the directory is created and
permissioned securely before STATE_FILE is touched.
- attach-floating-ip.sh: select public interface MAC (was private) for floating IP attachment per UpCloud ip-address modify semantics - deploy-systemd.sh: replace head -5 with sed -n '1,5p' to avoid early pipe closure under pipefail; make Telegram notifier non-fatal so an outage cannot fail an otherwise-healthy deploy - healthcheck.sh: pass --all to systemctl list-units so inactive/failed oper-* units surface as failures instead of being silently skipped - snapshot-cron.sh: propagate list/prune failures instead of swallowing them; alert on delete errors via distinct snapshot-prune key - health/route.ts: race pool.connect() against the 2s timeout so a wedged pool does not stall the health probe indefinitely - rescue.sh: validate DEAD_UUID format (hex/punct only) before it flows into a filesystem path; use printf %s for user-supplied UUIDs in plan output to avoid printf-format-directive interpretation (SC2059)
CodeRabbit review findings — addressed (commit
|
| # | Finding | Fix |
|---|---|---|
| 1 | attach-floating-ip.sh selects wrong interface (private) for floating IP MAC |
Now selects type == "public" per UpCloud ip-address modify --mac semantics |
| 2 | deploy-systemd.sh uses head -5 which can close pipe under pipefail and reject valid sitemap |
Replaced with sed -n "1,5p" (consumes full stream) |
| 3 | deploy-systemd.sh notifier can fail healthy deploy under set -e |
Both fail() and pass() notifier calls now ` |
| 4 | healthcheck.sh only lists active oper-* units — inactive/failed units silently skipped |
Added --all flag so stopped/failed units surface as failures |
| 5 | snapshot-cron.sh swallows prune/listing failures |
List failure now exits 1 + alerts; per-delete failures emit snapshot-prune alert key |
| 6 | health/route.ts timeout did not cover pool acquisition |
pool.connect() now races against the same 2s timeout as query() |
| 7 | rescue.sh DEAD_UUID flows into filesystem path without format validation |
Added ^[0-9A-Fa-f-]+$ regex guard (exits 2 on bad input) |
| 8 | rescue.sh interpolates DEAD_UUID into printf format string (SC2059: % in UUID interpreted as directive) |
Both stop/delete printouts now use printf '" ! upctl server stop %s\n"' "$DEAD_UUID" pattern |
Skipped (out of scope or already-resolved):
- Separating maintenance-silence state from alert dedup state — current design intentionally uses one dir keyed by alert name; separating is a larger design change, deferred to a future wave
snapshot-cron.shBOOT_DISK selection bycreated_at— heuristic is reasonable without additional API response shape knowledge; current.type=="disk"+ newest filter is functional- Pre-resolved nitpicks from the first review (
StrictHostKeyChecking=accept-new,run_cmd "$@",add_memory_highfor oper-scraper) were already in77daad7
Verified: bash -n passes on all scripts, tsc --noEmit passes for apps/one, eslint clean on the modified route file.
…t; was matching zero units) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016ytTRmZoRpCSnhKA3xJURj
1 participant
What
Extends the production resilience stack with full observability — we notice before the user does.
New: Health Endpoint (
/api/health)SELECT 1with 2s timeout) and Redis (best-effortPING){ status, db, redis, buildId, uptimeMs }— HTTP 503 on degradedNew: Telegram Notifier (
notify-telegram.sh)TELEGRAM_BOT_TOKEN/ALERTMANAGER_TELEGRAM_CHAT_ID--resolvedflag clears alertsNew: Host + Service Healthcheck (
oper-healthcheck.timer)oper-*units, HTTP health, oper-twoNew: Automated Snapshots (
oper-snapshot.timer)upctl storage backup createat 04:30 UTCNew: Post-Deploy Smoke Gate (
deploy-systemd.sh)/api/health,/sitemap.xml(XML),/robots.txt(Disallow),oper-two(200)Updated: Documentation
monitoring.md: all checks, thresholds, silencing, restore drillwave-7-open-items.md: DR drill documented, snapshot automation notedWhy
On 2026-07-24 the prod box OOMd and went fully dark. The first signal was the user noticing the site was down. There was no alert, no health check, no automated snapshot cadence. This PR closes that gap.
Files
apps/one/src/app/api/health/route.tsops/monitoring/notify-telegram.shops/monitoring/healthcheck.shops/monitoring/snapshot-cron.shops/systemd/oper-healthcheck.service+.timerops/systemd/oper-snapshot.service+.timerops/systemd/deploy-systemd.shdocumentation/operations/monitoring.mddocumentation/operations/wave-7-open-items.mdVerification
/api/healthreturns{ status: "ok", db: "up", redis: "up" }→ 200notify-telegram.sh --key test "hello"delivers; second call suppressedoper-healthcheck.timerruns every 2 min; stopping a unit triggers alertoper-snapshot.timercreates dailyoper-auto-*backups/sitemap.xmlreturns non-XMLSummary by CodeRabbit
New Features
Improvements