You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@geode.apache.org by GitBox <gi...@apache.org> on 2022/02/03 17:24:11 UTC

[GitHub] [geode-native] mmartell opened a new pull request #918: GEODE-10016: Add map of threadId to threadName

mmartell opened a new pull request #918:
URL: https://github.com/apache/geode-native/pull/918


   Since all internally created native client threads are named, we should print the threadName instead of threadId. This will be extremely helpful to understanding the flow of messages since there are many background threads in the native client.


-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode-native] mmartell commented on a change in pull request #918: GEODE-10016: Add map of threadId to threadName

Posted by GitBox <gi...@apache.org>.
mmartell commented on a change in pull request #918:
URL: https://github.com/apache/geode-native/pull/918#discussion_r801207475



##########
File path: cppcache/src/Log.cpp
##########
@@ -378,12 +421,21 @@ std::string Log::formatLogLine(LogLevel level) {
       now - std::chrono::system_clock::from_time_t(secs));
   auto tm_val = apache::geode::util::chrono::localtime(secs);
 
+  std::thread::id id = std::this_thread::get_id();
+  std::stringstream ss;
+  ss << id;
+  std::string threadIdWithName;
+  if (getThreadName() != "") {
+    threadIdWithName = ss.str() + " (" + getThreadName() + ")";
+  } else {
+    threadIdWithName = ss.str();
+  }
+
   msg << "[" << Log::levelToChars(level) << " "
       << std::put_time(&tm_val, "%Y/%m/%d %H:%M:%S") << '.' << std::setfill('0')
       << std::setw(6) << microseconds.count() << ' '
       << std::put_time(&tm_val, "%z  ") << g_hostName << ":"
-      << boost::this_process::get_id() << " " << std::this_thread::get_id()
-      << "] ";
+      << boost::this_process::get_id() << " " << threadIdWithName << "] ";

Review comment:
       Good catch. Done.




-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode-native] pivotal-jbarrett commented on a change in pull request #918: GEODE-10016: Add map of threadId to threadName

Posted by GitBox <gi...@apache.org>.
pivotal-jbarrett commented on a change in pull request #918:
URL: https://github.com/apache/geode-native/pull/918#discussion_r799849525



##########
File path: cppcache/src/DistributedSystemImpl.cpp
##########
@@ -38,6 +38,10 @@
 #include "util/Log.hpp"
 #include "version.h"
 
+namespace {
+std::map<std::thread::id, std::string> m_threadNames;

Review comment:
       I am referring to the variable name. This is a global variable, now a member variable and even member variables aren't supported to have the old `m_` prefix.




-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode-native] mmartell commented on a change in pull request #918: GEODE-10016: Add map of threadId to threadName

Posted by GitBox <gi...@apache.org>.
mmartell commented on a change in pull request #918:
URL: https://github.com/apache/geode-native/pull/918#discussion_r801291414



##########
File path: cppcache/src/ThreadPool.cpp
##########
@@ -16,20 +16,19 @@
  */
 #include "ThreadPool.hpp"
 
+#include "./util/Log.hpp"

Review comment:
       Done




-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode-native] mmartell commented on a change in pull request #918: GEODE-10016: Add map of threadId to threadName

Posted by GitBox <gi...@apache.org>.
mmartell commented on a change in pull request #918:
URL: https://github.com/apache/geode-native/pull/918#discussion_r801202171



##########
File path: cppcache/src/Log.cpp
##########
@@ -378,12 +421,21 @@ std::string Log::formatLogLine(LogLevel level) {
       now - std::chrono::system_clock::from_time_t(secs));
   auto tm_val = apache::geode::util::chrono::localtime(secs);
 
+  std::thread::id id = std::this_thread::get_id();

Review comment:
       Good catch. Inlined.




-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode-native] mmartell commented on a change in pull request #918: GEODE-10016: Add map of threadId to threadName

Posted by GitBox <gi...@apache.org>.
mmartell commented on a change in pull request #918:
URL: https://github.com/apache/geode-native/pull/918#discussion_r801148644



##########
File path: cppcache/src/DistributedSystemImpl.hpp
##########
@@ -48,6 +49,7 @@ using CliCallbackMethod = std::function<void(Cache&)>;
 class DistributedSystemImpl {
  public:
   static void setThreadName(const std::string& threadName);

Review comment:
       Indeed. I moved it to the Log class.




-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode-native] mmartell commented on a change in pull request #918: GEODE-10016: Add map of threadId to threadName

Posted by GitBox <gi...@apache.org>.
mmartell commented on a change in pull request #918:
URL: https://github.com/apache/geode-native/pull/918#discussion_r800935951



