durch / rust-s3

Rust library for interfacing with S3 API compatible services
MIT License
498 stars 195 forks source link

Move creation of the HTTP client from response() to the Bucket #360

Closed lyokha closed 8 months ago

lyokha commented 10 months ago

This must dramatically improve performance in high-loaded environments. This must fix #347 (at least, for the tokio backend).

Note that

durch commented 8 months ago

@lyokha, not opposed to the change in principle, but in practice, I'd prefer something like bring your own client, or similar, gonna give it some thought

durch commented 8 months ago

Implemented in https://github.com/durch/rust-s3/commit/7d7c57b5cc967299ebf338337c7fb3b68e57c03c

lyokha commented 8 months ago

Tested this (with axum as web server), looks good! Except it throws sometimes errors due to #337:

2023-10-16T10:41:18.466317Z  INFO request: Reading path tmp/hello.txt method=GET uri=/read/tmp/hello.txt version=HTTP/1.1
2023-10-16T10:41:18.466373Z  INFO request: Reading path tmp/hello.txt method=GET uri=/read/tmp/hello.txt version=HTTP/1.1
2023-10-16T10:41:18.466404Z  INFO request: Reading path tmp/hello.txt method=GET uri=/read/tmp/hello.txt version=HTTP/1.1
2023-10-16T10:41:18.466430Z ERROR request: S3 connection error: Could not get Read lock on Credentials method=GET uri=/read/tmp/hello.txt version=HTTP/1.1
2023-10-16T10:41:18.466458Z ERROR request: S3 connection error: Could not get Write lock on Credentials method=GET uri=/read/tmp/hello.txt version=HTTP/1.1
2023-10-16T10:41:18.466507Z  INFO request: finished processing request latency=0 ms status=502 method=GET uri=/read/tmp/hello.txt version=HTTP/1.1

In my case, the fix from #343 works, so it would be nice to include this into the new release. This would let using it reliably in high-loaded envs.