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

Fix Storage Quota Utilization Metric Emission for Realtime Tables #14780

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Yuyaoo
Copy link

@Yuyaoo Yuyaoo commented Jan 8, 2025

Overview

This PR updates the StorageQuotaChecker to ensure that the storage quota utilization metric is emitted during realtime segment validation. Previously, the metric was skipped for segments with a size of 0, which usually indicates a realtime validation path. Now that storage quota is supported for realtime tables, we need to monitor the metric.

Related PR: Storage quota imposition on realtime tables

Testing

Tested by triggering the RealtimeSegmentValidationManager periodic task within an integration test and using VisualVM to confirm that the tableStorageQuotaUtilization is emitted in controller metrics.

image

@sajjad-moradi
Copy link
Contributor

Please describe how you verified the metrics are actually emitted (maybe a screenshot of the metric in jvisualvm for an integration test)

@Yuyaoo Yuyaoo marked this pull request as ready for review January 8, 2025 23:27
@codecov-commenter
Copy link

codecov-commenter commented Jan 13, 2025

Codecov Report

Attention: Patch coverage is 50.00000% with 4 lines in your changes missing coverage. Please review.

Project coverage is 63.88%. Comparing base (59551e4) to head (07f482b).
Report is 1575 commits behind head on master.

Files with missing lines Patch % Lines
...not/controller/validation/StorageQuotaChecker.java 50.00% 3 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master   #14780      +/-   ##
============================================
+ Coverage     61.75%   63.88%   +2.13%     
- Complexity      207     1609    +1402     
============================================
  Files          2436     2704     +268     
  Lines        133233   150929   +17696     
  Branches      20636    23317    +2681     
============================================
+ Hits          82274    96418   +14144     
- Misses        44911    47302    +2391     
- Partials       6048     7209    +1161     
Flag Coverage Δ
custom-integration1 100.00% <ø> (+99.99%) ⬆️
integration 100.00% <ø> (+99.99%) ⬆️
integration1 100.00% <ø> (+99.99%) ⬆️
integration2 0.00% <ø> (ø)
java-11 63.85% <50.00%> (+2.14%) ⬆️
java-21 63.77% <50.00%> (+2.14%) ⬆️
skip-bytebuffers-false 63.88% <50.00%> (+2.13%) ⬆️
skip-bytebuffers-true 63.72% <50.00%> (+35.99%) ⬆️
temurin 63.88% <50.00%> (+2.13%) ⬆️
unittests 63.87% <50.00%> (+2.13%) ⬆️
unittests1 56.33% <ø> (+9.44%) ⬆️
unittests2 34.14% <50.00%> (+6.41%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Yuyaoo Yuyaoo changed the title Fix Storage Quota Utilization Metric Emission for Zero-Sized Segments Fix Storage Quota Utilization Metric Emission for Realtime Tables Jan 14, 2025
@Yuyaoo
Copy link
Author

Yuyaoo commented Jan 22, 2025

Hi @Jackie-Jiang, could you help take a look at this PR too when you get a chance? Thank you! cc @sajjad-moradi

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

Successfully merging this pull request may close these issues.

3 participants