feat(firehose): honor S3 size and interval buffering hints

Merged
CA
3d ago
+8443-647

Summary

Final increment of the Firehose S3 delivery series tracked in #2708. Stacked on GZIP (#2745) and Xerial Snappy (#2746). Until those merge, this PR's diff against main also contains their commits; the buffering changes are the commits from feat(firehose): honor S3 size and interval buffering hints onward.

  • Honor decoded-byte size and first-arrival interval buffering hints across producer calls, with effective defaults of 5 MiB / 300 seconds and paired validation of 1–128 MiB / 0–900 seconds (fixes the old 0 or 60-900 check, per #2708 item 2).
  • Share one engine across public Firehose APIs, CloudWatch Logs subscriptions, and SES event destinations. Admit whole API batches without splitting records; empty blobs remain valid without creating empty objects unless delimiters are enabled. Exact byte concatenation is unchanged.
  • Capture destination/configuration/key at detachment, preserve FIFO retry ordering with 1–30 s exponential backoff, and guard commits against delete/reset/recreate races using runtime stream identities.
  • As on AWS for DirectPut streams, stop retrying a batch 24 hours after its oldest record arrived and discard it with a warning; this also bounds memory retained during prolonged destination failure.
  • Preserve upstream S3 versioning, default Object Lock retention, ownership, durability, default CRC64NVME checksums of the encoded bytes, and bucket-default encryption (including KMS), replication, and notifications in the guarded writer. The producer guard is taken before the S3 write lock and both are released before notification callbacks (documented and regression-tested).
  • Narrow, Firehose-owned lifecycle: one worker started by the server, reset discards pending buffers, and graceful shutdown closes admission after the HTTP server stops and performs one final drain with undelivered counts (#2708 lifecycle guidance).
  • Reject record admission for AWS-valid but unimplemented settings — ZIP/HADOOP_SNAPPY, processing other than AppendDelimiterToRecord alone, format conversion, dynamic partitioning, prefix expressions, non-UTC time zones, explicit destination KMS, S3 backup, and non-S3 destinations — with HTTP 400 InvalidArgumentException naming the setting (#2708 item 3). Create/UpdateDestination and CloudFormation keep accepting every modeled value; an unsupported update pauses already accepted active data until the configuration is supported again. Admission, scheduling, and the final drain share one capability check.
  • Enforce the model's S3 destination length limits (Prefix/ErrorOutputPrefix ≤ 1,024, CustomTimeZone ≤ 50, RoleARN 1–512, BucketARN 1–2,048) on create/update and CloudFormation, so unbounded values cannot be copied into every buffered batch and object key. ARN and time-zone patterns are not enforced. Values such as an empty RoleARN, accepted on main, now return InvalidArgumentException.
  • CloudFormation coerces quoted BufferingHints/RetryOptions integers and Enabled flags (as produced by Ref parameters and AWS's own examples), including inside S3BackupConfiguration.

Scope and compatibility

Acknowledgment now means accepted into process-local memory, not immediately visible in S3; poll S3 for delivery. A malformed bucket ARN, which main acknowledges and only logs, now fails record admission with InvalidArgumentException (Create/UpdateDestination still accept it). Pending buffers/retries are not durable across a crash; healthy graceful shutdown drains them. Within the 24-hour retention window there is no memory-budget backpressure.

Delivery does not assume RoleARN or evaluate IAM/bucket policies (unchanged from main; now documented), and Logs/SES may target another account's stream as before. A stream stored without an S3 destination (an S3 configuration missing RoleARN/BucketARN, which AWS rejects) still acknowledges records without writing them, as on main; documented rather than changed here.

The Logs-to-Firehose raw GZIP fix already landed in #2720. The test(logs) commit only adds multi-member payload and buffered-delivery coverage.

Review

Before publication the stack had an independent code/design review (engine concurrency and lifecycle; S3/CloudFormation/Logs/SES integration, tests, and docs) and a STRIDE security review. No Critical findings. Addressed here: stale docs, duplicated readiness checks that could let the drain spin, unbounded Prefix × retained batches, unlimited retries vs AWS's 24 hours, per-scan configuration copies under global locks, caller-controlled text (object keys, errors, stream names, ARNs) in log lines, now Debug-escaped, the CloudFormation quoted-hint regression, an unused S3 helper, and weak negative E2E waits (now proven against healthy control streams). Not changed (pre-existing, follow-ups): SES's delivery log can list records Firehose refused because the internal FirehoseDelivery hook returns no result; Firehose is not IAM-enforced.

Test plan

Verified on 6945abc71 (stacked on Snappy c5907ef3a, upstream 774c772d6):

  • 1,826 focused unit tests: CloudFormation (638), Firehose (161), Logs (364), S3 (663); 27 server reset tests.
  • 212 rebuilt-server SDK tests across Firehose buffering/codecs/persistence/validation, Logs, SES and SES→Firehose events, S3 checksums/persistence/SSE-KMS/write side effects; one Athena regression; 107 Firehose/SES/S3 conformance tests.
  • Workspace all-target Clippy with warnings denied, formatting, documentation counts, diff checks, and the E2E nextest partition check (2,673 tests, each covered exactly once).
  • Every commit in the stack compiles on its own (cargo check --workspace --all-targets), so the history bisects.
  • New behavior was observed failing before its fix (24-hour retention, destination string limits, log escaping, notification lock release).
  • The unrestricted cargo test --workspace (every E2E test, including container runtimes) was not rerun on this revision. On the GZIP predecessor it stopped on the unchanged, upstream-reproduced API Gateway fixed-port test (expected 502, got 504 at closed 127.0.0.1:54321 in this WSL environment). No exclusions were added, and the workspace gate is not claimed as passing.

Series context

Refs #2708; completes the four-increment plan. Branch feat/firehose-s3-buffering is stacked on feat/firehose-snappy. As each predecessor merges, I will rebase onto main so only the buffering commits remain, and revalidate.

Summary by cubic

Completes the Firehose S3 delivery series by adding buffered delivery that honors decoded-byte size and first-arrival interval hints from PutRecord/PutRecordBatch, CloudWatch Logs subscriptions, and SES event destinations, all through one shared engine. Delivery now retries up to 24 hours, rejects unsupported settings with HTTP 400, and enforces destination string limits.

New Features:

  • Buffering hints default to 5 MiB / 300 s and validate to 1–128 MiB / 0–900 s.
  • One engine serves the public API, Logs, and SES; batches are not split across calls.
  • Unsupported settings (e.g., ZIP, processing, dynamic partitioning, non-S3 destinations) reject record admission with InvalidArgumentException.
  • Failed batches retry in FIFO order every 5 seconds and stop after the oldest record exceeds 24 hours.
  • Destination SSE-KMS encryption is carried through buffered delivery and survives UpdateDestination.

Migration:

  • Acknowledgment now means accepted into process-local memory; poll S3 for delivery.
  • A malformed bucket ARN fails record admission (previously only logged).
  • Pending buffers are not durable across a crash; graceful shutdown drains them once.
  • CloudFormation coerces quoted BufferingHints/RetryOptions integers and fails the resource on unparseable or wrong-typed scalars.