feat(lib-transfer-manager): add file based download api and worker thread based download. by smilkuri · Pull Request #8259 · aws/aws-sdk-js-v3

smilkuri · 2026-08-10T16:05:29Z

Issue

Internal JS-7054

Description

Adds downloadToFile API which takes a destination file path and downloads an S3 object to that location. It supports both PART based and RANGE based strategy. It uses a temp file with atomic rename, the destination file only appears once the download complete successfully. On failure or abort, the temp file is cleaned up.
Adds downloadByPartWithWorkers and downloadByRangeWithWorkers for the download() API. When workerThreadCount > 1, download requests are routed through these methods based on the configured multipart strategy. Internally, Part 1 is fetched on the main thread to discover object metadata(PartsCount, ETag, total size). Remaining parts/range requests are dispatched to worker threads via WorkerHttpHandler, which assembles response bytes into dedicated ArrayBuffers and transfers ownership back to the main thread (zero-copy). An OrderedPartQueue reorders parts that arrive out-of-order, while a Readable stream pulls from the queue sequentially and is passed to joinStreams to deliver a single ordered body to the caller.

Testing

How was this change tested?

$ yarn test
 ✓ src/submodules/transfer-manager/join-streams.spec.ts (10 tests) 578ms
     ✓ should handle backpressure when consumer is slow  554ms
 ✓ src/submodules/transfer-manager/S3TransferManager.spec.ts (101 tests) 377ms

 Test Files  2 passed (2)
      Tests  111 passed (111)
   Start at  14:39:19
   Duration  952ms (transform 418ms, setup 0ms, import 663ms, tests 954ms, environment 0ms)

  $ yarn test:e2e
   Test Files  1 passed (1)
     Tests  39 passed (39)
  Start at  14:40:53
  Duration  37.66s (transform 412ms, setup 0ms, import 616ms, tests 36.90s, environment 0ms)

Checklist

If the PR is a feature, add integration tests (*.integ.spec.ts) or E2E tests.
- It's not a feature.
My E2E tests are resilient to concurrent i/o.
- I didn't write any E2E tests.
I added access level annotations e.g. @public, @internal tags and enabled doc generation on the package. Remember that access level annotations go below the description, not above.
- I didn't add any public functions.
Streams - how do they work?? My WebStream readers/locks are properly lifecycled. Node.js stream backpressure is handled. Error handling.
- No streams here.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

smilkuri · 2026-08-14T10:25:33Z

+    // the other download's temp file.
+    const fd = await open(tempFilePath, "wx");
+    try {
+      await fd.truncate(totalSize);


Specification says not to pre-validate available space, we are just setting the logical file length here so it should be okay ig.

kuhe · 2026-08-14T14:37:14Z

+ *
+ * @internal
+ */
+


this should not be in tsdoc format if it's module level, use /* */ or //.

or attach it to the main export symbol

kuhe · 2026-08-14T14:38:28Z

+  byteLength: number;
+}
+
+export class OrderedPartQueue {


what does this do? ~2 sentences as class description

It reorders the parts and deliver them sequentially. Will update it.

kuhe · 2026-08-14T15:05:42Z

+  }
+
+  /**
+   * Returns whether an error has been set.


I can tell from the code this is what is happening. Who is the caller and why do they need to know?

Dispatch loop calls it to stop launching new part requests in case of failure.

kuhe · 2026-08-14T15:06:37Z

-      signal.addEventListener("abort", removeListenerAfterAbort, { once: true });
+      signal.addEventListener("abort", removeListenerAfterAbort, {
+        once: true,
+      });


why is this reformatted?

Formatted when ran make format

kuhe · 2026-08-14T15:08:03Z

+            streams,
+            requests,
+            metadata,
+            checksumValidationEnabled


are these parameters identical? prebuild in a variable

kuhe · 2026-08-14T15:09:30Z

+   * When false, the existing file is overwritten.
+   * Defaults to false.
+   */
+  failIfExists?: boolean;


this parameter is usually called overwrite: boolean = true. Spec driven?

That's according to the specification.

kuhe · 2026-08-14T15:10:59Z

+      metadata.ChecksumCRC32 = undefined;
+      metadata.ChecksumCRC32C = undefined;
+      metadata.ChecksumSHA1 = undefined;
+      metadata.ChecksumSHA256 = undefined;


why do this? there are other checksums now too

The reason for setting them to undefined is s3 returns the checksum value for the whole multipart object (a composite of per-part checksums), not for the byte range being returned. Since we reassemble parts into a single stream, the per-part checksum from the initial response doesn't match to the joined output. But I agree we have to add that new checksums too.

kuhe · 2026-08-14T15:12:14Z

