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

Expose shard index used for metrics purposes #335

Open
tfreiberg-fastly opened this issue Mar 5, 2025 · 2 comments
Open

Expose shard index used for metrics purposes #335

tfreiberg-fastly opened this issue Mar 5, 2025 · 2 comments

Comments

@tfreiberg-fastly
Copy link
Contributor

We want to instrument our use of a large, single dashmap in our server process with some metrics like lock held time.
These metrics are only accurate if we can track which shard we operate on.
For now we're simply recording metrics for the entire dashmap and the shard count, which gives us the average per shard but nothing more.

The current raw-api feature theoretically allows getting the shard, but it requires us to hash twice, once to get the shard and once inside dashmap, which is an unacceptable waste of work.

I can think of two possible solutions, both of which would require an API change (or a private fork):

  1. Make many more internals public with the raw-api feature (similar to what Consider making RefMut::new public when raw-api is enabled #248 asks for) and reimplement get/get_mut/entry_ref in our codebase, giving us visibility of the shard index for free.
  2. Adding variants of the main public methods that also return the shard index, e.g. get_mut_with_shard_index(..) -> (Option<RefMut<...>>, usize or something like that, probably behind a new feature flag

I would be willing to implement any of these, if they're welcome, but i'm aware that they're both not very elegant ideas

@xacrimon
Copy link
Owner

xacrimon commented Mar 5, 2025

Hm, yeah these are all somewhat kludgy but the use-case seems legit based on this and the other issue you linked, so I'm definently amenable to a solution here, since being practically useful to people is why I maintain this in the first place.

I'm really not a fan of 2, at least not before some showerthinking for a less "specific" API. My issue there is mostly method count ballooning for what ends up being very specialized operations with very low re-use value. Alt. 1 however sounds good to me, I'm fine with exposing more impl details behind that feature, that's not really a problem for me at all.

I'm currently somewhat bogged by finals in the coming week or two, but if you're willing to implement these I should definently have enough time for "low latency" reviews and pushing it in a new rc. Feel free to ping me for a look before spending a ton of time on refinements, when you have a draft that exposes the functionality you'd need so there's less re-do work if anything needs adjusting.

Also, you probably already know this well, but remember to pick an appropriate hasher for your workload, though the new default in the v7 release candidates is probably a good pick for you folks anyhow. This is much more important on v6 or older.

@tfreiberg-fastly
Copy link
Contributor Author

sounds great! good luck with your finals :)

we decided to start using dashmap without these per-shard metrics, which means i've got some higher priority things to do before i start with this.
if you don't hear from me in the next weeks, then more high priority things came up 😅
otherwise i'll proceed with option 1

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

No branches or pull requests

2 participants