Skip to content

fix(geotiff): revert ConcurrencyLimiter to a chunkd SourceMiddleware#572

Merged
kylebarron merged 1 commit into
mainfrom
kyle/565-limiter-middleware
May 26, 2026
Merged

fix(geotiff): revert ConcurrencyLimiter to a chunkd SourceMiddleware#572
kylebarron merged 1 commit into
mainfrom
kyle/565-limiter-middleware

Conversation

@kylebarron
Copy link
Copy Markdown
Member

Written by Claude on behalf of @kylebarron

Closes #565.

Summary

chunkd #1697 shipped (@chunkd/source@11.4.1, @chunkd/middleware@11.3.1), so SourceView now forwards the request signal to middleware (and SourceChunk forwards it on multi-chunk sub-requests). That removes the reason for the LimitedSource source-wrapper workaround introduced in #557: a middleware can now observe an abort, so a request whose caller aborts while queued for a slot is dropped before any network I/O.

This reverts the limiter to the originally-intended LimiterMiddleware (a chunkd SourceMiddleware):

  • limiter.tsLimitedSourceLimiterMiddleware.
  • geotiff.ts — compose the limiter into the header SourceView after SourceChunk/SourceCache (cache hits short-circuit and never burn a slot), plus a tiny SourceView wrapping the data source. getPriority threading preserved.
  • deps — bump @chunkd/source / @chunkd/source-http to ^11.4.1 and @chunkd/middleware to ^11.3.1 (the middleware approach hard-requires the fix, so the floors move, not just the lockfile).
  • spec — drop the temporary "implementation note" in dev-docs/specs/2026-05-19-concurrency-limiter-design.md.

Internal-only: the public API (GeoTIFF.fromUrl options, ConcurrencyLimiter, Priority, PerOriginSemaphore) is unchanged either way.

Not a blind revert

Two changes landed on main after the original workaround, so a wholesale git revert would have dropped them. Both are preserved:

  • the normalizePriority (NaN) helper in limiter.ts, and
  • the integration test that aborts a queued header read through the real fromUrl → SourceView → limiter path.

That integration test is the acceptance test for this change — it is unchanged and still passes, which is the end-to-end proof that the signal now reaches the middleware.

Verification

  • pnpm --filter @developmentseed/geotiff test — 140 passed (16 files)
  • pnpm --filter @developmentseed/geotiff typecheck — clean
  • pnpm check (biome) — clean

🤖 Generated with Claude Code

chunkd#1697 shipped (@chunkd/source 11.4.1, @chunkd/middleware 11.3.1),
so SourceView now forwards the request signal to middleware and
SourceChunk forwards it on multi-chunk sub-requests. That removes the
reason for the LimitedSource source-wrapper workaround (#557): a
middleware can now observe an abort, so a request whose caller aborts
while queued for a slot is dropped before any network I/O.

Replace the internal LimitedSource with LimiterMiddleware again,
composed into the header SourceView *after* SourceChunk/SourceCache (so
cache hits short-circuit and never burn a slot) and into a tiny
SourceView wrapping the data source. Bump the @chunkd/source,
@chunkd/source-http, and @chunkd/middleware floors to the fixed
releases.

Internal-only: GeoTIFF.fromUrl options, ConcurrencyLimiter, Priority,
and PerOriginSemaphore are unchanged. The integration test that aborts a
queued header read through the real fromUrl -> SourceView -> limiter
path is unchanged and still passes, proving the signal now reaches the
middleware end to end.

Closes #565.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added the fix label May 26, 2026
Copy link
Copy Markdown
Member Author

@kylebarron kylebarron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@kylebarron kylebarron merged commit 8a329b3 into main May 26, 2026
3 checks passed
@kylebarron kylebarron deleted the kyle/565-limiter-middleware branch May 26, 2026 20:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Revert ConcurrencyLimiter to a chunkd SourceMiddleware once chunkd forwards the abort signal

1 participant