+    };
+
+    const releaseSlot = (): void => {
+      inFlight--;


should this throw if already at 0?

or, the decrement should be in the conditional with the work being done

Yes will add a check here

kuhe · 2026-08-14T15:13:19Z

+          .catch((error) => {
+            queue.setError(error);
+            abortController.abort();
+            releaseSlot();


move releaseslot to finally?

kuhe · 2026-08-14T15:15:07Z

+    // Create a Readable stream that pulls parts sequentially from the ordered queue.
+    // Each chunk is a zero-copy Buffer view into the transferred ArrayBuffer.
+    const transferStream = new Readable({
+      read() {


is this read method fine to call if it's still working?

No we shouldn't. The queue only has one waitingConsumer slot, so a second dequeue call would overwrite the first one. Should have some kind of reading check here.

kuhe · 2026-08-14T15:17:12Z

+      ETag: headResponse.ETag,
+      LastModified: headResponse.LastModified,
+      $metadata: headResponse.$metadata,
+    });


do you have full ownership of the metadata object at this point where it is mutating?

Yes, we are creating metadata as empty object initially in download() right, nobody reads it before it reaches the download method, it is populated here. Instead should we return as a new object from the download method instead of mutating the parameter?

kuhe · 2026-08-14T15:18:10Z

+  /**
+   * Optional CRC algorithm for inline checksum computation.
+   */
+  checksumAlgorithm?: string; // "CRC32" | "CRC32C" | "CRC64NVME"


why only 3 options here?

Initially didn't add the new XXHASH algorithms because the SDK doesn't have local implementations. Now updated it to just skip validation for those.

kuhe · 2026-08-14T15:19:37Z

+  /**
+   * Optional CRC algorithm for inline checksum validation against S3 response headers.
+   */
+  checksumAlgorithm?: string;


why not the ChecksumAlgorithm enum type from S3?

kuhe · 2026-08-14T15:20:15Z

 import { openSync, readSync, closeSync } from "node:fs";
 import type { LookupOptions } from "node:dns";
+import type { Checksum } from "@smithy/types";
+import { Crc32cJs, Crc64NvmeJs } from "@aws-sdk/checksums/crc";


why import the JS implementations of the checksums and not the default?

kuhe · 2026-08-14T15:20:59Z

+/**
+ * Thin wrapper around node:zlib crc32 to conform to the Checksum interface.
+ */
+class NodeCrc32 implements Checksum {


why is there a new checksum implementation here?

…downlaodToFile

…using worker threads

…events

smilkuri force-pushed the tm-download branch from 088a912 to 5d7abd1 Compare August 12, 2026 23:20

smilkuri marked this pull request as ready for review August 12, 2026 23:20

smilkuri requested a review from a team as a code owner August 12, 2026 23:20

smilkuri force-pushed the tm-download branch 2 times, most recently from 3aaff3f to 5ca70ed Compare August 13, 2026 00:08

smilkuri marked this pull request as draft August 13, 2026 12:16

smilkuri force-pushed the tm-download branch from 565ee85 to b4c36ec Compare August 14, 2026 13:19

smilkuri marked this pull request as ready for review August 14, 2026 13:20

smilkuri changed the title ~~WIP feat(lib-transfer-manager): add file based download api for part get~~ feat(lib-transfer-manager): add file based download api for part get Aug 14, 2026

smilkuri changed the title ~~feat(lib-transfer-manager): add file based download api for part get~~ feat(lib-transfer-manager): add file based download api and worker thread based download. Aug 14, 2026

smilkuri force-pushed the tm-download branch 2 times, most recently from f1faadb to 139a6ba Compare August 14, 2026 13:46

smilkuri commented Aug 14, 2026

View reviewed changes

kuhe reviewed Aug 14, 2026

View reviewed changes

smilkuri added 2 commits August 14, 2026 19:30

feat(lib-transfer-manager): add file based download api for part get

c87ebfc

feat(lib-transfer-manager): add support for range based strategy for …

73ab7f4

…downlaodToFile

smilkuri added 3 commits August 14, 2026 19:31

feat(lib-transfer-manager): add stream based download implementation …

dcedca7

…using worker threads

feat(lib-transfer-manager): add tests for downlaodToFile

0bb2518

fix(transfer-manager): add missing download validations and progress …

846b72a

…events

smilkuri force-pushed the tm-download branch 2 times, most recently from 78dc67b to 6880870 Compare August 14, 2026 19:36

fix(lib-transfer-manager): addressed comments from rev-1

3f95e5e

smilkuri force-pushed the tm-download branch from 6880870 to 3f95e5e Compare August 14, 2026 20:37

Uh oh!

Conversation

smilkuri commented Aug 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Issue

Description

Testing

Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

smilkuri commented Aug 10, 2026 •

edited

Loading