acelyc111 commented on a change in pull request #4110: URL: https://github.com/apache/incubator-doris/pull/4110#discussion_r457022902
########## File path: be/src/util/thread.cpp ########## @@ -169,6 +174,81 @@ void ThreadMgr::remove_thread(const pthread_t& pthread_id, const std::string& ca ANNOTATE_IGNORE_READS_AND_WRITES_END(); } +void ThreadMgr::start_instrumentation(const WebPageHandler::ArgumentMap& args, EasyJson* ej) const { + const auto* category_name = FindOrNull(args, "group"); + if (category_name) { + bool requested_all = (*category_name == "all"); + ej->Set("requested_thread_group", EasyJson::kObject); + (*ej)["group_name"] = escape_for_html_to_string(*category_name); + (*ej)["requested_all"] = requested_all; + + // The critical section is as short as possible so as to minimize the delay + // imposed on new threads that acquire the lock in write mode. + vector<ThreadDescriptor> descriptors_to_print; + if (!requested_all) { + MutexLock l(&_lock); + const auto* category = FindOrNull(_thread_categories, *category_name); + if (category) { + for (const auto& elem : *category) { + descriptors_to_print.emplace_back(elem.second); + } + } + } else { + MutexLock l(&_lock); + for (const auto& category : _thread_categories) { + for (const auto& elem : category.second) { + descriptors_to_print.emplace_back(elem.second); + } + } + } + + EasyJson found = (*ej).Set("found", EasyJson::kObject); + EasyJson threads = found.Set("threads", EasyJson::kArray); + for (const auto& desc : descriptors_to_print) { + summarize_thread_descriptor(desc, &threads); + } + } else { + // List all thread groups and the number of threads running in each. + vector<pair<string, uint64_t>> thread_categories_info; + uint64_t running; + { + MutexLock l(&_lock); + running = _threads_running_metric; + thread_categories_info.reserve(_thread_categories.size()); + for (const auto& category : _thread_categories) { + thread_categories_info.emplace_back(category.first, category.second.size()); + } + + (*ej)["total_threads_running"] = running; + EasyJson groups = ej->Set("groups", EasyJson::kArray); + for (const auto& elem : thread_categories_info) { + string category_arg; + url_encode(elem.first, &category_arg); + LOG(INFO) << "encode url path: " << category_arg; Review comment: This may cause too many useless logs. ########## File path: be/src/http/default_path_handlers.h ########## @@ -20,6 +20,8 @@ #include <stdio.h> +#include "util/thread.h" Review comment: I didn't see you use it, is it needed? ########## File path: be/src/util/thread.cpp ########## @@ -74,18 +78,17 @@ class ThreadMgr { // already been removed, this is a no-op. void remove_thread(const pthread_t& pthread_id, const std::string& category); -private: + void start_instrumentation(const WebPageHandler::ArgumentMap& args, EasyJson* ej) const; Review comment: I think we'd better call this function a xxx_handler, it's a callback of a http request. ########## File path: be/src/util/thread.cpp ########## @@ -397,8 +477,15 @@ Status ThreadJoiner::join() { } waited_ms += wait_for; } - return Status::Aborted(strings::Substitute("Timed out after $0ms joining on $1", - waited_ms, _thread->_name)); + return Status::Aborted( + strings::Substitute("Timed out after $0ms joining on $1", waited_ms, _thread->_name)); } +void start_thread_instrumentation(WebPageHandler* web_page_handler) { + web_page_handler->register_template_page( + "/threadz", "Threadz", Review comment: ```suggestion "/threadz", "Threads", ``` ########## File path: be/src/util/url_coding.cpp ########## @@ -122,59 +122,47 @@ static void encode_base64_internal(const std::string& in, std::string* out, out->assign((char*)buf.get(), d - buf.get()); } -void base64url_encode(const std::string& in, std::string *out) { +void base64url_encode(const std::string& in, std::string* out) { static unsigned char basis64[] = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-_"; encode_base64_internal(in, out, basis64, false); } void base64_encode(const std::string& in, std::string* out) { static unsigned char basis64[] = - "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/"; + "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/"; encode_base64_internal(in, out, basis64, true); } -static char encoding_table[] = { - 'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', - 'I', 'J', 'K', 'L', 'M', 'N', 'O', 'P', - 'Q', 'R', 'S', 'T', 'U', 'V', 'W', 'X', - 'Y', 'Z', 'a', 'b', 'c', 'd', 'e', 'f', - 'g', 'h', 'i', 'j', 'k', 'l', 'm', 'n', - 'o', 'p', 'q', 'r', 's', 't', 'u', 'v', - 'w', 'x', 'y', 'z', '0', '1', '2', '3', - '4', '5', '6', '7', '8', '9', '+', '/' -}; +static char encoding_table[] = {'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'J', 'K', 'L', 'M', Review comment: IMO, format files which are copied from other projects is not a good idea, because when we want to sync code from source project which have fixed some bugs or add some new features, there will be much diff and conflicts to resolve. ########## File path: be/src/util/thread.cpp ########## @@ -169,6 +174,81 @@ void ThreadMgr::remove_thread(const pthread_t& pthread_id, const std::string& ca ANNOTATE_IGNORE_READS_AND_WRITES_END(); } +void ThreadMgr::start_instrumentation(const WebPageHandler::ArgumentMap& args, EasyJson* ej) const { + const auto* category_name = FindOrNull(args, "group"); + if (category_name) { + bool requested_all = (*category_name == "all"); + ej->Set("requested_thread_group", EasyJson::kObject); + (*ej)["group_name"] = escape_for_html_to_string(*category_name); + (*ej)["requested_all"] = requested_all; + + // The critical section is as short as possible so as to minimize the delay + // imposed on new threads that acquire the lock in write mode. + vector<ThreadDescriptor> descriptors_to_print; + if (!requested_all) { + MutexLock l(&_lock); + const auto* category = FindOrNull(_thread_categories, *category_name); + if (category) { + for (const auto& elem : *category) { + descriptors_to_print.emplace_back(elem.second); + } + } + } else { + MutexLock l(&_lock); + for (const auto& category : _thread_categories) { + for (const auto& elem : category.second) { + descriptors_to_print.emplace_back(elem.second); + } + } + } + + EasyJson found = (*ej).Set("found", EasyJson::kObject); Review comment: If category is not found, we will also output normally? ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org For additional commands, e-mail: commits-h...@doris.apache.org