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: article on difficulties Zelda encountered while implementing streaming checksums #1427

Conversation

Velfi
Copy link
Contributor

@Velfi Velfi commented May 31, 2022

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

add: article on difficulties Zelda encountered while implementing streaming checksums
update: SUMMARY.md table of contents
@Velfi Velfi marked this pull request as ready for review May 31, 2022 19:27
@Velfi Velfi requested a review from a team as a code owner May 31, 2022 19:27
Copy link
Collaborator

@rcoh rcoh left a comment

Choose a reason for hiding this comment

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

this is great!! I suspect this sort of article may be useful for folks outside of the SDK as well–may be worth thinking about

constructed from files. In these cases we can ask the operating system for the file size before sending the request. So
long as that size doesn't change during sending of the request, all is well. In any other case, the request will fail.

### Adding trailers to a request changes the size of that request
Copy link
Collaborator

Choose a reason for hiding this comment

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

(!!!!)

Comment on lines +265 to +275
```rust
use axum::{
body::{Body, Bytes},
http::{request::Parts, Request, StatusCode},
middleware::{self, Next},
response::IntoResponse,
routing::put,
Router,
};
use std::net::SocketAddr;
use tracing_subscriber::{layer::SubscriberExt, util::SubscriberInitExt};
Copy link
Collaborator

Choose a reason for hiding this comment

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

do you think we should bundle this in the tools directory?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll submit a separate PR for that.

Copy link
Collaborator

@rcoh rcoh left a comment

Choose a reason for hiding this comment

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

we should also link Contributing from the root contribution guide when this lands and the link is available

@github-actions
Copy link

A new generated diff is ready to view.

A new doc preview is ready to view.

Rust Wrk benchmark report:

Duration: 90 sec, Connections: 32, Threads: 2

Measurement Deviation Current Old
Requests/sec 5.66% 39707.82 37581.37
Total requests 5.67% 3575747 3383902
Total errors NaN% 0 0
Total successes 5.67% 3575747 3383902
Average latency ms -3.41% 0.85 0.88
Minimum latency ms 0.00% 0.02 0.02
Maximum latency ms 8.83% 20.83 19.14
Stdev latency ms 12.07% 0.65 0.58
Transfer Mb 5.67% 371.7 351.76
Connect errors NaN% 0 0
Read errors NaN% 0 0
Write errors NaN% 0 0
Status errors (not 2xx/3xx) NaN% 0 0
Timeout errors NaN% 0 0

@github-actions
Copy link

A new generated diff is ready to view.

A new doc preview is ready to view.

Rust Wrk benchmark report:

Duration: 90 sec, Connections: 32, Threads: 2

Measurement Deviation Current Old
Requests/sec 7.23% 40920.53 38162.4
Total requests 7.27% 3684970 3435232
Total errors NaN% 0 0
Total successes 7.27% 3684970 3435232
Average latency ms -5.81% 0.81 0.86
Minimum latency ms 0.00% 0.02 0.02
Maximum latency ms 17.33% 19.57 16.68
Stdev latency ms 10.20% 0.54 0.49
Transfer Mb 7.27% 383.05 357.09
Connect errors NaN% 0 0
Read errors NaN% 0 0
Write errors NaN% 0 0
Status errors (not 2xx/3xx) NaN% 0 0
Timeout errors NaN% 0 0

@github-actions
Copy link

github-actions bot commented Jun 1, 2022

A new generated diff is ready to view.

A new doc preview is ready to view.

Rust Wrk benchmark report:

Duration: 90 sec, Connections: 32, Threads: 2

Measurement Deviation Current Old
Requests/sec -1.18% 84575.25 85586.34
Total requests -1.11% 7617857 7703629
Total errors NaN% 0 0
Total successes -1.11% 7617857 7703629
Average latency ms -4.58% 1.25 1.31
Minimum latency ms 0.00% 0.02 0.02
Maximum latency ms 23.41% 30.84 24.99
Stdev latency ms -3.07% 2.21 2.28
Transfer Mb -1.11% 791.88 800.8
Connect errors NaN% 0 0
Read errors NaN% 0 0
Write errors NaN% 0 0
Status errors (not 2xx/3xx) NaN% 0 0
Timeout errors NaN% 0 0

@Velfi Velfi merged commit 8ef9a85 into main Jun 1, 2022
@Velfi Velfi deleted the add/writing_and_debugging_a_low-level_feature_that_relies_on_HTTP.md branch June 1, 2022 15:03
82marbag pushed a commit that referenced this pull request Jun 7, 2022
…eaming checksums (#1427)

* add: contributing docs overview page
add: article on difficulties Zelda encountered while implementing streaming checksums
update: SUMMARY.md table of contents
add: link to contributor design docs in CONTRIBUTING.md
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