kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From danburk...@apache.org
Subject [2/2] kudu git commit: Fix bug causing undercounting of thread count metric
Date Thu, 18 Jan 2018 02:57:58 GMT
Fix bug causing undercounting of thread count metric

Early in tserver startup a handful (~5) threads are created before
StartThreadInstrumentation is called, which causes the thread manager
not to count them in the thread count metrics.

I found this bug because it would cause the thread count metric to dip
below zero on tserver shutdown, since the uncounted threads would still
decrement the metric. The fix is straightfoward and simplifies the code:
threads should always update the thread count metrics, even if the
thread pool is not yet registered with the metric handler.

Change-Id: Icd36b5c7d0ed1e157c0960011e0d8e44e143c205
Reviewed-on: http://gerrit.cloudera.org:8080/9049
Reviewed-by: Todd Lipcon <todd@apache.org>
Tested-by: Kudu Jenkins


Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/d3165ed2
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/d3165ed2
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/d3165ed2

Branch: refs/heads/master
Commit: d3165ed2b511d40a34c9f27a41bfe041b9c45bdc
Parents: df65df6
Author: Dan Burkert <dan@danburkert.com>
Authored: Wed Jan 17 18:07:41 2018 -0800
Committer: Dan Burkert <danburkert@apache.org>
Committed: Thu Jan 18 02:50:15 2018 +0000

----------------------------------------------------------------------
 src/kudu/util/thread.cc | 23 ++++++-----------------
 1 file changed, 6 insertions(+), 17 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/d3165ed2/src/kudu/util/thread.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/thread.cc b/src/kudu/util/thread.cc
index 0378bb6..b5db351 100644
--- a/src/kudu/util/thread.cc
+++ b/src/kudu/util/thread.cc
@@ -158,8 +158,7 @@ static GoogleOnceType once = GOOGLE_ONCE_INIT;
 class ThreadMgr {
  public:
   ThreadMgr()
-      : metrics_enabled_(false),
-        threads_started_metric_(0),
+      : threads_started_metric_(0),
         threads_running_metric_(0) {
   }
 
@@ -211,15 +210,12 @@ class ThreadMgr {
   // All thread categorys, keyed on the category name.
   typedef map<string, ThreadCategory> ThreadCategoryMap;
 
-  // Protects thread_categories_ and metrics_enabled_
+  // Protects thread_categories_ and thread metrics.
   Mutex lock_;
 
   // All thread categorys that ever contained a thread, even if empty
   ThreadCategoryMap thread_categories_;
 
-  // True after StartInstrumentation(..) returns
-  bool metrics_enabled_;
-
   // Counters to track all-time total number of threads, and the
   // current number of running threads.
   uint64_t threads_started_metric_;
@@ -263,7 +259,6 @@ void ThreadMgr::SetThreadName(const string& name, int64_t tid) {
 Status ThreadMgr::StartInstrumentation(const scoped_refptr<MetricEntity>& metrics,
                                        WebCallbackRegistry* web) {
   MutexLock l(lock_);
-  metrics_enabled_ = true;
 
   // Use function gauges here so that we can register a unique copy of these metrics in
   // multiple tservers, even though the ThreadMgr is itself a singleton.
@@ -325,10 +320,8 @@ void ThreadMgr::AddThread(const pthread_t& pthread_id, const string&
name,
   {
     MutexLock l(lock_);
     thread_categories_[category][pthread_id] = ThreadDescriptor(category, name, tid);
-    if (metrics_enabled_) {
-      threads_running_metric_++;
-      threads_started_metric_++;
-    }
+    threads_running_metric_++;
+    threads_started_metric_++;
   }
   ANNOTATE_IGNORE_SYNC_END();
   ANNOTATE_IGNORE_READS_AND_WRITES_END();
@@ -342,9 +335,7 @@ void ThreadMgr::RemoveThread(const pthread_t& pthread_id, const string&
category
     auto category_it = thread_categories_.find(category);
     DCHECK(category_it != thread_categories_.end());
     category_it->second.erase(pthread_id);
-    if (metrics_enabled_) {
-      threads_running_metric_--;
-    }
+    threads_running_metric_--;
   }
   ANNOTATE_IGNORE_SYNC_END();
   ANNOTATE_IGNORE_READS_AND_WRITES_END();
@@ -403,9 +394,7 @@ void ThreadMgr::ThreadPathHandler(const WebCallbackRegistry::WebRequest&
req,
     (*output) << "</tbody></table>";
   } else {
     (*output) << "<h2>Thread Groups</h2>";
-    if (metrics_enabled_) {
-      (*output) << "<h4>" << threads_running_metric_ << " thread(s)
running";
-    }
+    (*output) << "<h4>" << threads_running_metric_ << " thread(s)
running";
     (*output) << "<a href='/threadz?group=all'><h3>All Threads</h3>";
 
     for (const ThreadCategoryMap::value_type& category : thread_categories_) {


Mime
View raw message