feat(api): bulk upload observability — structured per-attempt logging (#2362)#2372
Open
jh-RLI wants to merge 12 commits into
Open
feat(api): bulk upload observability — structured per-attempt logging (#2362)#2372jh-RLI wants to merge 12 commits into
jh-RLI wants to merge 12 commits into
Conversation
Adds POST /api/v0/tables/<table>/bulk-upload - the tracer bullet of the bulk upload path (slice 2 of #2362): - The request body IS the CSV (text/csv); the server streams it into PostgreSQL COPY FROM STDIN without buffering the file in memory. - Append-only, one transaction per request: a malformed row anywhere rolls back the entire upload. - The delimiter is a required, whitelisted parameter (comma, semicolon, tab) - never inferred from metadata or content. - The CSV header (required) maps columns by name; header names are whitelisted against the table's actual columns and quoted, so no unvalidated identifier ever reaches the SQL. - Same authorization chain as the row API: token auth, write permission, embargo check, table-registry resolution (internal tables are unreachable by construction). - Deliberately bypasses the edit-journal meta tables: bulk-loaded rows have no per-row change history. This trades the (currently unread) per-row provenance for order-of-magnitude ingestion speed; an audit event record follows in a later slice. - COPY is FROM STDIN only; no code path for COPY FROM file/PROGRAM. New HTTP-seam test module api/tests/test_bulk_upload.py (14 tests) covers the happy path per delimiter, auth/permission/embargo denials, all-or-nothing rollback, journal bypass, and target-table containment. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Slice 3 of #2362, on top of the bulk upload tracer bullet: - Header preflight before the body streams: reject duplicate column names, names not in the table, and missing required columns (NOT NULL without default), each with a 400 naming the offenders. - Strip a UTF-8 BOM from the header (Excel exports). - FORCE_NULL on all uploaded columns: an empty field is NULL whether quoted or not - a deliberate deviation from COPY's native CSV rule, because many writers quote every field and would silently store empty strings instead of NULLs. - Sanitized failure responses: the database's data-level message plus the CSV line number and column (header-adjusted), never raw SQL, server context dumps, or internal paths - including the no-diagnostics fallback (lost connection), which stays generic. Test module grows to 20 HTTP-seam tests covering each contract rule. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
#2362) Slice 4 of #2362. Clients may include or omit the id column: - Omitted: the table's id sequence assigns ids as usual. - Included: after COPY, still inside the upload's transaction, the id sequence is advanced to the table's max(id) via setval(GREATEST(...)), so a subsequent row-API insert cannot hit a duplicate key. GREATEST plus a pg_advisory_xact_lock on the sequence keep the sequence from ever moving backwards, including under concurrent uploads (setval is non-transactional, so racing reads could otherwise regress it). - Uploads that RAISE the table's max(id) above a generous sanity bound (2^48) are rejected and rolled back, so a single upload cannot exhaust the id sequence for every writer of a shared table. The bound only judges ids introduced by the upload itself: a pre-existing high id (the row API enforces no bound) does not poison the table for future bulk uploads. Test module grows to 25 HTTP-seam tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Slice 5 of #2362. Bulk uploads bypass the per-row edit journal; the new BulkLoadEvent model is their provenance: - Every authenticated, authorized attempt - successful or failed - is recorded: table, user, timestamp, status (success / validation-error / copy-error / embargo), sanitized error message, and bytes received. Decorator-level denials (401/403/404) deliberately create no events so anonymous requests cannot write database rows. - Successes additionally record the row count and the exact id range of the loaded rows (found via xmin = current transaction, so it is correct for both explicit and sequence-assigned ids); the response references the event id. The id range is the forensics handle for block-deleting a mistaken or malicious upload. - Events live in the Django database, so a failure event survives the data transaction's rollback by construction. Event creation is best-effort: a failure to write the audit record is logged but never masks the upload's actual outcome. - Events are listed, filterable, and immutable in the Django admin. Test module grows to 30 HTTP-seam tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Slice 6 of #2362, part 1/3. Content-Encoding: gzip (and legacy x-gzip) request bodies are decompressed in streaming fashion straight into COPY FROM STDIN - gzip.GzipFile pulls compressed chunks on demand, so the whole body is never materialized in memory. Corrupt or truncated gzip aborts with a sanitized 400 (validation-error event) and full rollback, whether it fails at the header or mid-COPY. Tests: gzip round-trip vs plain twin, invalid gzip, truncated gzip mid-body.
Slice 6 of #2362, part 2/3. Adds the size-cap status to BulkLoadEvent (with migration) so uploads rejected by the decompressed size cap are distinguishable in the audit trail and admin filters. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Slice 6 of #2362, part 3/3. A configurable cap on DECOMPRESSED bytes per request (settings.BULK_UPLOAD_MAX_BYTES, env-overridable, default 10 GiB) rejects oversized uploads with HTTP 413 naming the cap and a size-cap Bulk Load Event. Counting after decompression is what neutralises gzip bombs: a few-KB body that inflates past the cap is cut off mid-stream, not expanded to disk or RAM. The cap is a backstop - clients are expected to split large datasets into several uploads. psycopg2 surfaces exceptions raised inside COPY's read() as a generic Error, so the stream flags the overflow and the error handler dispatches 413 size-cap vs 400 copy-error on that flag. Tests: plain-body cap breach (413, names the cap, rolled back, event recorded) and gzip bomb cut off at the cap. Module at 35 tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ap (#2362) Slice 7 of #2362, part 1/3. New module api/bulk_upload_guard.py: at most one running bulk upload per user plus a configurable global cap (BULK_UPLOAD_MAX_CONCURRENT, default 2). Excess requests get HTTP 429 with Retry-After and create no event - busy rejections are cheap pre-work denials. Slots are always released, including on error paths. The guard state is per worker process (documented: a multi-worker deployment bounds concurrency per process, still capping the damage). The guard module is deliberately its own seam with direct unit tests - concurrency cannot be exercised through the synchronous test client. HTTP-seam tests cover the wiring: per-user 429 and global-cap 429, both with Retry-After.
Slice 7 of #2362, part 2/3. Uploads whose average transfer rate falls below BULK_UPLOAD_MIN_BYTES_PER_SECOND (default 10 KiB/s) after a grace period (BULK_UPLOAD_STALL_GRACE_SECONDS, default 30 s) are aborted with HTTP 408, fully rolled back, and recorded as a stall Bulk Load Event (migration 0051) - a trickling client must not pin a synchronous worker and an open transaction. The StallDetector lives in the guard module with an injectable clock for its unit tests; it only catches trickle (each read eventually returns) - fully hung connections are bounded by the web server's timeouts and the DB session timeouts (next commit). Like the size cap, the abort surfaces through psycopg2 as a generic error, so a flag on the detector decides the failure class.
Slice 7 of #2362, part 3/3. The upload's transaction runs under SET LOCAL statement_timeout (BULK_UPLOAD_STATEMENT_TIMEOUT_MS, default 1 h) and idle_in_transaction_session_timeout (BULK_UPLOAD_IDLE_TX_TIMEOUT_MS, default 60 s), so a client that goes silent mid-protocol cannot hold an open transaction and its locks. SET LOCAL scopes both to the transaction, keeping the pooled connection clean for its next user; the in-code fallbacks mirror the settings defaults and fail closed (never 0/disabled). Together with the concurrency and stall guards this completes the worker-protection story: guard slot -> transfer rate -> statement and idle timeouts, each bounding a different way to pin a worker. Includes the changelog entry for the complete guards feature.
Slice 8 of #2362, part 1/2. bulk_upload_csv now measures where each upload's time goes: transfer (header read plus time spent inside stream reads - client I/O and gzip decompression), copy (COPY wall time minus that transfer share, i.e. database-side work), and setval (id-contract and id-range queries). The COPY wall clock is captured in a finally, so failure outcomes attribute their time truthfully instead of reporting copy_s=0. Timings travel in the success stats dict and on APIErrors (bulk_timings), alongside the existing error class and byte count, for the caller's log line.
Slice 8 of #2362, part 2/2. Every attempt that reaches the endpoint body emits exactly one structured (logfmt) line on the oeplatform.bulk_upload logger: bulk_upload table=<name> user=<username> outcome=<outcome> rows=<n|-> bytes=<n> total_s=<s> transfer_s=<s|-> copy_s=<s|-> setval_s=<s|-> Outcomes cover the full vocabulary: success, validation-error, copy-error, size-cap, stall, embargo, busy (429), and error (a catch-all for unexpected failures, which now also leave an event and a line instead of vanishing). Phase timings separate client transfer time from database-side COPY work and id-sequence bookkeeping, so a slow upload can be attributed to the uplink or the server at a glance. The format is documented on the logging helper for the observability ops follow-up (canary, dashboards, alerting). Together with the BulkLoadEvent table this completes the endpoint's shipped observability and the final slice of the bulk upload plan. Seven new HTTP-seam log tests (assertLogs): one line per outcome, exactly one line per attempt (55 tests across both modules).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary of the discussion
This PR stacks ontop of #2364 and #2366 and #2367 and
#2368 and
#2369 and
#2370 wait until both are merged and rebase this PR.
Part of #2362 (Slice 8 — observability, the final slice). Every bulk
upload attempt that reaches the endpoint body emits exactly one structured
(logfmt) line on the
oeplatform.bulk_uploadlogger:success,validation-error,copy-error,size-cap(413),stall(408),embargo(403),busy(429), anderror— a catch-all that makes unexpected failures leave an event anda line instead of vanishing (previously a non-APIError crash was
invisible to both audit trails).
decompression, measured inside the stream reads) from database-side COPY
work (COPY wall time minus the transfer share) and the id-sequence
bookkeeping — a slow upload is attributable to the uplink or the server
at a glance. The COPY wall clock is captured in a
finally, so failureoutcomes report their true copy time (review caught the
copy_s=0.000-on-stallbug before commit).follow-up issue (canary,
pg_stat_statements, dashboards, alerting).Together with the BulkLoadEvent table (queryable metrics source), this is
the endpoint's shipped observability.
Tests: seven HTTP-seam log tests via
assertLogs— one line peroutcome, exactly one line per attempt; 55 tests across the two bulk-upload
modules. Full suite green. Changelog entry included.
Workflow checklist
Automation
Closes #
PR-Assignee
CONTRIBUTING.md
CHANGELOG.md
mkdocs
Reviewer
Reviewer Guidelines