Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

S3 upload never finishes for HeapSnapshotStream #5861

Open
3 tasks done
ultsi-wolt opened this issue Mar 7, 2024 · 1 comment
Open
3 tasks done

S3 upload never finishes for HeapSnapshotStream #5861

ultsi-wolt opened this issue Mar 7, 2024 · 1 comment
Assignees
Labels
bug This issue is a bug. p3 This is a minor priority issue queued This issues is on the AWS team's backlog workaround-available This issue has a work around available.

Comments

@ultsi-wolt
Copy link

ultsi-wolt commented Mar 7, 2024

Checkboxes for prior research

Describe the bug

HeapSnapshotStream kind of Readables never finish upload process to S3.

SDK version number

"@aws-sdk/client-s3": "^3.521.0", "@aws-sdk/lib-storage": "^3.525.1",

Which JavaScript Runtime is this issue in?

Node.js

Details of the browser/Node.js/ReactNative version

node v18.18.0

Reproduction Steps

import { Upload } from '@aws-sdk/lib-storage';
import { S3Client } from '@aws-sdk/client-s3';
import { Readable } from 'stream';

export const uploadStreamToS3Async = async (
  stream: Readable,
  bucketName: string,
  keyName: string,
  acl: ACL,
) => {
  const uploadToS3 = new Upload({
    client: new S3Client({ region: process.env.AWS_REGION || 'eu-west-1' }),
    params: {
      Bucket: bucketName,
      Key: keyName,
      Body: stream,
      ACL: acl,
    },
  });
  return uploadToS3.done();
};

// this method works fine:

const uploadFile = async(fileStream: Readable) => {
  const passThroughStream = new PassThrough();
  fileStream.pipe(passThroughStream);

  try {
    await uploadStreamToS3AsyncV2(
      passThroughStream,
      'bucketname',
      'keyname',
      'public-read',
    );
  } catch (e: any) {
    throw new FileUploadError(`Could not write image in S3: ${e.message}`);
  }
}

// this method never finishes

const saveHeapDumpToAWS = async () => {
  return uploadStreamToS3Async(
    v8.getHeapSnapshot(),
    'bucketname',
    `heap-dumps/${Date.now()}.heapsnapshot`,
    'private',
  );
};

When I add a log on httpUploadProgress event to uploadToS3 in uploadStreamToS3Async method (below), it logs all 12 different parts being uploaded in 4 queues, but await uploadToS3.done() never gets resolved.

uploadToS3.on('httpUploadProgress', (progress) => {
  console.log(progress);
});

Observed Behavior

The reason why I consider this an issue is that inputting HeapSnapshotStream straight into upload body worked with SDK v2 before. The uploadFile solution we didn't need to change, but for heap snapshot we are now converting it to a buffer before uploading. So something is wrong in how HeapSnapshotStream kind of Readables get processed by the S3 upload library.

Expected Behavior

It should work like it did in AWS SDK v2 so that the migration would be easy. Now I consider this as a bug that we had to figure out a different solution for.

Possible Solution

No response

Additional Information/Context

No response

@ultsi-wolt ultsi-wolt added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Mar 7, 2024
@RanVaknin RanVaknin self-assigned this Mar 7, 2024
@RanVaknin
Copy link
Contributor

RanVaknin commented Mar 7, 2024

Hi @ultsi-wolt ,

Thanks for reaching out.
You can normalize the snapshot stream by piping it through a passthrough:

import { S3Client } from "@aws-sdk/client-s3";
import { Upload } from "@aws-sdk/lib-storage";
import { getHeapSnapshot } from "v8";
import { PassThrough } from "stream";

async function uploadHeapSnapshot() {
  try {
    const client = new S3Client({ region: "us-east-1" });
    const snapshotStream = getHeapSnapshot();
    const passThrough = new PassThrough();
    snapshotStream.pipe(passThrough);
  
    const upload = new Upload({
      client: client,
      params: {
        Bucket: "testbucket-REDACTED",
        Key: "foo-snapshot",
        Body: passThrough,
      },
    });
  
    upload.on("httpUploadProgress", (progress) => {
      console.log(progress);
    });
  
    await upload.done();
    console.log("success");
  } catch (error) {
    console.log(error)
  }

}

uploadHeapSnapshot()

/*
{
  loaded: 3764838,
  total: undefined,
  part: 2,
  Key: 'foo-snapshot',
  Bucket: 'testbucket-REDACTED'
}
{
  loaded: 9007718,
  total: undefined,
  part: 1,
  Key: 'foo-snapshot',
  Bucket: 'testbucket-REDACTED'
}
success
*/

As you can see the upload is successful.

It should work like it did in AWS SDK v2 so that the migration would be easy. Now I consider this as a bug that we had to figure out a different solution for.

I just want to highlight that using two major versions of the SDK inherently means that there would be major changes that do not necessarily mean migrating would be a "plug and play" scenario. Unlike v2 that consumes and checks the stream directly, v3 uses many layers of middleware to process and transform the payloads.
I don't have visibility to what component changed between the two major versions that causes this to fail only with v8.getHeapSnapshot() but works with all other streams.

Thanks,
Ran~

@RanVaknin RanVaknin added workaround-available This issue has a work around available. needs-review This issue/pr needs review from an internal developer. p3 This is a minor priority issue and removed needs-triage This issue or PR still needs to be triaged. labels Mar 7, 2024
@trivikr trivikr added needs-review This issue/pr needs review from an internal developer. queued This issues is on the AWS team's backlog and removed needs-review This issue/pr needs review from an internal developer. labels Mar 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug. p3 This is a minor priority issue queued This issues is on the AWS team's backlog workaround-available This issue has a work around available.
Projects
None yet
Development

No branches or pull requests

3 participants