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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
