sureshsubbiah commented on a change in pull request #1821: [TRAFODION-3291] Fix
core when multi-column stats are done on lots of columns
URL: https://github.com/apache/trafodion/pull/1821#discussion_r272291489
##########
File path: core/sql/ustat/hs_log.h
##########
@@ -277,5 +283,8 @@ class HSLogMan
timeval prevTime_;
char title_[MAX_TIMING_EVENTS][1000];
static THREAD_P HSLogMan* instance_; /* 1 and only 1 instance
*/
+
+ // intended as a long-term replacement for msg + Log()
+ NAString logStreamBuffer_;
Review comment:
I like this change very much. Using an NAString seems better than using
fixed length buffers. I do wonder if this NAString needs to be allocated on a
partcular heap. I think the default, is to use system heap. Since this class is
a singleton maybe the context heap is a better place for it, allowing heap
usage to traced using executor counters. I am not sure if the heap change is
necessary or even beneficial when considering all aspects such as multiple
Contexts being present in situations such as T2 and repository threads. Thread
safety of this variable when we have multiple T2 connections doing update stats
on different tables from the same process may also be an interesting test.
These are minor concerns. The approach and change are a resounding +1 for me.
----------------------------------------------------------------
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:
[email protected]
With regards,
Apache Git Services