chore: back-merge hotfix/v1.1.5 into production - #787
Conversation
Prod cycled between 0-1 instances and returned site-wide 500/503 with "Request was aborted after waiting too long to attempt to service your request." Requests died in the pending queue (blank instanceId, ~2ms latency), not in app code — App Engine never scaled out past one instance despite max_instances=10. Root cause: the scheduler had no visibility into real per-instance concurrency (gunicorn -w 4), so it kept routing bursts to a single saturated instance instead of spinning up more. min_instances=0 added cold-start pile-ups on top. - app.template.yaml: add max_concurrent_requests: 6 so the scheduler scales out before an instance saturates (2-worker headroom for bursts) - CD_production.yml: MIN_INSTANCES 0 -> 1 to kill cold-start pile-ups - CD_production.yml: gunicorn -w 4 -> -w 8 for more concurrency per F4 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
fix(deploy): prevent App Engine request starvation under burst load
…nches--hotfix/v1.1.1--components--OcotilloAPI chore(hotfix/v1.1.1): release 1.1.1
…ease tag passthrough The v1.1.1 production deploy failed at `alembic upgrade head` with "Can't locate revision identified by 'e2f3a4b5c6d7'". The production database (ocotillo) is already migrated to head e2f3a4b5c6d7 — 7 revisions ahead of the v1.1.0 line this hotfix branched from — so the hotfix's older migration set (head t6u7v8w9x0y1) couldn't resolve the DB's current revision. Bring the 7 intervening migration files (u8v9w0x1y2z3 .. e2f3a4b5c6d7) onto the hotfix line so its alembic head matches the production DB exactly. `alembic upgrade head` then no-ops on production (DB already at that revision); no schema change ships. App code stays at the v1.1.0 level — this is already how production runs today — so the only behavioral change to prod is the App Engine scaling config from v1.1.1. Also carry the release-please tag-passthrough fix (path-scoped `.--tag_name`) so the resulting v1.1.2 release actually triggers the production deploy instead of silently skipping. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…-rev fix(deploy): align hotfix migration head with production DB (unblock v1.1.x deploy)
…nches--hotfix/v1.1.1--components--OcotilloAPI chore(hotfix/v1.1.1): release 1.1.2
The /geospatial endpoint (geojson and shapefile formats) loaded the entire thing/location result set into memory on a single request, spiking memory enough for App Engine to terminate the process mid-request. - get_thing_features: stream the query result via yield_per instead of buffering the whole table with .all(). unique() still dedups eager-loaded rows. Callers iterate exactly once. - geojson format: stream the FeatureCollection feature-by-feature through a StreamingResponse instead of building a full list of feature dicts. - shapefile format: write to a tempfile.mkdtemp() dir instead of the read-only App Engine app directory (/tmp is RAM-backed), and clean up the temp dir via BackgroundTask after the response is sent. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
pyproject version was bumped to 1.1.0 without re-locking, so CI's `uv sync --locked` failed. Regenerate the lockfile; no dependency changes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
SQLAlchemy raises "Can't use the ORM yield_per feature in conjunction with unique()", and unique() is required to dedup eager-loaded rows. Revert to unique().all(); the memory win still comes from streaming the JSON response and writing the shapefile to a temp dir instead of building a second full copy / writing to the read-only app dir. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- create_shapefile defined 2 DBF fields (id, name) but wrote 3 values (id, name, elevation), which raises a field/record mismatch at runtime. This was latent because writes to the read-only app dir failed first; now that the shapefile is written to a temp dir it is exercised. Add the elevation field and fix the id field type (was "L"/logical -> "N"). - Wrap shapefile generation in try/except so the temp dir is removed if generation fails (BackgroundTask only runs on a successful response). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
.all() still materialized the entire result set, so both exports loaded everything into memory. Thing has no eager-loaded collections (all relationships are lazy), so unique() was unnecessary -- and unique() is incompatible with yield_per anyway. Make get_thing_features a generator that streams via yield_per and dedups defensively by id with a bounded int set. Because the geojson StreamingResponse body is produced after the request session is closed, the generator now opens a dedicated session via session_ctx() scoped to the stream. The shapefile path consumes the generator inside the endpoint (request session still open). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
fix: stop per-request OOM on /geospatial export endpoint
…nches--hotfix/v1.1.3--components--OcotilloAPI chore(hotfix/v1.1.3): release 1.1.3
A called workflow inherits the caller's event context, so
`github.event_name` inside CD (Production) is the caller's event (`push`
from release-please), never `workflow_call`. The old gate
startsWith((github.event_name == 'workflow_call' && inputs.tag_name)
|| github.event.release.tag_name, 'v')
therefore always fell through to `github.event.release.tag_name`, which
is empty on a push, so `production-deploy` was skipped every time
release-please invoked the deploy inline. Only release-event deploys
worked. Gate on `inputs.tag_name` directly (empty on the release path,
so `||` still falls through). Same fix already live on staging.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…gate-v1.1.3 fix(ci): deploy on inline workflow_call to CD (Production)
…nches--hotfix/v1.1.3--components--OcotilloAPI chore(hotfix/v1.1.3): release 1.1.4
Production ran gunicorn -w 8 on an F4 (1 GB) instance. Eight workers each importing the full stack (sqlalchemy + geoalchemy2 + shapely + cloud-sql connector + pygeoapi) exceeded 1 GB, so App Engine terminated processes for "using too much memory" ~44x per 3h and cycled workers continuously (304 "Booting worker", 297 SIGTERM in a 3h window). With the single min-instance constantly reborn, every request hit a booting instance and re-cold-loaded the app -- site-wide degraded latency even at low traffic. Raise instance_class F4 -> F4_1G (2 GB) so the eight workers fit, per App Engine's own "consider a larger instance class" guidance. Keeps the -w 8 / max_concurrent_requests: 6 scale-out tuning from the prior starvation fix intact. Template is shared; staging/testing (-w 4, scale-to-zero) gain harmless headroom at negligible idle cost. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…nches--hotfix/v1.1.5--components--OcotilloAPI chore(hotfix/v1.1.5): release 1.1.5
There was a problem hiding this comment.
Pull request overview
Back-merges the cumulative hotfix/v1.1.1→v1.1.5 changes into production so the branch history matches what’s already deployed from release tags, including geospatial OOM mitigations, release/deploy workflow fixes, and several DB/Alembic additions for NMW/OGC and pg_cron scheduling.
Changes:
- Aligns
productionmetadata and release automation with the already-deployedv1.1.5hotfix series (manifest, changelog, release-please outputs). - Reduces memory pressure in geospatial exports by streaming GeoJSON output and writing shapefiles in a temp directory with cleanup.
- Adds multiple Alembic migrations for NGWMN views, transducer daily materialized view, NMW mirror tables + OGC views, and an optional pg_cron nightly refresh job.
Reviewed changes
Copilot reviewed 15 out of 16 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
uv.lock |
Updates lockfile editable package metadata (currently version mismatch noted). |
pyproject.toml |
Bumps project version to 1.1.5. |
.release-please-manifest.json |
Updates release manifest version to 1.1.5. |
CHANGELOG.md |
Adds changelog entries for 1.1.1–1.1.5. |
.github/workflows/release-please.yml |
Fixes release-please outputs wiring to ensure tag propagates to CD. |
.github/workflows/CD_production.yml |
Adjusts deploy settings (gunicorn workers, min instances). |
.github/app.template.yaml |
Raises App Engine instance class and adjusts scaling/concurrency settings. |
api/geospatial.py |
Streams GeoJSON response and writes shapefile artifacts in /tmp with cleanup. |
services/geospatial_helper.py |
Streams DB results for geospatial exports; updates shapefile DBF schema. |
alembic/versions/u8v9w0x1y2z3_add_ngwmn_views_from_ocotillo_model.py |
Adds NGWMN export views sourced from the Ocotillo model. |
alembic/versions/v0w1x2y3z4a5_add_transducer_daily_data_materialized_view.py |
Adds transducer_daily_data materialized view + indexes. |
alembic/versions/w1x2y3z4a5b6_drop_child_release_filters_from_ngwmn_views.py |
Adjusts NGWMN views to avoid filtering child rows by unset release_status. |
alembic/versions/x2y3z4a5b6c7_schedule_nightly_matview_refresh_pg_cron.py |
Optionally registers a pg_cron nightly materialized view refresh job. |
alembic/versions/c0d1e2f3a4b5_nmw_mirror_tables.py |
Adds NMW mirror tables and FK constraints (docstring/header inconsistency noted). |
alembic/versions/d1e2f3a4b5c6_nmw_per_well_geothermal_ogc_views.py |
Adds per-well geothermal OGC views/materialized view over NMW mirror tables. |
alembic/versions/e2f3a4b5c6d7_nmw_measurement_ogc_views.py |
Adds individual-measurement and analytical OGC views for NMW. |
| # Stream the result with yield_per so the whole table is never buffered in | ||
| # memory at once. Thing has no eager-loaded collections (all relationships | ||
| # are lazy), so unique() is unnecessary -- and unique() is incompatible with | ||
| # yield_per anyway. Dedup defensively by id with a bounded set of ints in | ||
| # case the joins ever produce duplicate rows. | ||
| seen = set() | ||
| result = session.execute(sql.execution_options(yield_per=1000)) | ||
| for row in result: | ||
| thing_id = row[0].id | ||
| if thing_id in seen: | ||
| continue | ||
| seen.add(thing_id) | ||
| yield row |
There was a problem hiding this comment.
Declining this one — the premise doesn't hold.
latest_assoc groups by thing_id on max(effective_start), which selects a single timestamp per thing, not a single row. If two LocationThingAssociation rows for the same thing_id share that same effective_start, both satisfy the join predicate and both come back. The seen set is guarding exactly that tie case, not a hypothetical one.
On cost: the set holds ints, not rows — roughly 60 bytes per entry against the fully-buffered result set the yield_per change removed. That trade is the whole point of the fix.
If we want the guard gone, the right move is to make the tie-break deterministic in SQL (DISTINCT ON (thing_id) ... ORDER BY thing_id, effective_start DESC, id) rather than dropping the dedup and hoping the join never produces duplicates. Leaving as-is for now; noted as a possible follow-up.
| """NMW staging mirror tables and FK constraints | ||
|
|
||
| Revision ID: c0d1e2f3a4b5 | ||
| Revises: t6u7v8w9x0y1 |
There was a problem hiding this comment.
Confirmed. The docstring says Revises: t6u7v8w9x0y1 but down_revision on line 47 is x2y3z4a5b6c7. The executable value is the correct one — only the docstring is stale, so the migration graph itself is fine.
Not fixing it in this PR: per docs/release-flow.md §3 step 4, the hotfix/vX.Y.Z → production back-merge is a pure merge with no new commits. Tracking this on a staging branch alongside the two substantive findings below.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3d321090c9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
|
|
||
| def upgrade() -> None: | ||
| if not _pg_cron_enabled(): |
There was a problem hiding this comment.
In the production deploy workflow I checked .github/workflows/CD_production.yml lines 74-82, and the migration step does not pass ENABLE_PG_CRON. In that production path _pg_cron_enabled() is false here, so this revision returns and Alembic records it as applied without creating refresh_materialized_views() or the cron job; adding the variable later will not rerun this migration, leaving the nightly materialized-view refresh unscheduled.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed, and it's worse than a branch-config issue — this is already the live state in production.
Verified:
ENABLE_PG_CRONis absent fromCD_production.ymlonproduction, onhotfix/v1.1.5, and on this branch. Onlystaginghas it.- Migration
x2y3z4a5b6c7shipped inv1.1.2and was deployed to production.
So the production run hit _pg_cron_enabled() → False, returned early, and Alembic stamped the revision. refresh_materialized_views() and the cron job were never created, and — exactly as you note — adding the env var now will not rerun the migration.
To be clear on blast radius: CD_production.yml has a separate "Refresh materialized views on production database" step that runs the CLI refresh on every deploy, so the views are not stale indefinitely. What's actually missing is the nightly cadence between deploys.
The fix needs both halves, since the revision is already stamped:
ENABLE_PG_CRON: "1"on the production migration step.- A new migration that creates the helper function and schedules the job.
Not landing it here — this PR is a pure back-merge per docs/release-flow.md §3 step 4. Fixing on a staging branch.
| JOIN "NMW_WellRecords" AS r ON r."RecrdSetID" = s."RecrdsetID" | ||
| JOIN "NMW_WellZDatum" AS z ON z."RecrdsetID" = r."RecrdSetID" | ||
| JOIN "NMW_WellHeaders" AS hdr ON hdr."WellDataID" = r."WellDataID" | ||
| JOIN "NMW_WellLocations" AS loc ON loc."WellDataID" = r."WellDataID" |
There was a problem hiding this comment.
When a well has more than one NMW_WellLocations row for the same WellDataID—the previous geothermal-view migration explicitly handles this with DISTINCT ON because OBJECTID is the PK—this direct join multiplies a single BHT measurement into multiple rows with the same d."OBJECTID" id. That gives pygeoapi duplicate feature IDs and repeated features; use the same deduped location CTE before joining here and in the other measurement views that join NMW_WellLocations directly.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed. The precedent you're pointing at is explicit: d1e2f3a4b5c6 wraps NMW_WellLocations in a DISTINCT ON ("WellDataID") ... ORDER BY "WellDataID", "OBJECTID" CTE, and its own docstring states the reason — a well can have multiple location rows because OBJECTID is the PK, not WellDataID.
This migration joins NMW_WellLocations directly at four sites: lines 82, 123, 199, and 266. Each one can fan a single measurement row into N rows sharing the same id, which surfaces as duplicate feature IDs in pygeoapi.
Fix will reuse the same deduped-location CTE at all four joins. Since these views are already applied in production, it has to be a new migration that recreates them, not an edit to this file.
Not landing it here — this PR is a pure back-merge per docs/release-flow.md §3 step 4. Fixing on a staging branch.
The back-merge kept the stale root package version (1.1.0) in uv.lock while pyproject.toml is 1.1.5, so `uv sync --locked` failed in CI. Regenerated. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 16 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
services/geospatial_helper.py:96
- The comment says the dedup uses a "bounded set of ints", but
seen = set()grows with the number of unique Thing IDs returned (potentially large). This is misleading documentation for the OOM/streaming behavior.
# Stream the result with yield_per so the whole table is never buffered in
# memory at once. Thing has no eager-loaded collections (all relationships
# are lazy), so unique() is unnecessary -- and unique() is incompatible with
# yield_per anyway. Dedup defensively by id with a bounded set of ints in
# case the joins ever produce duplicate rows.
alembic/versions/c0d1e2f3a4b5_nmw_mirror_tables.py:5
- The module docstring says this migration "Revises: t6u7v8w9x0y1", but the actual
down_revisionisx2y3z4a5b6c7. This mismatch can confuse operators when inspecting migration history.
Revision ID: c0d1e2f3a4b5
Revises: t6u7v8w9x0y1
Create Date: 2026-06-22
Why
The last 5 hotfix releases (
v1.1.1–v1.1.5) were tagged and deployed to production, but their commits were never merged back into theproductionbranch. Production deploys off the release tag, so the running service is correct, but theproductionbranch history is 24 commits behind the code it's actually running.This is release-flow.md §3 step 4 (
hotfix/vX.Y.Z → production), which never ran for any of the 5 hotfixes. Without it, the next documented release risks regressing these fixes.What
Back-merges
hotfix/v1.1.5(cumulativev1.1.1→v1.1.5) intoproduction. No new release is cut — the release commits are already in the branch.Merge was clean, zero conflicts. Verified post-merge:
.release-please-manifest.json→1.1.5pyproject.tomlversion →1.1.5CD_production.ymldeploy-gate not duplicated (the CI fix that existed on both sides reconciled cleanly)api/geospatial.pyretains the OOM streaming fixBrings in: geospatial OOM fixes (
api/geospatial.py,services/geospatial_helper.py), F4_1G instance-class bump, and the nmw mirror / OGC-view / ngwmn / transducer-matview / pg_cron migrations.Next
Followed by a
production → stagingPR (§3 step 5) to propagate the one substantive fix staging is still missing (the geospatial OOM streaming fix).🤖 Generated with Claude Code