Ashwani Raina has posted comments on this change. ( http://gerrit.cloudera.org:8080/19281 )
Change subject: WIP KUDU-3406 memory budgeting for CompactRowSetsOp ...................................................................... Patch Set 2: (5 comments) http://gerrit.cloudera.org:8080/#/c/19281/2/src/kudu/tablet/tablet.cc File src/kudu/tablet/tablet.cc: http://gerrit.cloudera.org:8080/#/c/19281/2/src/kudu/tablet/tablet.cc@199 PS2, Line 199: 8.0 Do we need to consider different values for compressed and uncompressed data? http://gerrit.cloudera.org:8080/#/c/19281/2/src/kudu/tablet/tablet.cc@200 PS2, Line 200: Euristic nit: Did you mean 'Heuristic' ? http://gerrit.cloudera.org:8080/#/c/19281/2/src/kudu/tablet/tablet.cc@210 PS2, Line 210: avialable nit: available http://gerrit.cloudera.org:8080/#/c/19281/2/src/kudu/tablet/tablet.cc@2361 PS2, Line 2361: quality I remember we were discussing about scenario wherein AdjustedPerfScore relies on perf_imp score, workload score and priority and in some cases where perf_improvement score is 0, it would just return with 0 as perf_improvement and possibly not give priority to Compaction op (in this case) a chance to run. This comment not be relevant anymore, but do we need to increase the perf score as well if quality is 0? Just in case PickRowSets keeps quality as 0 and also picks some rowsets for compaction, we would have such a case. Feel free to ignore this comment if that is never the case. http://gerrit.cloudera.org:8080/#/c/19281/2/src/kudu/tablet/tablet_mm_ops.cc File src/kudu/tablet/tablet_mm_ops.cc: http://gerrit.cloudera.org:8080/#/c/19281/2/src/kudu/tablet/tablet_mm_ops.cc@153 PS2, Line 153: uint64_t new_num_mrs_flushed = metrics->flush_mrs_duration->TotalCount(); : uint64_t new_num_dms_flushed = metrics->flush_dms_duration->TotalCount(); : uint64_t new_num_rs_compacted = metrics->compact_rs_duration->TotalCount(); I vaguely remember during my testing, I was getting these counter values as 0 and prev_stats.valid() used to return true. So the below condition was coming out to be true. However, I am not sure whether mrs and rowset compaction ops were running during the time. Please make sure that these metrics values are changing with corresponding ops. I guess we could rely on valid() method but it is better to verify. -- To view, visit http://gerrit.cloudera.org:8080/19281 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I89c171284944831e95c45a993d85fbefe89048cf Gerrit-Change-Number: 19281 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin <[email protected]> Gerrit-Reviewer: Abhishek Chennaka <[email protected]> Gerrit-Reviewer: Ashwani Raina <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yifan Zhang <[email protected]> Gerrit-Reviewer: Yingchun Lai <[email protected]> Gerrit-Comment-Date: Tue, 29 Nov 2022 16:40:14 +0000 Gerrit-HasComments: Yes
