-
Notifications
You must be signed in to change notification settings - Fork 10
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
momento: support store workflow #240
Conversation
#[derive(Debug, PartialEq)] | ||
pub enum ClientWorkItemKind<T> { | ||
Reconnect, | ||
Request { request: T, sequence: u64 }, | ||
} | ||
|
||
pub async fn reconnect<TRequestKind>( | ||
work_sender: Sender<ClientWorkItemKind<TRequestKind>>, | ||
config: Config, | ||
) -> Result<()> { |
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 the impetus for DRY'ing up the WorkItem
types. It seemed unnecessary to copy and paste this entire sequence of code to support a Store client when the end result and means is the exact same.
The only difference between the two is the inner request type, which can be specified at compile-time (as seen in changes throughout the codebase).
Overall this looks good to me. We can ignore the cargo audit failure. I think if you merge main into this feature branch and address the merge conflict, that this might go away. Let's also add some CLI metrics that are meaningful for storage workloads. Once that's all addressed, I'm fine with merging this. Thanks for opening the PR! |
…nct store client metrics. add store metrics output to cli.
@brayniac thank you for the feedback! I've gone ahead and fixed merge conflicts as well as added the CLI metrics for storage workloads in c168050 Yes, a lot of it is copy-paste from client metrics, but I realized as I was adding it that the storage client is indeed a distinct set of metrics and we should capture that. As a user, there is a very real possibility I have a cache client with different set of configurations and resulting behaviors when compared to the store client, and I would want to see a good comparison of how the two are performing. I've also gone ahead and changed Finally, I added distinct |
Problem
v0.42.0
store
workflowstore
is a high-performance, low-latency storage system for storing persistent itemsstore
supports is very small for now: get/set/deletestorage
workflow run simultaneously to acache
workflowSolution
storage
client config.client
andstorage
should not share the same config settingsstore
workflow(s), only supporting the Momento protocol (for now).WorkItem
typing so that reconnection code can be reused between thecache
client andstore
client code, but the distinct enum per work item type remains distinct. This ensures there is still compile-time checks for the right enum being sent to aReceiver<T>
(see https://github.com/iopsystems/rpc-perf/pull/240/files#r1680025693)Result
Built binary and deployed to a local testing environment, running with only a store config. Example config:
Upon executing the newly generated binary, I could confirm that the
store
workflow successfully exercised traffic and recorded expected metrics a distinctSET
andGET
metrics.