##########
File path: cppcache/src/Log.cpp
##########
@@ -382,7 +383,8 @@ std::string Log::formatLogLine(LogLevel level) {
       << std::put_time(&tm_val, "%Y/%m/%d %H:%M:%S") << '.' << std::setfill('0')
       << std::setw(6) << microseconds.count() << ' '
       << std::put_time(&tm_val, "%z  ") << g_hostName << ":"
-      << boost::this_process::get_id() << " " << std::this_thread::get_id()
+      << boost::this_process::get_id() << " "
+      << DistributedSystemImpl::getThreadName(std::this_thread::get_id())

Review comment:
       Good catch. gnmsg has been updated to handle either kind of log lines as below:
   ```[debug 2022/02/06 07:44:02.450497 -0800  FirstPro:20336 18736] EventId::EventId ...
   [fine 2022/02/06 07:44:02.453857 -0800  FirstPro:20336 18812 (NC ETM Thread)] ExpiryTaskManager thread is running.
   ```[debug 2022/02/06 07:44:02.450497 -0800  FirstPro:20336 18736] EventId::EventId ...




-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode-native] mmartell commented on a change in pull request #918: GEODE-10016: Add map of threadId to threadName

Posted by GitBox <gi...@apache.org>.
mmartell commented on a change in pull request #918:
URL: https://github.com/apache/geode-native/pull/918#discussion_r800935951



##########
File path: cppcache/src/Log.cpp
##########
@@ -382,7 +383,8 @@ std::string Log::formatLogLine(LogLevel level) {
       << std::put_time(&tm_val, "%Y/%m/%d %H:%M:%S") << '.' << std::setfill('0')
       << std::setw(6) << microseconds.count() << ' '
       << std::put_time(&tm_val, "%z  ") << g_hostName << ":"
-      << boost::this_process::get_id() << " " << std::this_thread::get_id()
+      << boost::this_process::get_id() << " "
+      << DistributedSystemImpl::getThreadName(std::this_thread::get_id())

Review comment:
       Good catch. gnmsg has been updated to handle either kind of log lines as below:
   ```
   [fine 2022/02/06 07:44:02.453857 -0800  FirstPro:20336 18812 (NC ETM Thread)] ExpiryTaskManager thread is running.
   ```[debug 2022/02/06 07:44:02.450497 -0800  FirstPro:20336 18736] EventId::EventId ...




-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode-native] mmartell commented on a change in pull request #918: GEODE-10016: Add map of threadId to threadName

Posted by GitBox <gi...@apache.org>.
mmartell commented on a change in pull request #918:
URL: https://github.com/apache/geode-native/pull/918#discussion_r801205976



##########
File path: cppcache/src/Log.cpp
##########
@@ -38,6 +38,7 @@
 #include <geode/ExceptionTypes.hpp>
 #include <geode/util/LogLevel.hpp>
 
+#include "DistributedSystemImpl.hpp"

Review comment:
       Done




-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode-native] gaussianrecurrence commented on a change in pull request #918: GEODE-10016: Add map of threadId to threadName

Posted by GitBox <gi...@apache.org>.
gaussianrecurrence commented on a change in pull request #918:
URL: https://github.com/apache/geode-native/pull/918#discussion_r799326466



##########
File path: cppcache/src/Log.cpp
##########
@@ -382,7 +383,8 @@ std::string Log::formatLogLine(LogLevel level) {
       << std::put_time(&tm_val, "%Y/%m/%d %H:%M:%S") << '.' << std::setfill('0')
       << std::setw(6) << microseconds.count() << ' '
       << std::put_time(&tm_val, "%z  ") << g_hostName << ":"
-      << boost::this_process::get_id() << " " << std::this_thread::get_id()
+      << boost::this_process::get_id() << " "
+      << DistributedSystemImpl::getThreadName(std::this_thread::get_id())

Review comment:
       I am not totally sure about this, but changing the logging format might break the GNMSG tool that @pdxcodemonkey implemented some time ago. I am not sure what level of support you are intending to provide for this tool, but might be  good to make sure that the tool could handle both log formats

##########
File path: cppcache/src/ThreadPool.cpp
##########
@@ -16,6 +16,8 @@
  */
 #include "ThreadPool.hpp"
 
+#include <sstream>

Review comment:
       I think this might be a remaining of some dev debugging. Is this needed anymore?

##########
File path: cppcache/src/DistributedSystemImpl.cpp
##########
@@ -143,11 +147,24 @@ void DistributedSystemImpl::unregisterCliCallback(int appdomainId) {
   }
 }
 
