Repository navigation
hoglake sink: pyhoglake 1.3.7 writer path — in-memory prepared appends, read_snapshot kept, zero sink-side reads per flush - #148
Merged
Conversation
…s, read_snapshot kept, zero sink-side reads per flush Pin pyhoglake[fast-upload]>=1.3.7. _prepare encodes each partition's Arrow table to a buffer and uploads it through prepare_append_tables (single-request PutObject for small objects, concurrent fan-out) instead of writing temp files for prepare_append_files. The payload keeps read_snapshot. The pre-commit table GET and _check_destination_still_ours are gone: the server's typed refusals (ddl_since_read_snapshot, table_recreated, the 410 for an expired basis) are handled by discarding the prepared payload, dropping the cached shape and raising retryable so main.py rebuilds. pyhoglake is told to invalidate its own cache on the same refusal. The sink keeps the last TableInfo and re-reads it only on first use, reset, a destination-moved refusal, the alignment self-heal, or when it is older than retention/4 — strictly shorter than pyhoglake's own retention/2 refresh, so the sink can never be the staler of the two. Without that TTL a same-arity partition-spec change landed outside the conflict window after pyhoglake refreshed, and files computed under the old spec would have been accepted and stamped with the live spec_id. The periodic re-read also runs _reconcile_specs. Every remaining info call sends totals=False. The liveness budget charges a 20 s per-attempt prepare allowance, so HOGLAKE_MAX_RETRY_COUNT defaults to 6. The orphan counter gains a reason label. PYHOGLAKE_UPLOAD_CONCURRENCY is validated at startup. The integration stack's server-image pin never reached compose and the suites ran against a stale :latest; the pin is now passed in, asserted via docker inspect, and the compose default requires it.
jghoman
force-pushed
the
jakob/pyhoglake-1.3.7
branch
from
October 1, 2026 17:56
b8423bb to
920f4c8
Compare
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Cache and receipt handling can compromise publication correctness, and the retry budget understates liveness exposure.
Review effort: Balanced
Findings: 4
Open (6)
Validate table UUID before accepting replayed receipts · New Reset sink caches after accepting reused-key receipts · New Clear cached table handle and UUID after recreation refusal · New Reconcile cached layout before returning adopted table info · New Bound total flush time across retries, uploads, and requests · New Select table-info requests independently of query parameters · New
What changed in this PR
Upgrades the Hoglake sink to pyhoglake 1.3.7’s in-memory prepared writer path, reducing upload overhead and steady-state catalog reads.
Changes:
- Uses buffered, concurrent uploads while preserving
read_snapshot. - Adds cache expiry, typed-refusal recovery, labeled orphan accounting, and revised retry validation.
- Expands tests and verifies the integration server image.
| File | Description |
|---|---|
| uv.lock | Locks pyhoglake 1.3.7 with fast-upload dependencies. |
| tests/unit/test_hoglake.py | Tests cached shapes, refusal recovery, and orphan accounting. |
| tests/unit/test_config.py | Tests upload concurrency and retry budgets. |
| tests/integration/test_hoglake_integration.py | Exercises real-server publication and request behavior. |
| tests/hoglake_stack/stack.py | Updates the server digest and adds image inspection. |
| tests/hoglake_stack/docker-compose.yaml | Requires an explicit server image. |
| tests/e2e/test_hoglake_e2e.py | Verifies the running server image. |
| README.md | Documents writer behavior and deployment considerations. |
| pyproject.toml | Raises the client dependency floor and enables fast uploads. |
| millpond/metrics.py | Adds orphan-reason labels. |
| millpond/hoglake.py | Implements buffered preparation and cached-shape recovery. |
| millpond/config.py | Validates concurrency and revises retry budgeting. |
| AGENT.md | Updates writer and dependency documentation. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…sal, full reset on table_recreated, re-reconcile adopted specs, runtime flush deadline
The sink's cached shape must never outlive pyhoglake's writer cache:
pyhoglake invalidates on any non-retryable refusal whose basis it
cached, on 410 and on re_prepare, so the sink now drops its shape at
the top of every HoglakeError path out of commit_prepared, the
reused-key accept included. A table_recreated refusal resets the
handle and uuid as well, so a retry without reset_caches() rebuilds
against the new incarnation instead of re-presenting the dead one.
A shape adopted through schema evolution or the alignment self-heal
is reconciled against config whenever its spec identity differs from
the last verdict, with no table read.
main._write_with_retry stops retrying when the next attempt cannot
finish inside the liveness budget (450 s with the margin), re-raises
the last error and counts errors_total{type="flush_deadline_exceeded"};
the startup arithmetic stays as the model, the deadline is the
enforcement.
The wire test selects table reads by endpoint, which exposed one read
the old filter hid: pyhoglake's Namespace.table() takes no totals
parameter, so a resolve pays the live-totals scan once per resolve.
The receipt-before-incarnation ordering on replay is documented as
the intended exactly-once-by-key semantics.
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
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.


