-
Notifications
You must be signed in to change notification settings - Fork 27
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
Coalesce requests to KV #712
Conversation
98b51be
to
62bfaab
Compare
cfd31c4
to
3138c92
Compare
ca3c9e4
to
65e2f29
Compare
65e2f29
to
5c2a9fd
Compare
ab6d381
to
1750d54
Compare
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.
Nice functionality, changes are ok.
crates/daphne-server/src/storage_proxy_connection/kv/request_coalescer.rs
Outdated
Show resolved
Hide resolved
crates/daphne-server/src/storage_proxy_connection/kv/request_coalescer.rs
Outdated
Show resolved
Hide resolved
let op_key = OperationKey { | ||
op_id: operation_id::of::<F, Fut, R, E>(), | ||
key: key.clone(), | ||
}; |
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.
can the op_key
variable be defined before the loop?
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.
That results in an additional clone when passing it to the ResponseSender
type CoalescedValue = | ||
Result<Option<Marc<dyn Any + Send + Sync>>, CoalesceError<dyn Any + Send + Sync>>; | ||
|
||
/// The send half to be used by the `executor` to send the result of it's computation to the |
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.
/// The send half to be used by the `executor` to send the result of it's computation to the | |
/// The send have to be used by the `executor` to send the result of it's computation to the |
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.
This is actually correct. A channel has two parts, the sending part and the receiving part, commonly referred to as the sending and receiving halves
1750d54
to
a8cea45
Compare
This PR is best viewed commit by commit.