impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Attila Jeges (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-2716: Hive/Impala incompatibility for timestamp data in Parquet
Date Wed, 26 Apr 2017 16:25:58 GMT
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 <attilaj@cloudera.com>
Gerrit-Reviewer: Alex Behm <alex.behm@cloudera.com>
Gerrit-Reviewer: Attila Jeges <attilaj@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhecht@cloudera.com>
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Taras Bobrovytsky <tbobrovytsky@cloudera.com>
Gerrit-Reviewer: Zoltan Ivanfi <zi+gerrit@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message