Ashwani Raina has posted comments on this change. ( http://gerrit.cloudera.org:8080/24618 )
Change subject: KUDU-3734 Compaction budget refinements and observability ...................................................................... Patch Set 9: (2 comments) http://gerrit.cloudera.org:8080/#/c/24618/9/src/kudu/tablet/compaction_policy.cc File src/kudu/tablet/compaction_policy.cc: http://gerrit.cloudera.org:8080/#/c/24618/9/src/kudu/tablet/compaction_policy.cc@690 PS9, Line 690: metrics_->compact_rs_rowsets_budget_skipped->set_value(skipped_count); : metrics_->compact_rs_bytes_budget_skipped->set_value(skipped_bytes); > The comment above claims 'skipped_count' and 'skipped_bytes' contains not o Yes, it is possible but it has its own challenges like no difference between 'no oversized rowsets' and 'not yet computed in this iteration'. If metric type is counter, the accumulation over all past invocations won't show correct picture. Event rates might give somewhat close picture but it add more complexity in usage. We are only interested in knowing rowsets that individually exceeded the budget and by how much in total, and reporting those can be achieved via the warning log message. compaction_budget_skip_health is enough to know that something is wrong with a tablet compaction budget-wise and warning message gives more information that is actionable. http://gerrit.cloudera.org:8080/#/c/24618/2/src/kudu/tablet/tablet_metrics.cc File src/kudu/tablet/tablet_metrics.cc: http://gerrit.cloudera.org:8080/#/c/24618/2/src/kudu/tablet/tablet_metrics.cc@447 PS2, Line 447: : 60000LU, 1); > I don't think that switching from an accumulative metric to a gauge resolve compact_rs_rowsets_budget_skipped and compact_rs_bytes_budget_skipped are never used to decide whether a tablet's compaction budget health is impacted or not. Check out the logic here: https://gerrit.cloudera.org/#/c/24618/9/src/kudu/tablet/compaction_policy.cc @ lineno 543-553 This is the logic that decides which tablet has compaction budget health issues and it decides that purely based on oversized_count that is directly related the base and delta size of a rowset. There is no interference of these two counters in decision making. The idea of having these counters was to get a sense of 'how much data is not being compacted' after each compaction cycle. The outcome is not actionable, but just some additional information. I agree that these numbers represent mix of both types - rowsets skipped due to budget and compaction policy scoring algorithm, which makes the use of these two metrics confusing even if these are not actionable. That said, I don't have any issue with removing both these i.e. compact_rs_rowsets_budget_skipped and compact_rs_bytes_budget_skipped. It has no impact on the actual core logic. -- To view, visit http://gerrit.cloudera.org:8080/24618 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id63db0006c77ad1d55eb2119d99ee64499435819 Gerrit-Change-Number: 24618 Gerrit-PatchSet: 9 Gerrit-Owner: Ashwani Raina <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Ashwani Raina <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber <[email protected]> Gerrit-Reviewer: Zoltan Martonka <[email protected]> Gerrit-Comment-Date: Tue, 15 Sep 2026 12:19:23 +0000 Gerrit-HasComments: Yes