-void DistributedSystemImpl::setThreadName(const std::string& threadName) {
+std::string DistributedSystemImpl::getThreadName(std::thread::id id) {

Review comment:
       Is there any particular reason why you would want to obtain the thread name from another thread which is not the caller thread? Because if there is no reason, I'd say it'd be best to get the thread ID of the current thread within the function, and that would make syntax a little bit simpler.

##########
File path: cppcache/src/DistributedSystemImpl.cpp
##########
@@ -143,11 +147,24 @@ void DistributedSystemImpl::unregisterCliCallback(int appdomainId) {
   }
 }
 
-void DistributedSystemImpl::setThreadName(const std::string& threadName) {
+std::string DistributedSystemImpl::getThreadName(std::thread::id id) {
+  std::string threadName = m_threadNames[id];
+  if (threadName == "") {
+    std::stringstream ss;
+    ss << id;
+    threadName = ss.str();
+  }
+  return threadName;
+}
+
+void DistributedSystemImpl::setThreadName(const std::string& threadName,

Review comment:
       Same comment regarding thread ID as for getThreadName, IMHO I don't see a reason why you would need to pass a thread ID here




-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode-native] mmartell commented on a change in pull request #918: GEODE-10016: Add map of threadId to threadName

Posted by GitBox <gi...@apache.org>.
mmartell commented on a change in pull request #918:
URL: https://github.com/apache/geode-native/pull/918#discussion_r800104741



##########
File path: cppcache/src/DistributedSystemImpl.cpp
##########
@@ -38,6 +38,10 @@
 #include "util/Log.hpp"
 #include "version.h"
 
+namespace {
+std::map<std::thread::id, std::string> m_threadNames;

Review comment:
       Good catch.




-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode-native] mmartell commented on a change in pull request #918: GEODE-10016: Add map of threadId to threadName

Posted by GitBox <gi...@apache.org>.
mmartell commented on a change in pull request #918:
URL: https://github.com/apache/geode-native/pull/918#discussion_r800935951



##########
File path: cppcache/src/Log.cpp
##########
@@ -382,7 +383,8 @@ std::string Log::formatLogLine(LogLevel level) {
       << std::put_time(&tm_val, "%Y/%m/%d %H:%M:%S") << '.' << std::setfill('0')
       << std::setw(6) << microseconds.count() << ' '
       << std::put_time(&tm_val, "%z  ") << g_hostName << ":"
-      << boost::this_process::get_id() << " " << std::this_thread::get_id()
+      << boost::this_process::get_id() << " "
+      << DistributedSystemImpl::getThreadName(std::this_thread::get_id())

Review comment:
       Good catch. gnmsg has been updated to handle either kind of log lines as below:
   
   ```[debug 2022/02/06 07:44:02.450497 -0800  FirstPro:20336 18736] EventId::EventId ...
   [fine 2022/02/06 07:44:02.453857 -0800  FirstPro:20336 18812 (NC ETM Thread)] ExpiryTaskManager thread is running.```




-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode-native] mmartell commented on a change in pull request #918: GEODE-10016: Add map of threadId to threadName

Posted by GitBox <gi...@apache.org>.
mmartell commented on a change in pull request #918:
URL: https://github.com/apache/geode-native/pull/918#discussion_r800935951



##########
File path: cppcache/src/Log.cpp
##########
@@ -382,7 +383,8 @@ std::string Log::formatLogLine(LogLevel level) {
       << std::put_time(&tm_val, "%Y/%m/%d %H:%M:%S") << '.' << std::setfill('0')
       << std::setw(6) << microseconds.count() << ' '
       << std::put_time(&tm_val, "%z  ") << g_hostName << ":"
-      << boost::this_process::get_id() << " " << std::this_thread::get_id()
+      << boost::this_process::get_id() << " "
+      << DistributedSystemImpl::getThreadName(std::this_thread::get_id())

Review comment:
       Good catch. gnmsg has been updated to handle either kind of log lines as below:
   ```[debug 2022/02/06 07:44:02.450497 -0800  FirstPro:20336 18736] EventId::EventId ...
   [fine 2022/02/06 07:44:02.453857 -0800  FirstPro:20336 18812 (NC ETM Thread)] ExpiryTaskManager thread is running.```




-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode-native] mmartell commented on a change in pull request #918: GEODE-10016: Add map of threadId to threadName

Posted by GitBox <gi...@apache.org>.
mmartell commented on a change in pull request #918:
URL: https://github.com/apache/geode-native/pull/918#discussion_r801207149



##########
File path: cppcache/src/Log.cpp
##########
@@ -369,6 +372,46 @@ LogLevel Log::charsToLevel(const std::string& chars) {
   }
 }
 
+void Log::setThreadName(const std::string& threadName) {

Review comment:
       Need to leave this alone because of the try/except block. If string is not const we get this error:
   
   Error	C2712	Cannot use __try in functions that require object unwinding	apache-geode-static	C:\temp\geode-native\cppcache\src\Log.cpp	413




-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode-native] mmartell commented on a change in pull request #918: GEODE-10016: Add map of threadId to threadName

Posted by GitBox <gi...@apache.org>.
mmartell commented on a change in pull request #918:
URL: https://github.com/apache/geode-native/pull/918#discussion_r801202748



##########
File path: cppcache/src/SystemProperties.cpp
##########
@@ -169,7 +169,8 @@ SystemProperties::SystemProperties(
       m_sslTrustStore(DefaultSslTrustStore),
       m_sslKeystorePassword(DefaultSslKeystorePassword),
       m_conflateEvents(DefaultConflateEvents),
-      m_threadPoolSize(DefaultThreadPoolSize),
+      // m_threadPoolSize(DefaultThreadPoolSize),
+      m_threadPoolSize(16),

Review comment:
       Good catch. Yup that was for investigating impact of threadLocals on TSSDataOutput buffers. Reverted.




-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode-native] mmartell commented on a change in pull request #918: GEODE-10016: Add map of threadId to threadName

Posted by GitBox <gi...@apache.org>.
mmartell commented on a change in pull request #918:
URL: https://github.com/apache/geode-native/pull/918#discussion_r801200713



##########
File path: cppcache/src/ExpiryTaskManager.cpp
##########
@@ -51,7 +51,7 @@ void ExpiryTaskManager::start() {
   auto start_future = start_promise.get_future();
   runner_ = std::thread{[this, &start_promise] {
     start_promise.set_value(true);
-    DistributedSystemImpl::setThreadName(NC_ETM_Thread);
+    Log::setThreadName(NC_ETM_Thread);

Review comment:
       Done




-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode-native] mmartell commented on a change in pull request #918: GEODE-10016: Add map of threadId to threadName

Posted by GitBox <gi...@apache.org>.
mmartell commented on a change in pull request #918:
URL: https://github.com/apache/geode-native/pull/918#discussion_r801200642



##########
File path: cppcache/src/DistributedSystemImpl.hpp
##########
@@ -24,6 +24,7 @@
 #include <memory>
 #include <mutex>
 #include <string>
+#include <thread>

Review comment:
       Done




-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode-native] mmartell commented on a change in pull request #918: GEODE-10016: Add map of threadId to threadName

Posted by GitBox <gi...@apache.org>.
mmartell commented on a change in pull request #918:
URL: https://github.com/apache/geode-native/pull/918#discussion_r800935951



##########
File path: cppcache/src/Log.cpp
##########
@@ -382,7 +383,8 @@ std::string Log::formatLogLine(LogLevel level) {
       << std::put_time(&tm_val, "%Y/%m/%d %H:%M:%S") << '.' << std::setfill('0')
       << std::setw(6) << microseconds.count() << ' '
       << std::put_time(&tm_val, "%z  ") << g_hostName << ":"
-      << boost::this_process::get_id() << " " << std::this_thread::get_id()
+      << boost::this_process::get_id() << " "
+      << DistributedSystemImpl::getThreadName(std::this_thread::get_id())

Review comment:
       Good catch. gnmsg has been updated to handle either kind of log lines as below:
   
        [debug 2022/02/06 07:44:02.450497 -0800  FirstPro:20336 18736] EventId::EventId ...
        [fine 2022/02/06 07:44:02.453857 -0800  FirstPro:20336 18812 (NC ETM Thread)] ExpiryTaskManager thread is running.




-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode-native] pivotal-jbarrett commented on a change in pull request #918: GEODE-10016: Add map of threadId to threadName

Posted by GitBox <gi...@apache.org>.
pivotal-jbarrett commented on a change in pull request #918:
URL: https://github.com/apache/geode-native/pull/918#discussion_r799169581



##########
File path: cppcache/src/DistributedSystemImpl.cpp
##########
@@ -38,6 +38,10 @@
 #include "util/Log.hpp"
 #include "version.h"
 
+namespace {
+std::map<std::thread::id, std::string> m_threadNames;

Review comment:
       Does not conform to naming convention.




-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode-native] mmartell commented on a change in pull request #918: GEODE-10016: Add map of threadId to threadName

Posted by GitBox <gi...@apache.org>.
mmartell commented on a change in pull request #918:
URL: https://github.com/apache/geode-native/pull/918#discussion_r800968844



##########
File path: cppcache/src/DistributedSystemImpl.cpp
##########
@@ -143,11 +147,24 @@ void DistributedSystemImpl::unregisterCliCallback(int appdomainId) {
   }
 }
 
-void DistributedSystemImpl::setThreadName(const std::string& threadName) {
+std::string DistributedSystemImpl::getThreadName(std::thread::id id) {
+  std::string threadName = m_threadNames[id];

Review comment:
       Good catch!




-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode-native] mmartell commented on a change in pull request #918: GEODE-10016: Add map of threadId to threadName

Posted by GitBox <gi...@apache.org>.
mmartell commented on a change in pull request #918:
URL: https://github.com/apache/geode-native/pull/918#discussion_r800104791



##########
File path: cppcache/src/DistributedSystemImpl.cpp
##########
@@ -143,11 +147,24 @@ void DistributedSystemImpl::unregisterCliCallback(int appdomainId) {
   }
 }
 
-void DistributedSystemImpl::setThreadName(const std::string& threadName) {
+std::string DistributedSystemImpl::getThreadName(std::thread::id id) {

Review comment:
       Good catch! You're right. Don't need to pass in threadId.




-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode-native] mmartell commented on a change in pull request #918: GEODE-10016: Add map of threadId to threadName

Posted by GitBox <gi...@apache.org>.
mmartell commented on a change in pull request #918:
URL: https://github.com/apache/geode-native/pull/918#discussion_r801126305



##########
File path: cppcache/src/Log.cpp
##########
@@ -382,7 +383,8 @@ std::string Log::formatLogLine(LogLevel level) {
       << std::put_time(&tm_val, "%Y/%m/%d %H:%M:%S") << '.' << std::setfill('0')
       << std::setw(6) << microseconds.count() << ' '
       << std::put_time(&tm_val, "%z  ") << g_hostName << ":"
-      << boost::this_process::get_id() << " " << std::this_thread::get_id()
+      << boost::this_process::get_id() << " "
+      << DistributedSystemImpl::getThreadName(std::this_thread::get_id())

Review comment:
       Yah this is a perfect case threadlocal storage. Just required moving the threadName and its accessors to the Log class. Which makes more sense anyway since logging is the only use of threadName.




-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode-native] mmartell commented on a change in pull request #918: GEODE-10016: Add map of threadId to threadName

Posted by GitBox <gi...@apache.org>.
mmartell commented on a change in pull request #918:
URL: https://github.com/apache/geode-native/pull/918#discussion_r799580487



##########
File path: cppcache/src/DistributedSystemImpl.cpp
##########
@@ -38,6 +38,10 @@
 #include "util/Log.hpp"
 #include "version.h"
 
+namespace {
+std::map<std::thread::id, std::string> m_threadNames;

Review comment:
       I'm using this anonymous namespace to solve an apparent linking issue. Without it, this map is an unresolved reference in the clicache. Do you know of another way to solve that?




-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode-native] mmartell merged pull request #918: GEODE-10016: Add map of threadId to threadName

Posted by GitBox <gi...@apache.org>.
mmartell merged pull request #918:
URL: https://github.com/apache/geode-native/pull/918


   


-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode-native] pivotal-jbarrett commented on a change in pull request #918: GEODE-10016: Add map of threadId to threadName

Posted by GitBox <gi...@apache.org>.
pivotal-jbarrett commented on a change in pull request #918:
URL: https://github.com/apache/geode-native/pull/918#discussion_r801145569



##########
File path: cppcache/src/ExpiryTaskManager.cpp
##########
@@ -51,7 +51,7 @@ void ExpiryTaskManager::start() {
   auto start_future = start_promise.get_future();
   runner_ = std::thread{[this, &start_promise] {
     start_promise.set_value(true);
-    DistributedSystemImpl::setThreadName(NC_ETM_Thread);
+    Log::setThreadName(NC_ETM_Thread);

Review comment:
       Can we just inline the string constants, they aren't used anywhere else.

##########
File path: cppcache/src/SystemProperties.cpp
##########
@@ -169,7 +169,8 @@ SystemProperties::SystemProperties(
       m_sslTrustStore(DefaultSslTrustStore),
       m_sslKeystorePassword(DefaultSslKeystorePassword),
       m_conflateEvents(DefaultConflateEvents),
-      m_threadPoolSize(DefaultThreadPoolSize),
+      // m_threadPoolSize(DefaultThreadPoolSize),
+      m_threadPoolSize(16),

Review comment:
       Looks like an unintentional change!

##########
File path: cppcache/src/Log.cpp
##########
@@ -369,6 +372,46 @@ LogLevel Log::charsToLevel(const std::string& chars) {
   }
 }
 
+void Log::setThreadName(const std::string& threadName) {

Review comment:
       Change signature to `std::string threadName` then `std::move` into `g_threadName`.

##########
File path: cppcache/src/Log.cpp
##########
@@ -38,6 +38,7 @@
 #include <geode/ExceptionTypes.hpp>
 #include <geode/util/LogLevel.hpp>
 
+#include "DistributedSystemImpl.hpp"

Review comment:
       You can drop this too I think.

##########
File path: cppcache/src/Log.cpp
##########
@@ -378,12 +421,21 @@ std::string Log::formatLogLine(LogLevel level) {
       now - std::chrono::system_clock::from_time_t(secs));
   auto tm_val = apache::geode::util::chrono::localtime(secs);
 
+  std::thread::id id = std::this_thread::get_id();

Review comment:
       Why the variable?

##########
File path: cppcache/src/Log.cpp
##########
@@ -60,6 +61,8 @@ static FILE* g_log = nullptr;
 
 static std::string g_hostName;
 
+static thread_local std::string g_threadName;

Review comment:
       See https://google.github.io/styleguide/cppguide.html#thread_local and https://google.github.io/styleguide/cppguide.html#Static_and_Global_Variables for naming guidelines. I think you want `kThreadName` but I am not too keen on that given it isn't really a constant. 

##########
File path: cppcache/src/DistributedSystemImpl.hpp
##########
@@ -24,6 +24,7 @@
 #include <memory>
 #include <mutex>
 #include <string>
+#include <thread>

Review comment:
       Drop this.

##########
File path: cppcache/src/Log.cpp
##########
@@ -378,12 +421,21 @@ std::string Log::formatLogLine(LogLevel level) {
       now - std::chrono::system_clock::from_time_t(secs));
   auto tm_val = apache::geode::util::chrono::localtime(secs);
 
+  std::thread::id id = std::this_thread::get_id();
+  std::stringstream ss;
+  ss << id;
+  std::string threadIdWithName;
+  if (getThreadName() != "") {
+    threadIdWithName = ss.str() + " (" + getThreadName() + ")";
+  } else {
+    threadIdWithName = ss.str();
+  }
+
   msg << "[" << Log::levelToChars(level) << " "
       << std::put_time(&tm_val, "%Y/%m/%d %H:%M:%S") << '.' << std::setfill('0')
       << std::setw(6) << microseconds.count() << ' '
       << std::put_time(&tm_val, "%z  ") << g_hostName << ":"
-      << boost::this_process::get_id() << " " << std::this_thread::get_id()
-      << "] ";
+      << boost::this_process::get_id() << " " << threadIdWithName << "] ";

Review comment:
       just 
   ```c++
   .. << std::this_thread::id << g_threadName << ...
   ```
   Avoid the extra stream creation.




-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode-native] pivotal-jbarrett commented on a change in pull request #918: GEODE-10016: Add map of threadId to threadName

Posted by GitBox <gi...@apache.org>.
pivotal-jbarrett commented on a change in pull request #918:
URL: https://github.com/apache/geode-native/pull/918#discussion_r801232140



##########
File path: cppcache/src/ThreadPool.cpp
##########
@@ -16,20 +16,19 @@
  */
 #include "ThreadPool.hpp"
 
+#include "./util/Log.hpp"

Review comment:
       ./ is redundant here.




-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode-native] mmartell commented on a change in pull request #918: GEODE-10016: Add map of threadId to threadName

Posted by GitBox <gi...@apache.org>.
mmartell commented on a change in pull request #918:
URL: https://github.com/apache/geode-native/pull/918#discussion_r800968284



##########
File path: cppcache/src/ThreadPool.cpp
##########
@@ -16,6 +16,8 @@
  */
 #include "ThreadPool.hpp"
 
+#include <sstream>

Review comment:
       You're right. Not needed.




-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode-native] pivotal-jbarrett commented on a change in pull request #918: GEODE-10016: Add map of threadId to threadName

Posted by GitBox <gi...@apache.org>.
pivotal-jbarrett commented on a change in pull request #918:
URL: https://github.com/apache/geode-native/pull/918#discussion_r799170401



##########
File path: cppcache/src/DistributedSystemImpl.cpp
##########
@@ -143,11 +147,24 @@ void DistributedSystemImpl::unregisterCliCallback(int appdomainId) {
   }
 }
 
-void DistributedSystemImpl::setThreadName(const std::string& threadName) {
+std::string DistributedSystemImpl::getThreadName(std::thread::id id) {
+  std::string threadName = m_threadNames[id];

Review comment:
       Not thread safe access to global variable.

##########
File path: cppcache/src/Log.cpp
##########
@@ -382,7 +383,8 @@ std::string Log::formatLogLine(LogLevel level) {
       << std::put_time(&tm_val, "%Y/%m/%d %H:%M:%S") << '.' << std::setfill('0')
       << std::setw(6) << microseconds.count() << ' '
       << std::put_time(&tm_val, "%z  ") << g_hostName << ":"
-      << boost::this_process::get_id() << " " << std::this_thread::get_id()
+      << boost::this_process::get_id() << " "
+      << DistributedSystemImpl::getThreadName(std::this_thread::get_id())

Review comment:
       If the only place we get the name is from within the thread itself when logging then why not store the name in a thread_local? This avoids the synchronization issues mentioned previously.




-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode-native] mmartell commented on a change in pull request #918: GEODE-10016: Add map of threadId to threadName

Posted by GitBox <gi...@apache.org>.
mmartell commented on a change in pull request #918:
URL: https://github.com/apache/geode-native/pull/918#discussion_r800935951



##########
File path: cppcache/src/Log.cpp
##########
@@ -382,7 +383,8 @@ std::string Log::formatLogLine(LogLevel level) {
       << std::put_time(&tm_val, "%Y/%m/%d %H:%M:%S") << '.' << std::setfill('0')
       << std::setw(6) << microseconds.count() << ' '
       << std::put_time(&tm_val, "%z  ") << g_hostName << ":"
-      << boost::this_process::get_id() << " " << std::this_thread::get_id()
+      << boost::this_process::get_id() << " "
+      << DistributedSystemImpl::getThreadName(std::this_thread::get_id())

Review comment:
       Good catch. gnmsg has been updated to handle either kind of log lines as below:
   ```[debug 2022/02/06 07:44:02.450497 -0800  FirstPro:20336 18736] EventId::EventId(000002894033BF60) - doInit=false, reserveSize=0, fullValueAfterDeltaFail=false
   
   [fine 2022/02/06 07:44:02.453857 -0800  FirstPro:20336 18812 (NC ETM Thread)] ExpiryTaskManager thread is running.```




-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode-native] mmartell commented on a change in pull request #918: GEODE-10016: Add map of threadId to threadName

Posted by GitBox <gi...@apache.org>.
mmartell commented on a change in pull request #918:
URL: https://github.com/apache/geode-native/pull/918#discussion_r800935951



##########
File path: cppcache/src/Log.cpp
##########
@@ -382,7 +383,8 @@ std::string Log::formatLogLine(LogLevel level) {
       << std::put_time(&tm_val, "%Y/%m/%d %H:%M:%S") << '.' << std::setfill('0')
       << std::setw(6) << microseconds.count() << ' '
       << std::put_time(&tm_val, "%z  ") << g_hostName << ":"
-      << boost::this_process::get_id() << " " << std::this_thread::get_id()
+      << boost::this_process::get_id() << " "
+      << DistributedSystemImpl::getThreadName(std::this_thread::get_id())

Review comment:
       Good catch. gnmsg has been updated to handle either kind of log lines as below:
   ``` [debug 2022/02/06 07:44:02.450497 -0800  FirstPro:20336 18736] EventId::EventId
   [fine 2022/02/06 07:44:02.453857 -0800  FirstPro:20336 18812 (NC ETM Thread)] ExpiryTaskManager thread is running.```




-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode-native] mmartell commented on a change in pull request #918: GEODE-10016: Add map of threadId to threadName

Posted by GitBox <gi...@apache.org>.
mmartell commented on a change in pull request #918:
URL: https://github.com/apache/geode-native/pull/918#discussion_r801150167



##########
File path: cppcache/src/DistributedSystemImpl.cpp
##########
@@ -143,11 +148,26 @@ void DistributedSystemImpl::unregisterCliCallback(int appdomainId) {
   }
 }
 
+std::string DistributedSystemImpl::getThreadName() {
+  std::thread::id id = std::this_thread::get_id();
+  std::stringstream ss;
+  ss << id;
+  std::string threadName;
+  if (g_threadNames[id] != "") {
+    threadName = ss.str() + " (" + g_threadNames[id] + ")";
+  } else {
+    threadName = ss.str();
+  }
+  return threadName;

Review comment:
       I moved this logic to the formatLogLine function. Should be fine since most work is on the main thread has no name.




-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode-native] mmartell commented on a change in pull request #918: GEODE-10016: Add map of threadId to threadName

Posted by GitBox <gi...@apache.org>.
mmartell commented on a change in pull request #918:
URL: https://github.com/apache/geode-native/pull/918#discussion_r801146648



##########
File path: cppcache/src/DistributedSystemImpl.cpp
##########
@@ -143,11 +148,26 @@ void DistributedSystemImpl::unregisterCliCallback(int appdomainId) {
   }
 }
 
+std::string DistributedSystemImpl::getThreadName() {
+  std::thread::id id = std::this_thread::get_id();
+  std::stringstream ss;
+  ss << id;
+  std::string threadName;
+  if (g_threadNames[id] != "") {

Review comment:
       Using thread_local now.




-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode-native] mmartell commented on a change in pull request #918: GEODE-10016: Add map of threadId to threadName

Posted by GitBox <gi...@apache.org>.
mmartell commented on a change in pull request #918:
URL: https://github.com/apache/geode-native/pull/918#discussion_r801145931



##########
File path: cppcache/src/DistributedSystemImpl.cpp
##########
@@ -143,11 +148,26 @@ void DistributedSystemImpl::unregisterCliCallback(int appdomainId) {
   }
 }
 
+std::string DistributedSystemImpl::getThreadName() {

Review comment:
       Done.




-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode-native] mmartell commented on a change in pull request #918: GEODE-10016: Add map of threadId to threadName

Posted by GitBox <gi...@apache.org>.
mmartell commented on a change in pull request #918:
URL: https://github.com/apache/geode-native/pull/918#discussion_r801201189



##########
File path: cppcache/src/Log.cpp
##########
@@ -60,6 +61,8 @@ static FILE* g_log = nullptr;
 
 static std::string g_hostName;
 
+static thread_local std::string g_threadName;

Review comment:
       Leaving this one to be consistent with name of several other variables in this file.




-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode-native] mmartell commented on a change in pull request #918: GEODE-10016: Add map of threadId to threadName

Posted by GitBox <gi...@apache.org>.
mmartell commented on a change in pull request #918:
URL: https://github.com/apache/geode-native/pull/918#discussion_r800104808



##########
File path: cppcache/src/DistributedSystemImpl.cpp
##########
@@ -143,11 +147,24 @@ void DistributedSystemImpl::unregisterCliCallback(int appdomainId) {
   }
 }
 
-void DistributedSystemImpl::setThreadName(const std::string& threadName) {
+std::string DistributedSystemImpl::getThreadName(std::thread::id id) {
+  std::string threadName = m_threadNames[id];
+  if (threadName == "") {
+    std::stringstream ss;
+    ss << id;
+    threadName = ss.str();
+  }
+  return threadName;
+}
+
+void DistributedSystemImpl::setThreadName(const std::string& threadName,

Review comment:
       Good catch.




-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode-native] pivotal-jbarrett commented on a change in pull request #918: GEODE-10016: Add map of threadId to threadName

Posted by GitBox <gi...@apache.org>.
pivotal-jbarrett commented on a change in pull request #918:
URL: https://github.com/apache/geode-native/pull/918#discussion_r800946679



##########
File path: cppcache/src/DistributedSystemImpl.cpp
##########
@@ -143,11 +148,26 @@ void DistributedSystemImpl::unregisterCliCallback(int appdomainId) {
   }
 }
 
