Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/23726 )
Change subject: KUDU-3722 unit mismatch in ProceedWithFlush ...................................................................... Patch Set 1: Code-Review+1 (2 comments) Thank you for detecting and fixing the bug! http://gerrit.cloudera.org:8080/#/c/23726/1/src/kudu/util/maintenance_manager-test.cc File src/kudu/util/maintenance_manager-test.cc: http://gerrit.cloudera.org:8080/#/c/23726/1/src/kudu/util/maintenance_manager-test.cc@352 PS1, Line 352: static_cast<double> nit for here and below: it's safe to remove static_cast, isn't it? http://gerrit.cloudera.org:8080/#/c/23726/1/src/kudu/util/maintenance_manager.cc File src/kudu/util/maintenance_manager.cc: http://gerrit.cloudera.org:8080/#/c/23726/1/src/kudu/util/maintenance_manager.cc@724 PS1, Line 724: if (process_memory::UnderMemoryPressure(used_memory_percentage)) { : double pressure_threshold = static_cast<double>(FLAGS_memory_pressure_percentage); : double soft_limit = static_cast<double>(FLAGS_memory_limit_soft_percentage); : return pressure_threshold >= soft_limit || *used_memory_percentage >= soft_limit || : rand_.NextDoubleFraction() >= FLAGS_run_non_memory_ops_prob * : (soft_limit - *used_memory_percentage) / (soft_limit - pressure_threshold); : } : return false; : } I'd suggest to update the implementation of this method a bit. There is a room for improvement since FLAGS_memory_pressure_percentage and FLAGS_memory_limit_soft_percentage aren't changing in runtime: at least they aren't supposed to change according to the code in process_memory.cc in DoInitLimits(). Also, multiplication of floating point numbers are faster than division due to the nature of the corresponding CPU operations, IIRC. It might be something like below. bool MaintenanceManager::ProceedWithFlush(double* used_memory_percentage) { if (!process_memory::UnderMemoryPressure(used_memory_percentage)) { return false; } static const double pressure_threshold = FLAGS_memory_pressure_percentage; static const double soft_limit = FLAGS_memory_limit_soft_percentage; static const double pressure_diff = soft_limit - pressure_threshold; const double used_diff = soft_limit - *used_memory_percentage; return pressure_diff <= 0 || used_diff <= 0 || rand_.NextDoubleFraction() * pressure_diff >= FLAGS_run_non_memory_ops_prob * used_diff; } -- To view, visit http://gerrit.cloudera.org:8080/23726 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Icb4c299a83c61bd934d67f32446696988bbe3a08 Gerrit-Change-Number: 23726 Gerrit-PatchSet: 1 Gerrit-Owner: Yongheng Deng <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 03 Dec 2025 03:39:23 +0000 Gerrit-HasComments: Yes
