momentohq/client-sdk-rust

only call scs.set when I invoke `cache set`

Closed this issue · 5 comments

image

Consider:

for i in {1..1000000}; do momento cache set --key yo --value hi ; done ;

Expected: I am calling set() on my cache.

Observed: I am calling get() once for every set() on my cache.

Confirmed I could reproduce this in my dev cell: question is whether we are actually calling get after set or if we are not emitting metrics right on the service-side

Definitely a CLI issue, did a bunch of sets without gets from the Python SDK against my dev cell and there were no gets associated.

I see what this is, this is an issue with the Rust SDK itself, not the CLI itself. Going to move this issue to the SDK repo so we can properly track and I can link the sections in the code for why this happens

@tylerburdsall this is an artifact of how we originally wrote the sdks. Originally we wanted to make sure a cache existed before we let the customer continue to use our client.

Since the momento cli creates a new client for every request, on every request it was doing a get to validate that the cache existed, before performing the requested command.

We have since decided to alter how our sdks are created, to support multiple caches per client, and therefore not verify caches exists on client creation. We should make the rust sdk parity with the java and python sdk implementations

momentohq/client-sdk-java#100

Here's how this happens:

  1. When a Rust client is created and we call set, we have to get our cache info: https://github.com/momentohq/momento-cli/blob/main/src/commands/cache/cache.rs#L60
  2. We call get_cache here: https://github.com/momentohq/momento-cli/blob/main/src/commands/cache/cache.rs#L15
  3. When we call get_cache, we test our connection via connect(): https://github.com/momentohq/client-sdk-rust/blob/main/src/sdk.rs#L98
  4. Inside connect we make a call to verify_cache_exists: https://github.com/momentohq/client-sdk-rust/blob/main/src/cache.rs#L92
  5. We then make a test get when verifying the cache exists: https://github.com/momentohq/client-sdk-rust/blob/main/src/cache.rs#L97

Including this just for debugging purposes and documenting how I found this although we already know the issue. Like @bruuuuuuuce mentioned we didn't update this SDK and it needs to match parity with the Java/Python SDK implementations instead