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

add segment size metric on segment push #8387

Merged
merged 2 commits into from
Mar 23, 2022
Merged

add segment size metric on segment push #8387

merged 2 commits into from
Mar 23, 2022

Conversation

jadami10
Copy link
Contributor

@jadami10 jadami10 commented Mar 22, 2022

Description

Adds an offline segment size metric only when a new segment is pushed. this is stored as a gauge and based on table name. This is useful to build detectors in case:

  • someone tries to push a segment that is too large
  • your segments are organically growing in size and you want to know once you've hit a threshold

I tested this by deploying this change to one of our clusters, manually uploading a CSV file, and observing the metric in our metrics platform.

Upgrade Notes

Does this PR prevent a zero down-time upgrade? (Assume upgrade order: Controller, Broker, Server, Minion)

  • Yes (Please label as backward-incompat, and complete the section below on Release Notes)

Does this PR fix a zero-downtime upgrade introduced earlier?

  • Yes (Please label this as backward-incompat, and complete the section below on Release Notes)

Does this PR otherwise need attention when creating release notes? Things to consider:

  • New configuration options
  • Deprecation of configurations
  • Signature changes to public methods/interfaces
  • New plugins added or old plugins removed
  • Yes (Please label this PR as release-notes and complete the section on Release Notes)

Release Notes

This adds a metric for the last pushed segment size in bytes after the segment is successfully pushed.

Documentation

Copy link
Contributor

@Jackie-Jiang Jackie-Jiang left a comment

Choose a reason for hiding this comment

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

Let's add some release notes on this added metrics.
Also note that different controller will have their own gauge value based on the segment uploaded to them

@@ -69,6 +69,10 @@
// Estimated size of offline table
OFFLINE_TABLE_ESTIMATED_SIZE("OfflineTableEstimatedSize", false),

// Size of an uploaded offline segment
OFFLINE_SEGMENT_SIZE("OfflineSegmentSize", false),
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's name it LAST_PUSHED_SEGMENT_SIZE to be more specific

Copy link
Contributor Author

Choose a reason for hiding this comment

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

great suggestion

@jadami10
Copy link
Contributor Author

Let's add some release notes on this added metrics.

added. let me know if it's sufficient, please?

Also note that different controller will have their own gauge value based on the segment uploaded to them

that's ok for us as we can aggregate the metric across dimension that do not include the controller host. any concerns on your end?

@Jackie-Jiang Jackie-Jiang added the release-notes Referenced by PRs that need attention when compiling the next release notes label Mar 23, 2022
@Jackie-Jiang Jackie-Jiang merged commit 6d2f749 into apache:master Mar 23, 2022
Jackie-Jiang pushed a commit that referenced this pull request Mar 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-notes Referenced by PRs that need attention when compiling the next release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants