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

disable aws sdk logger if log level is not "trace" #460

Merged
merged 5 commits into from
Jul 18, 2022
Merged

disable aws sdk logger if log level is not "trace" #460

merged 5 commits into from
Jul 18, 2022

Conversation

kucukaslan
Copy link
Contributor

Currently, we request aws-sdk's logs regardless of s5cmd's log level, then we either use or discard those logs depending on the log level.

This commit disables aws-sdk logs (unless log level is "trace") instead of discarding the logs after they created.

So, it will reduce the memory/CPU usage when other log levels used, since those unneeded logs won't be created at all.

@kucukaslan kucukaslan requested a review from a team as a code owner July 5, 2022 15:59
@kucukaslan kucukaslan requested review from aykutfarsak and ilkinulas and removed request for a team July 5, 2022 15:59
Currently, we request aws-sdk's logs regardless of s5cmd's log level, then we either use or discard those logs depending on the log level.

This commit disables aws-sdk logs (unless log level is "trace") instead of discarding the logs after they created.

So, it will reduce the memory/CPU usage when other log levels used, since those unneeded logs won't be created at all.
CHANGELOG.md Outdated Show resolved Hide resolved
storage/s3.go Outdated Show resolved Hide resolved
storage/s3.go Outdated Show resolved Hide resolved
AWS logs should be written only if log level is trace, not unless.
@kucukaslan kucukaslan requested a review from igungor July 7, 2022 12:53
@kucukaslan kucukaslan merged commit 5b64469 into peak:master Jul 18, 2022
@kucukaslan kucukaslan deleted the aws-logger-memory-usage branch July 21, 2022 10:53
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.

4 participants