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

Track store metrics by DB column #6041

Merged
merged 1 commit into from
Jul 18, 2024
Merged

Conversation

dapplion
Copy link
Collaborator

@dapplion dapplion commented Jul 3, 2024

Issue Addressed

When adding metrics for

Noted that the DB metrics are already there but we don't track them by column. I would like to be able to tell:

  • How many reads / writes happen on the state diff buffer column?
  • How many bytes do we load / store on the state diff buffer column?

Proposed Changes

  • Switch store metrics from IntCounter to IntCounterVec and label by column enum 3 letter string.
  • Track metrics in do_atomically calls, optimistically assuming success

leveldb_batch.put(BytesKey::from_vec(key), &value);
}

KeyValueStoreOp::DeleteKey(key) => {
let col = get_col_from_key(&key).unwrap_or("unknown".to_owned());
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

get_col_from_key should never error as we define the DBColumn enum variants. The use of col is purely informational on metrics, so unwrapping to unknown to have something

@dapplion dapplion added the ready-for-review The code is ready for review label Jul 3, 2024
Copy link
Member

@michaelsproul michaelsproul left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@michaelsproul michaelsproul added ready-for-merge This PR is ready to merge. and removed ready-for-review The code is ready for review labels Jul 16, 2024
@jimmygchen
Copy link
Member

@mergify queue

Copy link

mergify bot commented Jul 18, 2024

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at b0b142c

mergify bot added a commit that referenced this pull request Jul 18, 2024
@mergify mergify bot merged commit b0b142c into sigp:unstable Jul 18, 2024
28 checks passed
@dapplion dapplion deleted the store-metrics-vec branch July 24, 2024 07:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-for-merge This PR is ready to merge.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants