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

Add error when chunkSize is greater than the strict memory limit of the CacheLimiter. #2228

Merged

Conversation

mkrautz
Copy link
Contributor

@mkrautz mkrautz commented May 24, 2023

Add error when chunkSize is greater than the strict memory limit of the CacheLimiter.

Scenario:

export AZCOPY_BUFFER_GB=5
# Copy a single 5GB file to an Azure Blob Storage Container with a block size of 4000MB
./acopy [...] --block-size-mb=4000 [...]

Since the chunk size in that case is greater than the strict memory limit (75% of 5GB) this would end up in a deadlock: CacheLimiter.TryAdd would loop forever, waiting for memory to be freed (which it never will).

This patch tries to remedy that by making it an error if a chunk is greater than the strict memory limit of the associated CacheLimiter.

…he CacheLimiter.

Scenario:

    export AZCOPY_BUFFER_GB=5
    # Copy a single 5GB file to an Azure Blob Storage Container with a block size of 4000MB
    ./acopy [...] --block-size-mb=4000 [...]

Since the chunk size in that case is greater than the strict memory limit (75% of 5GB)
this would end up in a deadlock: CacheLimiter.TryAdd would loop forever, waiting for memory
to be freed (which it never will).

This patch tries to remedy that by making it an error if a chunk is greater than the strict
memory limit of the associated CacheLimiter.
@gapra-msft gapra-msft added this to the 10.19.0 milestone May 24, 2023
Copy link
Member

@adreed-msft adreed-msft left a comment

Choose a reason for hiding this comment

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

I think this is a good change; we should also do this with cap-mbps (though, we could partially move chunks in that scenario with a little refactoring)

Copy link
Contributor

@tasherif-msft tasherif-msft left a comment

Choose a reason for hiding this comment

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

thanks for the contribution

@adreed-msft adreed-msft merged commit 52de15b into Azure:dev May 25, 2023
9 checks passed
nakulkar-msft pushed a commit that referenced this pull request May 31, 2023
…he CacheLimiter. (#2228)

Scenario:

    export AZCOPY_BUFFER_GB=5
    # Copy a single 5GB file to an Azure Blob Storage Container with a block size of 4000MB
    ./acopy [...] --block-size-mb=4000 [...]

Since the chunk size in that case is greater than the strict memory limit (75% of 5GB)
this would end up in a deadlock: CacheLimiter.TryAdd would loop forever, waiting for memory
to be freed (which it never will).

This patch tries to remedy that by making it an error if a chunk is greater than the strict
memory limit of the associated CacheLimiter.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants