Merged
Conversation
mildbyte
reviewed
Jun 20, 2024
src/frontend/flight/sync/metrics.rs
Outdated
Comment on lines
6
to
11
| const REQUEST: &str = "seafowl_sync_writer_request"; | ||
| const IN_MEMORY: &str = "seafowl_sync_writer_in_memory"; | ||
| const COMPACTION_TIME: &str = "seafowl_sync_writer_compaction_time"; | ||
| const COMPACTED: &str = "seafowl_sync_writer_compacted"; | ||
| const FLUSHING_TIME: &str = "seafowl_sync_writer_flushing_time"; | ||
| const FLUSHED: &str = "seafowl_sync_writer_flushed"; |
Contributor
There was a problem hiding this comment.
https://prometheus.io/docs/practices/naming/
The rule of thumb for these is that if you sum up some metric by all possible label values, it should still be a meaningful value, so we can't have different units in the same metric, instead:
# counter: total rows requested and total size of requests
seafowl_sync_writer_requests_rows_total
seafowl_sync_writer_requests_bytes_total
# gauges: how many rows in memory and how much space do they take
seafowl_sync_writer_in_memory_rows_total
seafowl_sync_writer_in_memory_bytes_total
# histogram: time taken by compaction
seafowl_sync_writer_compaction_time_seconds
# counter: total rows compacted, amount and size
seafowl_sync_writer_compacted_rows_total
seafowl_sync_writer_compacted_bytes_total
# histogram: time taken by flushes
seafowl_sync_writer_flushed_time_seconds
# counter: total rows flushed, amount and size
seafowl_sync_writer_flushed_bytes_total
seafowl_sync_writer_flushed_rows_total
src/frontend/flight/sync/metrics.rs
Outdated
| Gauge, Histogram, | ||
| }; | ||
|
|
||
| const REQUEST: &str = "seafowl_sync_writer_request"; |
Contributor
There was a problem hiding this comment.
I think we should rename this sync to something else, seafowl_delta_writer? seafowl_writer? seafowl_changeset_writer?
Contributor
Author
There was a problem hiding this comment.
I'll go with seafowl_changeset_writer.
mildbyte
approved these changes
Jun 20, 2024
src/frontend/flight/sync/metrics.rs
Outdated
| const REQUEST_ROWS: &str = "seafowl_changeset_writer_request_rows"; | ||
| const IN_MEMORY_BYTES: &str = "seafowl_changeset_writer_in_memory_bytes"; | ||
| const IN_MEMORY_ROWS: &str = "seafowl_changeset_writer_in_memory_rows"; | ||
| const IN_MEMORY_OLDEST: &str = "seafowl_changeset_writer_in_memory_oldest"; |
Contributor
There was a problem hiding this comment.
_oldest_timestamp_seconds?
src/frontend/flight/sync/metrics.rs
Outdated
| "The reduction in byte size due to batch compaction" | ||
| ); | ||
| describe_counter!( | ||
| COMPACTED_BYTES, |
Contributor
There was a problem hiding this comment.
Suggested change
| COMPACTED_BYTES, | |
| COMPACTED_ROWS, |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add metrics tracking byte size/row count for:
as well as timings for the last two ops.