Repository navigation
feat: add partial (batched) deployment support - #491
Open
LautaroPetaccio wants to merge 10 commits into
Open
LautaroPetaccio wants to merge 10 commits into
LautaroPetaccio wants to merge 10 commits into
Conversation
add a new `deployPartial` method that uploads an entity's content across several POST /entities requests instead of one, for entities too large for a single request. the regular `deploy` is unchanged. - splits content into size-bounded batches (first-fit-decreasing, default 50 MiB per request) and uploads them with `partial=true` - the entity file travels only in the first request (the server needs the manifest before the parallel batches); remaining batches upload through a bounded worker pool - skips content already on the server via /available-content, and resumes on network/5xx failures by re-querying and re-uploading only what is missing - returns the creationTimestamp once the server finalizes; surfaces PartialDeploymentValidationError on a 400 and PartialDeploymentNotSupportedError against a server without partial support
Test this pull request
|
when a batch's 200 finalizes the deployment the worker pool aborts the other in-flight batches; their abort errors were propagating and triggering a spurious resume cycle (an extra /available-content query, request and delay). swallow the abort once the session has already succeeded so the parallel path finalizes in a single session. adds a regression test for the parallel worker pool.
- detect a server without partial support via either server's missing-content message: worlds-content-server's "neither present in the storage" and catalyst's (via @dcl/content-validator) "was not uploaded or previously available". previously only the worlds phrasing matched, so a multi-batch deploy against an old catalyst threw a terminal validation error instead of PartialDeploymentNotSupportedError and callers never fell back to deploy() - honor the caller's AbortSignal: it was overwritten by the internal pool controller (or undefined) via mergeRequestOptions, so cancellation was ignored. each request now aborts when either the caller or the pool aborts - treat 429 (and 5xx / network) as retryable so a rate-limited staging request resumes instead of failing terminally; only other 4xx are terminal adds tests for old-catalyst detection, 429 resume, and caller-signal propagation.
the infrastructure in front of the content servers times out requests larger than ~200MB, so the default batch cap moves from 50 MiB to 100 MiB — half the requests for large scenes while keeping a ~2x safety margin (verified: the first request is entity JSON + the fullest batch, every other request is one batch, and a batch can only exceed the cap when a single file does). hardening from the follow-up review: - fail fast when a single file exceeds the request cap: files cannot be split across requests, so such an upload previously burned every resume attempt re-uploading hundreds of MB before failing with an opaque network error - abort promptly: check the caller signal at session entry (covers aborts landing during the resume backoff), carry it on the available-content query, and combine it with the pool controller once per session instead of per request (avoids accumulating abort listeners on large uploads) - exponential backoff between resume attempts, so 429 rate-limit retries have a chance of landing outside the limit window
- consume the body of 202 responses: with native fetch an unread response pins the undici socket and buffered bytes, and a large upload produces one 202 per non-finalizing batch - treat a finalizing 200 whose body fails to parse (empty body, proxy rewrite) as the success it is, with a fallback timestamp, instead of burning every resume attempt re-observing the already-deployed entity and reporting failure for a deployment that succeeded - cap the exponential resume backoff at 60s so a large maxResumeAttempts can't grow 2^attempt into effectively-hung multi-hour sleeps - detach the combined abort-signal listeners when a session's worker pool drains, so a long-lived caller signal reused across many uploads doesn't accumulate one listener per session
- 200 with an unparseable body: only fabricate a fallback timestamp on a genuine parse failure. If the body read was cancelled (a sibling worker finalized and aborted the pool, or the caller aborted), let the abort propagate — otherwise the loser of a concurrent finalize fabricates a client-clock timestamp that overwrites the winner's real server timestamp, and a caller cancellation mid-read is masked as success - worker pool: abort the controller and await all workers to settle in the finally. A retryable worker throw rejected Promise.all without aborting, so sibling requests kept uploading uncancelable while the resume loop started a new overlapping session - drain 202 bodies via text() instead of json() (drain without the wasted parse) - resume backoff cap never drops below the caller's configured resumeDelay, so a caller that set a large base delay to respect an upstream limiter keeps it adds a regression test for the concurrent-finalize / aborted-loser timestamp.
… fail fast on undeliverable content
- deliver cancellation via an abortController, not a signal: the default
@dcl/fetch-component overwrites a caller-supplied signal with its own
internal controller and only honors an abortController option, so an
in-flight request (caller abort, or the pool's first-200-wins abort) was
never actually cancelled on the wire — only the next batch was suppressed
- a concurrent win now takes precedence over a sibling worker's retryable
failure: previously a retryable throw rejected Promise.all and propagated
before the win was returned, discarding a completed deployment and
triggering a spurious resume
- honor a server Retry-After header as the resume backoff floor (capped), so
a redeploy inside a rate-limit window waits the window out instead of
exhausting the ~7s exponential backoff and failing terminally
- only reject an oversized single file when it actually needs uploading; an
already-stored oversized file (shared with a prior deploy) needs zero bytes
and must not block the deployment (also fixes the doc contradiction)
- read the 202 { missing } body and fail fast with a terminal, diagnostic
error when a referenced hash is absent from the provided files, instead of
looping through full re-upload sessions to the same dead end
Aligns deployPartial with ADR-325: /available-content only plans the first round, and when every batch is accepted without publishing, the client sends the hashes from the latest 202 missing list (or an empty batch when nothing is listed) instead of re-querying availability. Retries 408, 409, 429 and 5xx with backoff floored at Retry-After and gives up with a DeploymentError after maxResumeAttempts rounds without progress. The public API is unchanged.
deployPartial now posts every batch to /entities?partial=true while keeping the partial=true form field, so the server can tell a batch from a regular deployment before reading the body (a Catalyst whose daily quota for the source is spent otherwise answers 429 before parsing).
…nt waits Quota rejections now answer 429 with a Retry-After that can be hours away (an upload slot frees only when older uploads expire). The resume loop capped its wait at 60 s, so it retried before Retry-After, which cannot succeed, and failed after its attempts. A Retry-After longer than the backoff ceiling now ends the call with PartialDeploymentRetryLaterError, carrying the server's reason and when to retry.
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
1 participant
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.
What
Adds
deployPartial, which deploys an entity across severalPOST /entitiesrequests following decentraland/adr#325, for entities too large for a single request.deployis unchanged.How
/available-content, then packs the missing files into batches (first-fit-decreasing, 100 MiB per request by default viamaxBatchSizeBytes). The first request carries the entity file; the rest go through a bounded worker pool (concurrency, default 2).202missinglist rather than re-querying availability; the first200wins.408,409,429(never sooner thanRetry-After),5xxand network errors with exponential backoff. Other 4xx responses, including quota400s, are terminal. It gives up aftermaxResumeAttemptsrounds without progress.PartialDeploymentNotSupportedError.API
New exports:
PartialDeploymentOptions,PartialDeploymentProgress,PartialDeploymentResult,DeploymentError,PartialDeploymentValidationError,PartialDeploymentNotSupportedError,splitIntoBatches,DEFAULT_MAX_BATCH_SIZE_BYTES.How to test
yarn test:test/unit/batching.spec.tscovers the batcher andtest/PartialDeployment.spec.tsthe flow (missing-list rounds, retries andRetry-After, terminal400s, stalls, old-server detection, progress).Additive API only (semver minor). The
202type stays local until decentraland/catalyst-api-specs#171 is released. The SDK integration is decentraland/js-sdk-toolchain#1635.