Attila Jeges has posted comments on this change.

Change subject: IMPALA-2716: Hive/Impala incompatibility for timestamp data in 
Parquet
......................................................................


Patch Set 8:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5939/7/be/src/runtime/timestamp-value.cc
File be/src/runtime/timestamp-value.cc:

Line 123:   ToPtime(&temp);
> I seem to remember there also being a lock in the boost code related to loc
I've investigated the history of UtcToLocal():

- UtcToLocal() was first introduced in IMPALA-1658 (87b9fac2adda). This version 
of UtcToLocal() already used localtime_r() for UTC -> local time conversion. It 
also used couple boost functions to convert back and forth between 'ptime' and 
'struct tm' types (to_tm(), ptime_from_tm(), nanoseconds())

- The first version of UtcToLocal() made querying tables with timestamps a lot 
slower (up to 30x times slower). The root cause was a global lock in 
localtime_r(). This is discussed in IMPALA-3316 and IMPALA-2125. Both JIRAs are 
still open.

- UtcToLocal() was made 50% faster by 4ddce7f97880. This change got rid of the 
boost functions (to_tm(), ptime_from_tm(), nanoseconds()) and added the comment 
at L77. The change kept localtime_r() though, so it didn't do anything to solve 
the global lock problem.

I wrote a simple benchmark program to confirm that UtcToLocal()'s performance 
degrades if the number of threads running it in parallel is increased. The 
benchmark program also confirms that FromUtc() does not have this problem.


-- 
To view, visit http://gerrit.cloudera.org:8080/5939
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I3f24525ef45a2814f476bdee76655b30081079d6
Gerrit-PatchSet: 8
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Attila Jeges <atti...@cloudera.com>
Gerrit-Reviewer: Alex Behm <alex.b...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <atti...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhe...@cloudera.com>
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Taras Bobrovytsky <tbobrovyt...@cloudera.com>
Gerrit-Reviewer: Zoltan Ivanfi <zi+ger...@cloudera.com>
Gerrit-HasComments: Yes

Reply via email to