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 handling when reaching max keys listing S3 objects #676

Merged
merged 1 commit into from
Sep 19, 2023

Conversation

bryanlb
Copy link
Contributor

@bryanlb bryanlb commented Sep 15, 2023

Summary

There exists an issue today where when attempting to list files from S3, it can fail to exit the loop listing files. This is because there are no checks against reaching the default max keys of 1000. The new AWS CRT client has different behavior when attempting to read the continuation token of a listing that has reached max, which causes this to infinitely block downloading the chunk.

This increases the default of 1000 to 2500, and adds error checking for this edge case. The current chunk sizes of 15GB can occasionally just exceed 1k files which eventually causes the cluster to become unhealthy. Improvements such as #649 are resulting in better utilizing the disk space which can increase the likelihood of this occurring.

@bryanlb bryanlb force-pushed the bburkholder/crt3 branch 2 times, most recently from 850cd90 to b577822 Compare September 18, 2023 18:10
@bryanlb bryanlb changed the title Add more aggressive healthchecking for CRT client Add error handling when reaching max keys listing S3 objects Sep 18, 2023
@bryanlb bryanlb marked this pull request as ready for review September 18, 2023 23:33
Copy link
Contributor

@vthacker vthacker left a comment

Choose a reason for hiding this comment

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

nice find!

@bryanlb bryanlb merged commit eef7a7e into master Sep 19, 2023
2 checks passed
@bryanlb bryanlb deleted the bburkholder/crt3 branch September 19, 2023 16:23
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.

2 participants