Skip to content

Feature iceberg: hy-ib-2: Iceberg on S3 + commit hardening (hy-xq70) - #17

Merged
philcunliffe merged 1 commit into
integration/icebergfrom
polecat/hy-xq70
May 22, 2026
Merged

Feature iceberg: hy-ib-2: Iceberg on S3 + commit hardening (hy-xq70)#17
philcunliffe merged 1 commit into
integration/icebergfrom
polecat/hy-xq70

Conversation

@philcunliffe

Copy link
Copy Markdown
Contributor

Summary

Feature iceberg: hy-ib-2: Iceberg on S3 + commit hardening

Implemented hy-ib-2: format-iceberg now commits through @hypaware/s3 with ifNoneMatch metadata writes, classifyAwsError->iceberg_blob_store_missing/iceberg_commit_conflict mapping, bucket/prefix/etag attributes on iceberg commit spans, concurrent-commit retry + create-race + stale/missing version-hint coverage, hermetic iceberg_export_s3_fixture smoke, env-gated iceberg_export_s3_roundtrip acceptance smoke. Drive-by fix to s3-export-batch.test.js (await activate). Rebased onto current origin/integration/iceberg tip (20ca78c). npm test 173/173 green; npm run lint + typecheck clean; iceberg_export_local_fs / iceberg_export_s3_fixture / iceberg_export_s3_roundtrip (skipped) / s3_sink_export_fixture / cli_bundled_plugins_activated smokes all green. Branch polecat/hy-xq70 pushed to origin off integration/iceberg.

Delivery

  • Issue: hy-xq70
  • Branch: polecat/hy-xq70
  • Integration target: integration/iceberg
  • Eventual target: integration/s3-sink

This PR is auto-merged into the integration branch once CI is green. Human
review for the full feature happens on the downstream PR
(integration/iceberg -> <final target>), which is opened separately when the feature
is ready to ship.

