Skip to content

perf: cap query time, bound discussion recursion, isolate health checks - #375

Merged
kuny0707 merged 2 commits into
masterfrom
perf/pool-exhaustion-fixes
Aug 1, 2026
Merged

perf: cap query time, bound discussion recursion, isolate health checks#375
kuny0707 merged 2 commits into
masterfrom
perf/pool-exhaustion-fixes

Conversation

@ety001

@ety001 ety001 commented Jul 31, 2026

Copy link
Copy Markdown
Member

Summary

Address the aiopg connection-pool exhaustion that causes periodic steemit-production-beta-hivemind-001 health degradation (34–48% HTTP 4xx, EB instance replacement every 12–24h). Related: PR #374 (partial fix already deployed).

Root cause: all 25 pool connections per instance get consumed by slow get_discussion queries on deep nested threads. The /health check (which also needs a DB connection) then times out → ELB marks the instance unhealthy → EB replaces it.

Changes

  • db.py — statement_timeout=30s (P0): Set via the standard libpq options string (-c statement_timeout=30000) so PostgreSQL cancels any runaway query server side (observed up to 160s) and releases its connection immediately. The pool acquire timeout only bounds waiting for a free connection, NOT execution time. (options is used rather than the newer server_settings dict — see the deployment note below: the runtime image's older psycopg2 rejects server_settings as an invalid DSN option.)
  • db.py — isolated health engine (P1): Add a dedicated maxsize=1 engine + query_row_health() so /health and /head_age cannot be starved by a saturated main pool — the direct cause of the ELB killing healthy instances.
  • thread.py — bound recursion (P0): Cap _load_discussion with MAX_DEPTH=50 / MAX_THREAD_POSTS=500. Previously a deep/wide thread issued an unbounded number of sequential _child_ids queries (one per depth level).
  • thread.py — cache hide-id lookups (P2): Cache _get_author_hide_id / _check_posts_hide_id (300s TTL) to drop two per-request connections from every get_discussion call.
  • Tests: Add tests/bridge_thread/ pure-logic unit tests (no live DB needed) — 6 tests covering the depth/post caps, leaf termination, and cache-param forwarding. Placed outside tests/server/ because tests/server/__init__.py eagerly opens a real DB connection.

Verification

Unit + lint

  • ✅ Unit tests: 6 passed (run in an isolated venv with project deps installed)
  • ✅ pylint (W/E/F categories, matching make test-lint): zero new warnings vs. master baseline on changed code
  • ⚠️ Full make test not run in this environment (Python 3.14 + deps not preinstalled; existing tests/server/ condenser integration tests require a live populated DB and are pre-existing/stale)

Dev deployment + live API tests (steemit-dev-hivemind-001)

Deployed as image steemit/hivemind:pr375-test2 (version pr375-test2-20260731055551). EB health Green/Ok/Ready, VersionLabel correctly switched. Live tests via SSM port-forward (dev ELB is internal/VPC-only):

Check Result
/health endpoint ✅ HTTP 200, status:OK, db_head_age:6s
bridge.get_discussion functionality ✅ Returns full post objects
MAX_THREAD_POSTS=500 cap ✅ An 850-children thread returns 497 posts and completes in ~2s (previously unbounded recursion)
/health isolation under pool load ✅ With the main pool saturated (15 concurrent heavy get_discussion, each ~17s), /health still returns in 251ms→913ms — never hits the 10s acquire timeout. Old behavior here was an ELB-killing timeout.

The ~660ms /health slowdown under load is I/O contention on the small dev instance, not pool starvation (that would push it to the 10s timeout). The fix keeps the instance alive in exactly the scenario that used to kill it.

Deployment note: psycopg2 version mismatch (fixed in 45cab86)

The first dev deploy (pr375-test1) failed to start. The runtime image is Ubuntu 18.04 / Python 3.6 with an older psycopg2 whose make_dsn() rejects the newer server_settings dict keyword:

psycopg2.ProgrammingError: invalid dsn: invalid connection option "server_settings"

This crashed init_db on startup → server never came up → ELB healthcheck failed → deploy rolled back. A local venv (newer psycopg2) accepted server_settings and hid the problem. Fix (45cab86): switched to the libpq options connection string (-c statement_timeout=30000), accepted by every psycopg2/libpq version, merged into conf.query to avoid clashing with a DATABASE_URL that carries its own options=. Lesson: DB-connection code changes must be validated by actually building the image and deploying to dev, not only by a local venv.

Decision: index check NOT included (original P1)

The incident doc suggested a missing (author, permlink) composite index. Investigation found this is not the case: hive_posts already has unique constraint hive_posts_ux1 (author, permlink) (backed by a B-tree index), and hive_posts_status already has idx_hive_posts_status_list_type_author (list_type, author) and idx_hive_posts_status_list_type_post_id (list_type, post_id) since migration v21. The 160s query was therefore caused by pool/I/O contention, not a missing index. No schema change (DB_VERSION bump) is warranted. Reviewers can confirm with:

SELECT indexname, indexdef FROM pg_indexes
WHERE tablename IN ('hive_posts', 'hive_posts_status') AND indexdef ILIKE '%author%';

Out of scope

  • Read replica for heavy queries (original P2): Deferred — requires RDS replica infra provisioning, not a pure-code change. Tracked for a separate PR.

Risk

  • statement_timeout=30s: rare legitimate long queries will start failing fast. Those are exactly the queries exhausting the pool; fail-fast is preferable to taking down the instance. Tunable via STATEMENT_TIMEOUT_MS.
  • Recursion caps: very large threads (>500 comments) return a partial tree. Acceptable vs. 500 errors.
  • Health engine: +1 connection per instance (25→26), negligible.

ety001 added 2 commits July 31, 2026 12:53
Address the aiopg connection-pool exhaustion that causes periodic
steemit-production-beta-hivemind-001 health degradation (34-48% 4xx,
EB instance replacement every 12-24h).

- db.py: set statement_timeout=30s via server_settings so a single
  runaway query (observed up to 160s) is cancelled server-side and
  releases its connection immediately. The pool acquire timeout only
  bounds waiting for a free connection, not execution time.
- db.py: add an isolated maxsize=1 health engine + query_row_health()
  so /health and /head_age cannot be starved by a saturated main pool
  (root cause of ELB marking the instance unhealthy).
- thread.py: bound _load_discussion with MAX_DEPTH=50 and
  MAX_THREAD_POSTS=500; previously a deep/wide thread issued an
  unbounded number of sequential _child_ids queries.
- thread.py: cache _get_author_hide_id / _check_posts_hide_id (300s)
  to drop two per-request connections from every get_discussion call.
- Add tests/bridge_thread/ pure-logic unit tests (no live DB needed).
…pat)

The runtime image ships an older psycopg2 whose make_dsn() rejects the
newer 'server_settings' dict keyword with 'invalid connection option
"server_settings"', crashing init_db on startup and preventing the
server from coming up (verified in steemit-dev-hivemind-001 deploy).

Switch to the standard libpq 'options' connection string
('-c statement_timeout=30000'), accepted by every psycopg2/libpq
version. Merge into conf.query so a DATABASE_URL that already carries
its own options= param does not trigger a duplicate-keyword error;
existing options are preserved and the timeout is appended.
@kuny0707
kuny0707 merged commit c89b5f5 into master Aug 1, 2026
1 check passed
@ety001
ety001 deleted the perf/pool-exhaustion-fixes branch August 1, 2026 00:57
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.

2 participants