-
Notifications
You must be signed in to change notification settings - Fork 1
fix(tests): create values of block_size #6
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
base: main
Are you sure you want to change the base?
Conversation
| protocol: PubSubProtocol::default(), | ||
| block_size: ByteSize::mib(1), | ||
| tx_size: ByteSize::kib(1), | ||
| rpc_max_size: ByteSize::kib(25), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's set to 25 KiB because there are syncing tests that have batch_size of 2 and hence rpc_max_size needs to be greater than 20 KiB. Note that the max_response_size PR is not in yet.
86206fd to
0063083
Compare
0063083 to
34c41b5
Compare
rnbguy
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍🏼
| } | ||
| } | ||
| out | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this can be written better
fn split_bytes(mut bytes: Bytes, chunks: usize) -> Vec<Bytes> {
if chunks == 0 {
return vec![];
}
let total = bytes.len();
let chunk_len = total.div_ceil(chunks);
(0..chunks)
.map(|_| {
let split_size = cmp::min(bytes.len(), chunk_len);
bytes.split_to(split_size)
})
.collect()
}4b09cf8 to
2cf89c5
Compare
Closes: #5
Description
This PR introduces a way to utilize the
extensionsfield ofValueso that we can have blocks of roughlyblock_sizein the tests.Note that the size of blocks will not be exactly
block_sizebecause a block besides theValuecontains the commit certificate that adds a few tens of bytes to the total block size. Additionally, the actual block sent over the wire might be slightly more bytes due to encoding. This PR also bringsProtobufCodecinstead ofJsonCodecwhen encoding values in order to be able to be "closer" to theblock_size, otherwise, 1 KiB ofextensions, might lead to having 10 more KiB in theblock_sizedue to the wayextensionswould JSON encoded.Finally, note that this PR, besides a nit change in
code/examples/channel/src/state.rs, did not modify thechannelexample and left it as is.Testing
To see that it works, you can try the following 2 tests in
code/crates/test/tests/it/value_sync.rs:Ideally, we should be able to include
test_block_size_more_than_rpc_size_times_outin the codebase and have it verify that it times out. However, the way the testing framework is currently written,run_with_paramsdoes not return a result andexits internally, so we cannot do this without quite some refactoring.PR author checklist
For all contributors
RELEASE_NOTES.mdif the change warrants itBREAKING_CHANGES.mdif the change warrants itFor external contributors