Why
hoglake 1.3.6/1.3.7 moved the writer path (hoglake #234, #237, #239, #255): in-memory prepared appends with concurrent single-request uploads, a cached table shape whose
read_snapshot_idrides the commit asread_snapshotso the server answers "did DDL touch this table since my read" instead of the client re-reading it, typed re-prepare refusals (ddl_since_read_snapshot,table_recreated, a 410 for a basis under the expiry floor), andinfo(totals=False)to skip the live-totals scan. millpond still wrote each partition's parquet to a temp dir, made two table GETs per flush with totals (a count and two sums over ~15M file rows on prod-us), and strippedread_snapshotfrom the payload. The server'sHOGLAKE_REFUSE_BLIND_PARTITIONED_APPENDSflag will refuse payloads without a basis once the fleet sends one.What
pyhoglake[fast-upload]>=1.3.7. Objects ≤ 8 MiB go up as onePutObjectthrough boto3 instead of pyarrow's three-request multipart; uploads fan out on a thread pool (capped at the group count, 64 on the events writer).uv lock; the extra lands in the default dependency set, so the Dockerfile is unchanged (verified with its exactuv synccommand)._prepare→prepare_append_tables. No temp files; each partition's Arrow table encodes to a buffer and uploads from it. Orphan accounting on exceptions (uploaded_files/uploaded_uris) unchanged. Variant destinations: millpond cannot create one andcolumns_to_arrow_schemaalready refuses one, so no fallback path.read_snapshotstays on the payload. The pre-commit table GET and_check_destination_still_oursare gone. Column drift is refused at prepare by the encoded-footer compare and self-heals in one retry with zero orphans. DDL that lands between the basis and the commit comes back as the typed 409/410, handled by a new_destination_movedarm: discard the payload (orphans counted), drop the cached shape, raise retryable so main.py rebuilds.commit_prepared(payload, table=...)so pyhoglake invalidates its own cache on a re-prepare refusal; without it the rebuild could resend the refused basis.TableInfoand re-reads only on first use,reset_caches(), a destination-moved refusal, the alignment self-heal, or when the copy is older than retention/4. That TTL is the correctness guard the review found missing: pyhoglake refreshes its own writer cache at retention/2, and once it has, a same-arity partition-spec change (identity → bucket) is outside the conflict window; a sink still computing values under the old spec would have published mis-partitioned files that the server stamps with the live spec_id. The sink's reads re-seed pyhoglake's cache, so with the shorter TTL the sink can never be the staler of the two. The periodic re-read also runs_reconcile_specs, so the "config vs live spec is fatal" tripwire fires on a long-lived pod, not only at start.totals=Falseon every remaininginfocall. Drop+recreate stays closed byexpected_table_uuid→table_recreated._hoglake_worst_case_flush_scharges a 20 s per-attempt prepare allowance (basis in the comment, flagged as an over-estimate); with it, 8 × 45 s models at ~634 s against the 480 s budget, soHOGLAKE_MAX_RETRY_COUNTdefaults to 6 (~429 s, 51 s margin). A values file pinning 8 explicitly is refused at startup by the existing budget check; no charts values file sets it today (charts/millpond/values.yamlleavesmaxRetryCountempty).millpond_hoglake_orphaned_files_totalgains areasonlabel (prepare_failed,commit_refused,ddl_since_read_snapshot,table_recreated,read_snapshot_expired,already_published,superseded,shutdown), so rollout churn and a broken pod stop looking alike. Summing overreasonis the old series. No dashboard queries the bare series.PYHOGLAKE_UPLOAD_CONCURRENCYvalidated atconfig.load()like every other knob.HOGLAKE_S3_REGIONshould be set explicitly even under IRSA: boto3 resolves region differently from Arrow's SDK (documented).tests/hoglake_stack/stack.pyresolvedDEFAULT_IMAGEfordocker pullonly; compose fell back to:latest, 13 days stale locally, which the first run exposed (a 1.3.7 refusal arriving as a barecommit_conflict). The pin now reaches compose, both session fixtures assert the booted image viadocker inspect, and the compose default is${HOGLAKE_SERVER_IMAGE:?…}so a baredocker compose upcannot resurrect:latest.Behaviour changes to know before promoting
add_columnnow refuses the in-flight flush instead of being invisible to it. Each racing DDL costs one rebuild and one flush's orphaned objects (~270 on the events writer); refusals per flush are bounded by distinct new columns, not by pod count (a loser's alter mints no change row). Thereasonlabel makes this visible.Tests
TestZeroSinkSideReadsPerFlush,TestTypedDestinationMovedRefusals,TestTheCachedShapeHasATtl(9 tests, injected clock; removing the TTL fails 3, raising it to retention/2 fails 4), the rebuild-under-the-new-spec test,totals=Falseasserted at every call site by the harness (all six sites mutation-verified), per-flush orphan(reason, count)assertions. The harness change exposed and fixed one vacuous pre-existing test (test_conflict_on_add_reresolves_and_proceedsnever attempted the add).TestOneRequestPerFlush(five steady-state flushes produce exactly fivePOST .../commit/preparedon the wire, every table GET carriestotals=false), per-flush exactly-once in the concurrent-writers test, the mid-flight re-spec refusal, the warm-cache recreate costing exactly one flush of uploads, and the boto3 transport's orphan accounting with a thread-safe Nth-failure gate.ruff checkandruff format --checkclean on tracked files.Deploy
HOGLAKE_S3_REGIONset (it already is incharts/millpond).millpond_hoglake_orphaned_files_total{reason=...}andhoglake_commits_total{result="ddl_since_read_snapshot"}on the server;hoglake_blind_partitioned_appends_totalshould drop to zero for millpond writers, after which the server flag can flip.