+std::string DistributedSystemImpl::getThreadName() {

Review comment:
       You can use `const std::string&` once you use the `thread_local` implementation.

##########
File path: cppcache/src/DistributedSystemImpl.cpp
##########
@@ -38,6 +38,10 @@
 #include "util/Log.hpp"
 #include "version.h"
 
+namespace {
+std::map<std::thread::id, std::string> g_threadNames;

Review comment:
       Replace with `static thread_local` member on `DistributedSystemImpl`:
   
   Header:
   ```c++
   class DistributedSystemImpl {
   ...
   private:
     static thread_local std::string threadName_;
   }
   ```
   Implementation:
   ```c++
   std::string initThreadName() {
     std::stringstream ss;
     ss << std::this_thread::get_id();
     return ss.str();
   }
   thread_local std::string DistributedSystemImpl::threadName_ = initThreadName();
   
   const std::string& DistributedSystemImpl::getThreadName() {
     return threadName_;
   }
   
   void DistributedSystemImpl::setThreadName(const std::string& threadName) {
     std::stringstream ss;
     ss << std::this_thread::get_id() << " (" << threadName << ")";
     threadName_ = ss.str();
   }
   ```
   
   I question the usefulness of putting the id in the threadName since when logging we include both `std::thread::id` and this thread name already. I suggest removing it which also simplifies the implementation.
   
   Default thread name is `""`, empty string.
   ```c++
   thread_local std::string DistributedSystemImpl::threadName_;
   
   const std::string& DistributedSystemImpl::getThreadName() {
     return threadName_;
   }
   
   void DistributedSystemImpl::setThreadName(std::string threadName) {
     threadName_ = std::move(threadName);
   }
   ```
   

##########
File path: cppcache/src/DistributedSystemImpl.cpp
##########
@@ -143,11 +148,26 @@ void DistributedSystemImpl::unregisterCliCallback(int appdomainId) {
   }
 }
 
+std::string DistributedSystemImpl::getThreadName() {
+  std::thread::id id = std::this_thread::get_id();
+  std::stringstream ss;
+  ss << id;
+  std::string threadName;
+  if (g_threadNames[id] != "") {

Review comment:
       This is not a thread safe access to a non-thread safe data structure. There is actually no reason to use a map with thread ID as key since that is pretty much what `thread_local` is.

##########
File path: cppcache/src/DistributedSystemImpl.hpp
##########
@@ -48,6 +49,7 @@ using CliCallbackMethod = std::function<void(Cache&)>;
 class DistributedSystemImpl {
  public:
   static void setThreadName(const std::string& threadName);

Review comment:
       Maybe none of this should be on `DistributedSystemImpl`. It has nothing to do with the Distributed System. Perhaps, since this is only used for logging, it should be in the logging classes somewhere. It could also be its own class/struct or just some global name spaced values and functions.
   
   

##########
File path: cppcache/src/DistributedSystemImpl.cpp
##########
@@ -143,11 +148,26 @@ void DistributedSystemImpl::unregisterCliCallback(int appdomainId) {
   }
 }
 
+std::string DistributedSystemImpl::getThreadName() {
+  std::thread::id id = std::this_thread::get_id();
+  std::stringstream ss;
+  ss << id;
+  std::string threadName;
+  if (g_threadNames[id] != "") {
+    threadName = ss.str() + " (" + g_threadNames[id] + ")";
+  } else {
+    threadName = ss.str();
+  }
+  return threadName;

Review comment:
       Augmenting the thread name on each all is expensive for an output that will effectively be constant. If the threadName needs to be augmented, though I argue it doesn't, then it should be done on the less frequently executed `setThreadName()` call.




-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode-native] mmartell commented on a change in pull request #918: GEODE-10016: Add map of threadId to threadName

Posted by GitBox <gi...@apache.org>.
mmartell commented on a change in pull request #918:
URL: https://github.com/apache/geode-native/pull/918#discussion_r800935951



##########
File path: cppcache/src/Log.cpp
##########
@@ -382,7 +383,8 @@ std::string Log::formatLogLine(LogLevel level) {
       << std::put_time(&tm_val, "%Y/%m/%d %H:%M:%S") << '.' << std::setfill('0')
       << std::setw(6) << microseconds.count() << ' '
       << std::put_time(&tm_val, "%z  ") << g_hostName << ":"
-      << boost::this_process::get_id() << " " << std::this_thread::get_id()
+      << boost::this_process::get_id() << " "
+      << DistributedSystemImpl::getThreadName(std::this_thread::get_id())

Review comment:
       Good catch. gnmsg has been updated to handle either kind of log lines as below:
   ```[debug 2022/02/06 07:44:02.450497 -0800  FirstPro:20336 18736] EventId::EventId(000002894033BF60) - doInit=false, reserveSize=0, fullValueAfterDeltaFail=false
   [fine 2022/02/06 07:44:02.453857 -0800  FirstPro:20336 18812 (NC ETM Thread)] ExpiryTaskManager thread is running.```




-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode-native] mmartell commented on a change in pull request #918: GEODE-10016: Add map of threadId to threadName

Posted by GitBox <gi...@apache.org>.
mmartell commented on a change in pull request #918:
URL: https://github.com/apache/geode-native/pull/918#discussion_r801148351



##########
File path: cppcache/src/DistributedSystemImpl.cpp
##########
@@ -38,6 +38,10 @@
 #include "util/Log.hpp"
 #include "version.h"
 
+namespace {
+std::map<std::thread::id, std::string> g_threadNames;

Review comment:
       Can't acually use thread_local in DistributedSystemImpl anyway since it's used my clicache. C++/CLI used to support [System::ThreadLocalAttribute] like C#. But no longer supported.




-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org