Wire @hypaware/format-iceberg to commit through @hypaware/s3 atomically:
- s3 BlobStore surfaces bucket/prefix as advisory properties for downstream
telemetry, and tags every thrown AWS SDK error with a stable errorKind via
classifyAwsError so the iceberg adapter no longer re-classifies SDK shapes.
- format-iceberg blob-io adapter maps the new error kinds:
blob_precondition_failed -> iceberg_commit_conflict (412)
s3_access_denied / s3_bucket_missing / s3_credentials_missing /
s3_region_mismatch / s3_config_invalid / s3_blob_store_unconfigured
-> iceberg_blob_store_missing
everything else -> iceberg_data_write_failed (writes)
iceberg_metadata_read_failed (reads)
An optional onWrite observer hands every successful metadata write
(key + etag + ifNoneMatch) back to the sink so the commit span can
surface S3's ETag without coupling the icebird writer surface to OTEL.
- iceberg.export_batch / table.load / snapshot.commit / table.create spans
now carry hyp_blob_store_kind, bucket, prefix, and (S3 only) etag on
successful commits.
- New traditional tests: full S3-error -> Iceberg-error mapping table,
concurrent-commit retry past a transient PreconditionFailed, initial
create race surfaces iceberg_commit_conflict (no retry), reader
recovers latest snapshot when version-hint.text is stale or missing,
onWrite observer contract.
- New hermetic smoke iceberg_export_s3_fixture: real plugin activation
over a fake S3 client, forces a v?+ ifNoneMatch collision and asserts
the retry path lands a snapshot, end-to-end icebird readback, and
bucket/prefix/etag attributes on the commit span.
- New acceptance smoke iceberg_export_s3_roundtrip (env-gated by
HYP_SMOKE_REAL_S3=1) for real S3 / MinIO round-trip with per-run
prefix isolation and best-effort cleanup.
Drive-by: fix racy await in test/plugins/s3-export-batch.test.js so the
sink registration completes before the helper asserts on it.
@philcunliffe
philcunliffe merged commit baca8b4 into integration/icebergMay 22, 2026
6 checks passed
@philcunliffe
philcunliffe deleted the polecat/hy-xq70 branch May 22, 2026 01:18
philcunliffe added a commit that referenced this pull request May 22, 2026
* chore: seed integration/iceberg for feature flow
* Feature iceberg: hy-tf-1: Formalize blob-store API + table-format writer kernel support (hy-cxl1) (#12)
* feat(kernel): formalize blob-store API + table-format writer support (hy-cxl1)
Promote hypaware.blob-store from a metadata-only marker into a real
object API (BlobStore: put/get/list/delete) and add hypaware.table-format
as a new capability for directory-layout + manifest writers (Iceberg).
Public types (collectivus-plugin-kernel-types.d.ts)
- Add BlobStore, PutObjectInput/Result, GetObjectInput/Result,
ListObjectsInput/ListObjectResult, DeleteObjectInput.
- Add TableFormatProvider, TableFormatCreateContext.
- Document the BlobStore capability VALUE change (object, not marker).
- Add optional `encoder` field to SinkInstanceConfig for table-format
writer inner-encoder pinning.
Kernel
- src/core/config/validate.js: add CAP_TABLE_FORMAT constant; allow
blob sink writers to provide either hypaware.encoder (legacy) or
hypaware.table-format (new); add `sink_writer_invalid`,
`sink_destination_invalid`, `sink_encoder_invalid` error_kinds;
legacy `sink_pair_incompatible` retained for encoder-writer flow.
- src/core/registry/sinks.js: new `kind: 'table-format'` variant on
`instantiate(...)` that resolves TableFormatProvider + BlobStore +
inner encoder and invokes provider.createSink. Emits
`sink.resolved` / `sink.register` logs with table-format-specific
attributes; supports tags intersect with the inner encoder.
Plugins
- @hypaware/local-fs: provide a full BlobStore implementation
(put/get/list/delete) backed by <HYP_HOME>/exports (or pin via
plugin config `exports_dir`). Sink contribution unchanged.
putObject honours ifNoneMatch='*' via O_EXCL; rejects keys that
escape the root.
- @hypaware/s3: provide a full BlobStore over the AWS SDK
(Put/Get/List/Delete). Honour ifNoneMatch via S3's IfNoneMatch
header (maps PreconditionFailed/HTTP 412 to
blob_precondition_failed). Activation reads plugin-level config;
when bucket is unset, provide a sentinel BlobStore that throws
s3_blob_store_unconfigured on use.
Tests + smokes
- test/core/blob-store.test.js: local-fs round-trip + in-memory
fixture exercising the full BlobStore contract.
- test/core/sinks-dispatch.test.js: kernel instantiate dispatch for
table-format sinks; new error_kind coverage on validateConfig.
- test/plugins/s3-blob-store.test.js: s3 BlobStore against a fake S3
client (put/get/list/delete/ifNoneMatch/escape-rejection).
- hypaware-core/smoke/flows/blobstore_api_local_fs.js: hermetic
smoke that activates @hypaware/local-fs, resolves the capability,
and exercises every BlobStore method directly (no sink tick).
99/99 tests pass; 30/30 smokes pass; lint + typecheck clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(local-fs): hold a FileHandle in getObject so unlink-after-get is safe (hy-cxl1)
The pre-fix getObject called createReadStream(path), which opens the
underlying file asynchronously inside the ReadStream's _construct. A
consumer that called getObject and then deleted (or discarded) the
key before consuming the body raced the lazy open: the unlink would
land before the open syscall, the stream would emit an unhandled
ENOENT error event, and Node would surface it as an uncaughtException.
Linux CI exposed this on the existing 'deleteObject removes the key
and is idempotent' test (test/core/blob-store.test.js:114); macOS
scheduling happened to win the race in local runs.
Fix: open the file via fs.open() before returning, then back the
result stream with the held FileHandle (handle.createReadStream).
The open is now settled at await time, so any ENOENT surfaces from
getObject itself (and we already map ENOENT → null). After the
handle is held, a subsequent unlink is benign on POSIX — the fd
stays valid until the stream closes.
Tests
- Drain the body in the existing deleteObject test so the held
FileHandle is released before the next iteration.
- New 'getObject body survives a concurrent unlink' regression that
uses a sync unlink between getObject and body-consume to force
the race window deterministically on every platform.
100/100 tests pass; lint + typecheck clean; blobstore_api_local_fs
smoke ok.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Feature iceberg: hy-ib-1: @hypaware/format-iceberg on local-fs (hy-42fk) (#16)
Add the @hypaware/format-iceberg plugin providing
`hypaware.table-format@1.0.0`. The TableFormatProvider builds an
Iceberg writer over any BlobStore destination (local-fs in V1) and an
inner parquet encoder, leveraging icebird's `fileCatalog` + `icebergAppend`
with a thin BlobStore-backed Resolver/Lister adapter so metadata + data
files round-trip via the destination's put/get/list/delete contract.
- Plugin tree under `hypaware-core/plugins-workspace/format-iceberg/`:
manifest, index, table-format provider, blob-io adapter, schema,
commit, state markers.
- BlobStore IO wraps the destination's BlobStore behind icebird's
Resolver/Lister API. Writer reuses hyparquet-writer's ByteWriter and
flushes via `putObject({ ifNoneMatch })`, surfacing 412 conflicts as
`iceberg_commit_conflict`.
- Schema mapper assigns stable field ids on first commit and
reconciles ids on append with `mergeFieldIdsFromTable`; incompatible
type changes, new required columns, column removals, and nullable→
required tightenings raise `iceberg_schema_incompatible`.
- State markers under `state/exported-batches/<sink>/<dataset>/<batch>.json`
let retries detect already-committed batches.
- Telemetry spans: `iceberg.export_batch`, `iceberg.table.load`,
`iceberg.table.create`, `iceberg.snapshot.commit`. Log row
`iceberg.activate` fires once per boot.
- Hermetic smoke `iceberg_export_local_fs` activates real
format-iceberg + format-parquet + local-fs, lands 5 rows in the
cache, drives one `exportBatch` through `kernel.sinks.instantiate`,
asserts that metadata + data files land on disk, that `icebird`
reads them back, and that the marker + telemetry are recorded.
- Added to `V1_BUNDLED_PLUGIN_ALLOWLIST` so it appears in
`hyp plugin list`. The `cli_bundled_plugins_activated` smoke is
updated to reflect the new 9-plugin bundled count
(`plugins_skipped=3`, format-iceberg in the skipped set).
- Traditional tests cover schema mapping, blob-io path/key rendering,
marker roundtrip, manifest validity, allowlist membership, provider
shape, and commit-conflict normalisation through a real local-fs
BlobStore.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Feature iceberg: hy-ib-2: Iceberg on S3 + commit hardening (hy-xq70) (#17)
Wire @hypaware/format-iceberg to commit through @hypaware/s3 atomically:
- s3 BlobStore surfaces bucket/prefix as advisory properties for downstream
telemetry, and tags every thrown AWS SDK error with a stable errorKind via
classifyAwsError so the iceberg adapter no longer re-classifies SDK shapes.
- format-iceberg blob-io adapter maps the new error kinds:
blob_precondition_failed -> iceberg_commit_conflict (412)
s3_access_denied / s3_bucket_missing / s3_credentials_missing /
s3_region_mismatch / s3_config_invalid / s3_blob_store_unconfigured
-> iceberg_blob_store_missing
everything else -> iceberg_data_write_failed (writes)
iceberg_metadata_read_failed (reads)
An optional onWrite observer hands every successful metadata write
(key + etag + ifNoneMatch) back to the sink so the commit span can
surface S3's ETag without coupling the icebird writer surface to OTEL.
- iceberg.export_batch / table.load / snapshot.commit / table.create spans
now carry hyp_blob_store_kind, bucket, prefix, and (S3 only) etag on
successful commits.
- New traditional tests: full S3-error -> Iceberg-error mapping table,
concurrent-commit retry past a transient PreconditionFailed, initial
create race surfaces iceberg_commit_conflict (no retry), reader
recovers latest snapshot when version-hint.text is stale or missing,
onWrite observer contract.
- New hermetic smoke iceberg_export_s3_fixture: real plugin activation
over a fake S3 client, forces a v?+ ifNoneMatch collision and asserts
the retry path lands a snapshot, end-to-end icebird readback, and
bucket/prefix/etag attributes on the commit span.
- New acceptance smoke iceberg_export_s3_roundtrip (env-gated by
HYP_SMOKE_REAL_S3=1) for real S3 / MinIO round-trip with per-run
prefix isolation and best-effort cleanup.
Drive-by: fix racy await in test/plugins/s3-export-batch.test.js so the
sink registration completes before the helper asserts on it.
* Fix dual-review findings on PR #8 (iceberg) (hy-cerp) (#19)
* fix(iceberg): scope probeTable misses to ENOENT (codex 1/3)
The blob-io reader raises `iceberg_metadata_read_failed` for two
structurally different conditions: a true miss (sets `code='ENOENT'`)
and a transient read failure (no `code`). The old `isProbeMissError`
matched the kind alone, so a flaky read drove the sink into a fresh
`create` path against an existing table instead of surfacing the real
error.
Restrict the miss classification to explicit not-found signals
(`code === 'ENOENT'`, "no metadata files" messages) and let any other
error propagate so the sink driver can retry the batch (hy-cerp).
* fix(iceberg): walk parent snapshots for marker supersedence (codex 2/3)
`markerSubsumedBySnapshot` previously returned true only when the
marker's snapshot equalled the current snapshot — the documented
"superseded" branch was a stub that always returned false. That made
the marker-skip logic incorrect for the very scenario it was meant to
cover: a retry of an already-committed batch after another commit
landed re-staged and re-appended the same rows.
Walk `metadata.snapshots` via `parent-snapshot-id` from the current
snapshot, looking for the marker's snapshot id. Treat the marker as
subsumed iff it sits on the ancestor chain. Bounded by the snapshot
array length so a malformed cyclic chain cannot hang the walk.
The public API now also accepts the full probe state (`{ metadata,
currentSnapshotId }`) so ancestry has the data it needs; a bare
snapshot string is still accepted for callers that don't have
metadata in hand (equality-only behavior preserved) (hy-cerp).
* fix(s3): scope listObjects to '<prefix>/' to stop sibling-namespace leak (codex 3/3)
S3's ListObjectsV2 treats `Prefix` as a bare string-prefix match.
The BlobStore was passing the configured `normalized` value without
a trailing slash, so `Prefix: 'hyp/exports'` would also enumerate
keys under `hyp/exports2/...`. `relativeFromFullKey` then surfaced
those siblings as if they belonged to this BlobStore, and cleanup /
delete loops could touch out-of-scope objects.
Force a trailing slash on the S3 Prefix whenever a non-empty
configured prefix is in play, and add a defense-in-depth scope guard
that refuses to yield any returned key that does not start with
`${normalized}/`. The no-prefix case (lists the entire bucket) is
preserved (hy-cerp).
---------
Co-authored-by: feature-launch <feature-launch@gas.city>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for freeto 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.

1 participant

@philcunliffe