Add a committed, re-runnable Bulk Upload benchmark harness - #2415
Open
jh-RLI wants to merge 2 commits into
Open
Conversation
Every throughput number in the Bulk Upload PRD came from a Django test client against a local Postgres, produced by api/tests/bench_apply.py and api/tests/bench_bulk.py - which were never committed and survive only as stale .pyc files. The numbers cannot be reproduced today. This adds the harness back, in the repository, so the next release re-measures instead of re-deriving. benchmarks/ is a new top-level package rather than scripts/, because scripts/ is gitignored (.gitignore:117) and therefore cannot hold anything meant to be re-run per release. The harness is standard library only and imports no Django: a benchmark has to run from the machine that has the uplink and the reference data, not only from one that can boot the platform. What it does per rung: slice a rung-sized CSV out of a reference file, create a fresh sandbox table, upload it gzipped, time the phases client-side, append a result row, drop the table and confirm it is gone. The slicer is the part that can silently poison every measurement. The fat reference file carries a ~111 KB quoted JSON array per row, full of commas, so a byte slice splits a record, COPY fails on the truncated line and the run measures a rollback rather than a load. Records are therefore found with a quote-aware byte scanner and copied whole; the target size is a target, while the actual bytes and rows are measured. Verified against the real files: the scanner reproduces the stdlib CSV reader's record count on both, conserves every byte of the 2,040,220,631-byte fat file, and a full 1.95 GB slice re-parses as 17,727 rows of exactly five fields. Failures are findings, not absences. A rung refused mid-body records the status, who answered, the bytes that actually left and the response body, then stops the staircase. Who answered is decided by response body format, never by status code alone: Apache emits its own HTML 408 on an idle request body and the platform's stall guard emits a JSON 408. A failed rung reports no throughput at all, because dividing a fraction of the payload by a fraction of the time invents a headline number out of a refusal. Column definitions are configuration, not code, and are replaceable from a JSON file: typing the fat file's `series` as text measures raw ingest while json/jsonb measures validated-JSON ingest, and those are different benchmarks. Each result row carries a fingerprint of the exact table definition it was loaded into, so rungs measured under different typings are never silently compared. The credential is read only from OEP_BENCH_TOKEN and appears in no source file (there is a test for that). Production is write-protected behind an explicit flag, and --insecure is refused against production outright. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The harness could not upload anything larger than 1 MiB. Against production, a 4,224,952-byte gzipped payload timed out after 240 s and left the table with 0 rows, while curl sent the identical payload in 1.13 s. Root cause: the send loop treated a readable socket as "the server has answered". On TLS 1.3 the server sends a NewSessionTicket immediately after the handshake, so select() reports the fd readable with no HTTP response behind it. Measured on production: chunk 1 readable with SSLWantReadError, chunks 2-5 not readable. The loop broke after one 1 MiB chunk with 3.18 MB unsent, then blocked in getresponse() while the server waited for the rest of the declared Content-Length. That also explains the threshold exactly: a body that fits in one SEND_CHUNK is fully sent before the false positive fires, so small rungs passed and every larger one hung. Fix: confirm readability with the SSL layer before believing it. peek_early_response() does a non-blocking recv and treats SSLWantReadError/BlockingIOError as "TLS bookkeeping, keep sending". A genuine early rejection is still honoured -- those bytes are already off the socket, so _finish_from_bytes() parses the response from them rather than calling getresponse(). Also: document that send_seconds measures buffer fill rather than wire time for payloads that fit in the socket buffers (444 KB reported 0.091 s; 4.2 MB reports 1.472 s, which is real). Adds arms_wf05.json, the decided target-table schemas, so runs use the schema of record rather than the provisional built-ins. Verified: 47 tests green; the 10 MB rung now lands on production in 1.76 s (201, 80 rows) against curl's 1.13 s.
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
Every throughput number in the Bulk Upload PRD came from a Django test client against a local Postgres, produced by api/tests/bench_apply.py and api/tests/bench_bulk.py - which were never committed and survive only as stale .pyc files. The numbers cannot be reproduced today. This adds the harness back, in the repository, so the next release re-measures instead of re-deriving.
benchmarks/ is a new top-level package rather than scripts/, because scripts/ is gitignored (.gitignore:117) and therefore cannot hold anything meant to be re-run per release. The harness is standard library only and imports no Django: a benchmark has to run from the machine that has the uplink and the reference data, not only from one that can boot the platform.
What it does per rung: slice a rung-sized CSV out of a reference file, create a fresh sandbox table, upload it gzipped, time the phases client-side, append a result row, drop the table and confirm it is gone.
The slicer is the part that can silently poison every measurement. The fat reference file carries a ~111 KB quoted JSON array per row, full of commas, so a byte slice splits a record, COPY fails on the truncated line and the run measures a rollback rather than a load. Records are therefore found with a quote-aware byte scanner and copied whole; the target size is a target, while the actual bytes and rows are measured. Verified against the real files: the scanner reproduces the stdlib CSV reader's record count on both, conserves every byte of the 2,040,220,631-byte fat file, and a full 1.95 GB slice re-parses as 17,727 rows of exactly five fields.
Failures are findings, not absences. A rung refused mid-body records the status, who answered, the bytes that actually left and the response body, then stops the staircase. Who answered is decided by response body format, never by status code alone: Apache emits its own HTML 408 on an idle request body and the platform's stall guard emits a JSON 408. A failed rung reports no throughput at all, because dividing a fraction of the payload by a fraction of the time invents a headline number out of a refusal.
Column definitions are configuration, not code, and are replaceable from a JSON file: typing the fat file's
seriesas text measures raw ingest while json/jsonb measures validated-JSON ingest, and those are different benchmarks. Each result row carries a fingerprint of the exact table definition it was loaded into, so rungs measured under different typings are never silently compared.The credential is read only from OEP_BENCH_TOKEN and appears in no source file (there is a test for that). Production is write-protected behind an explicit flag, and --insecure is refused against production outright.
Workflow checklist
Automation
Closes #
PR-Assignee
CONTRIBUTING.md
CHANGELOG.md
mkdocs
Reviewer
Reviewer Guidelines