perf: reduce S3 read latency (connection pooling, prewarm, GridIndex)#7
Merged
Conversation
Latency is dominated by per-request round trips. These changes cut the
cost per round trip and let callers avoid recurring ones.
Item 1 — S3 connection pooling. S3StoreOptions gains maxSockets (default
128, keep-alive on), keepAlive, connectionTimeoutMs and a requestHandler
escape hatch. createClient builds a NodeHttpHandler over a keep-alive
https.Agent so many-chunk reads aren't capped at the SDK's ~50-socket
default; degrades gracefully if @smithy/node-http-handler is unavailable.
Item 3 — S3 connection prewarming. S3Store.prewarm() opens a pooled TLS
connection ahead of the first read (best-effort); warmOnCreate triggers it
on construction.
Item 2 — GridIndex (@i4sea/zarr-node/spatial). Nearest (lat,lon)->(i,j) on a
2D curvilinear grid. fromCoordinates/fromGroup build it once (queries are
pure CPU); loadCached(group, { cache }) adds L1 (process) + L2 (shared
Cache/Redis) so only the first pod pays the coordinate fetch, keyed per
domain (source_model/experiment/grid_id + shape) with gridKey override and
optional verifyGrid fingerprint. toBytes/fromBytes give a compact snapshot.
Adds unit tests (grid-index, s3-store), the /spatial subpath export, the
@smithy/node-http-handler optional dependency, README + CHANGELOG, a local
flow benchmark (local/s3/s3-ab/s3-serving modes), and the design plan doc.
brandao-andre
approved these changes
Jun 12, 2026
There was a problem hiding this comment.
Pull request overview
This PR reduces S3 read latency by (1) enabling connection pooling/keep-alive tuning in S3Store, (2) adding best-effort connection prewarming, and (3) introducing a new @i4sea/zarr-node/spatial subpath with GridIndex for fast (lat,lon)→(i,j) lookups with optional Redis-backed caching to avoid repeatedly fetching static grid coordinates.
Changes:
- Add configurable S3 HTTP connection pooling (
maxSockets, keep-alive, connection timeout) plus an escape-hatchrequestHandler. - Add
S3Store.prewarm()andwarmOnCreatefor eager TLS/socket warm-up. - Add
GridIndex(new/spatialexport) with binary snapshotting and layered caching (L1 in-process + optional L2Cache/Redis), plus documentation/tests/benchmark updates.
Reviewed changes
Copilot reviewed 11 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/s3-store.test.ts | Adds unit tests for request handler “escape hatch” and prewarm() never rejecting. |
| tests/unit/grid-index.test.ts | Adds unit tests for GridIndex queries, validation, snapshots, and cached loading behavior. |
| src/store/store.ts | Extends S3StoreOptions with pooling/prewarm/request handler options. |
| src/store/s3.ts | Implements pooling handler construction, prewarm(), and warmOnCreate. |
| src/spatial/index.ts | New /spatial entrypoint exporting GridIndex types and class. |
| src/spatial/grid-index.ts | Implements GridIndex construction, nearest-neighbor queries, snapshotting, and cache-key derivation. |
| README.md | Documents S3 pooling/prewarm usage and introduces GridIndex usage patterns. |
| package.json | Adds ./spatial export and makes @smithy/node-http-handler an optional dependency. |
| package-lock.json | Reflects version bump and optional dependency resolution changes. |
| examples/benchmark-local-flow.ts | Adds an end-to-end benchmark example covering caches and S3 before/after modes. |
| docs/plano-reducao-latencia-s3.md | Adds the detailed latency-reduction plan/design rationale. |
| CHANGELOG.md | Adds Unreleased notes describing the new pooling/prewarm/GridIndex features. |
| .gitignore | Ignores .bench/ benchmark dataset directory. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- GridIndex grid key now includes lon chunks/dtype (not just lat), so two domains differing only in lon chunking/dtype no longer collide. - An explicit gridKey that sanitizes to empty (all non-ASCII/whitespace) falls back to a hash of the raw value instead of collapsing to "gridindex:" and colliding across datasets. - buildRequestHandler now also configures an http.Agent, so maxSockets/ keep-alive apply to http:// endpoints (MinIO/LocalStack), not only https.
Collaborator
Author
|
Os 3 apontamentos do Copilot foram válidos e tratados em 0891b92:
Suíte completa verde (327 testes, lint, typecheck, build, validate-exports). |
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.
Contexto
A latência de leitura no S3 é dominada por round-trips por requisição (~127 ms RTT Brasil→us-east-1). Estas mudanças reduzem o custo por round-trip e permitem evitar os recorrentes. Implementa o plano em
docs/plano-reducao-latencia-s3.md.Mudanças
Item 1 — Pooling de conexão (
S3Store)S3StoreOptions:maxSockets(default 128, keep-alive on),keepAlive,connectionTimeoutMs,requestHandler(escape hatch).createClientmontaNodeHttpHandlersobre umhttps.Agentkeep-alive — reads de muitos chunks deixam de ser limitados ao default de ~50 sockets do SDK. Degrada graciosamente se@smithy/node-http-handlerfaltar.concurrency: 50) inalterado — mudança puramente aditiva.Item 3 — Prewarming (
S3Store.prewarm())warmOnCreatedispara na construção.Item 2 —
GridIndex(@i4sea/zarr-node/spatial)(lat,lon)→(i,j)numa grade 2D curvilínea.fromCoordinates/fromGroupmontam 1× (queries viram CPU puro).loadCached(group, { cache })com L1 (processo) + L2 (Redis) — só o primeiro pod paga o fetch das coordenadas. Chave por domínio (source_model/experiment/grid_id+ shape), com overridegridKeyeverifyGridopcional.toBytes/fromBytespara snapshot binário.Validação (S3 real, padrão de serviço)
Ganhos de pooling/concurrency são maximizados in-region — recomenda-se re-rodar
MODE=s3-abde um pod em us-east-1 antes de afinar oconcurrencydefault.Testes
npm test(326 passam, +17 novos: grid-index, s3-store), lint, format, typecheck, build CJS+ESM, validate-exports (/spatial), interop ESM/CJS — todos verdes.examples/benchmark-local-flow.ts(modoslocal/s3/s3-ab/s3-serving).🤖 Generated with Claude Code