impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Zoram Thanga (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-6136: Part 1: Query duration should not be normally negative.
Date Wed, 01 Nov 2017 21:21:28 GMT
Hello Michael Ho, Philip Zeyliger, Dan Hecht, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/8430

to look at the new patch set (#3).

Change subject: IMPALA-6136: Part 1: Query duration should not be normally negative.
......................................................................

IMPALA-6136: Part 1: Query duration should not be normally negative.

The second commit for IMPALA-5599, 1640aa97, introduced a regression
where calculating the duration of an in-progress query can result
in negative values. This can happen for two reasons:

1. The value of ClientRequestState::end_time_us_ is not initialized
   in the constructor, and may have some random value until
   ClientRequestState::Done() is called.
2. If ClientRequestState::end_time_us_ happens to have a positive
   value less than ClientRequestState::start_time_us_, the query
   duration will be negative. Here, since the query is still in
   flight, we need to use the local current time as the end time.

The fix has been manually verified by executing long-running queries
and checking the query profiles to ensure the durations are not some
random junks. A new test case will be added to check for sane time
values in a follow-on commit.

Since we are using Unix time (system clock) for time stamps, it is
still possible for end_time_us_ to be less than start_time_us_ if
the system clock gets reset while the query is executing. If there
are clients that assume that a query duration is never negative,
we really should switch to a monotonic clock to time queries.

Change-Id: Ib6f971ebd5f0f00f3e38df0495692ffe9d52ef90
---
M be/src/service/client-request-state.cc
M be/src/service/client-request-state.h
M be/src/service/impala-http-handler.cc
M be/src/service/impala-server.h
4 files changed, 13 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/30/8430/3
-- 
To view, visit http://gerrit.cloudera.org:8080/8430
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib6f971ebd5f0f00f3e38df0495692ffe9d52ef90
Gerrit-Change-Number: 8430
Gerrit-PatchSet: 3
Gerrit-Owner: Zoram Thanga <zoram@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhecht@cloudera.com>
Gerrit-Reviewer: Michael Ho <kwho@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <philip@cloudera.com>
Gerrit-Reviewer: Zoram Thanga <zoram@cloudera.com>

Mime
  • Unnamed multipart/alternative (inline, 8-Bit, 0 bytes)
View raw message