This is an automated email from the ASF dual-hosted git repository.

alexey pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git


The following commit(s) were added to refs/heads/master by this push:
     new 84c838d3b [util] Add a warning into 'heap_sample_every_n_bytes' flag 
description.
84c838d3b is described below

commit 84c838d3b687c498467c87507eec687c446347c7
Author: 宋家成 <[email protected]>
AuthorDate: Mon Nov 20 17:53:12 2023 +0800

    [util] Add a warning into 'heap_sample_every_n_bytes' flag description.
    
    Parameter --heap_sample_every_n_bytes is actually a parameter of
    tcmalloc. Setting it to a positive value will make thread cache
    request some sample allocations from page cache, which will
    compete the lock of page heap with other threads. So does
    the free operation of the sample.
    
    A tserver with high throughput might lose so much performance
    because of the lock contention of tcmalloc. Almost all the apply
    threads are waiting for the spin lock.
    
    So add a warning on --heap_sample_every_n_bytes in case that
    some users do not know the terrible influence of this parameter.
    
    Change-Id: I00d55126f0cad0ac6cc130576cc45808f96f0a47
    Reviewed-on: http://gerrit.cloudera.org:8080/20716
    Tested-by: Kudu Jenkins
    Reviewed-by: Wang Xixu <[email protected]>
    Reviewed-by: Alexey Serbin <[email protected]>
---
 src/kudu/util/flags.cc | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/src/kudu/util/flags.cc b/src/kudu/util/flags.cc
index a069a2029..db64db2ec 100644
--- a/src/kudu/util/flags.cc
+++ b/src/kudu/util/flags.cc
@@ -96,10 +96,22 @@ DEFINE_int64(heap_sample_every_n_bytes, 0,
              "Enable heap occupancy sampling. If this flag is set to some 
positive "
              "value N, a memory allocation will be sampled approximately every 
N bytes. "
              "Lower values of N incur larger overhead but give more accurate 
results. "
-             "A value such as 524288 (512KB) is a reasonable choice with 
relatively "
-             "low overhead.");
+             "Warning: Setting this flag to a positive value increases lock 
contention "
+             "in tcmalloc; the lower the value, the higher the lock 
contention.");
 TAG_FLAG(heap_sample_every_n_bytes, advanced);
 TAG_FLAG(heap_sample_every_n_bytes, experimental);
+TAG_FLAG(heap_sample_every_n_bytes, unsafe);
+
+static bool ValidateHeapSample(const char* /*flagname*/, int64_t value) {
+  if (value > 0) {
+    LOG(WARNING) << "Flag heap_sample_every_n_bytes is set to a positive 
value, this may "
+                    "significantly increases lock contention in tcmalloc and 
degrade "
+                    "performance.";
+  }
+  return true;
+}
+
+DEFINE_validator(heap_sample_every_n_bytes, &ValidateHeapSample);
 #endif
 
 DEFINE_bool(disable_core_dumps, false, "Disable core dumps when this process 
crashes.");

Reply via email to