impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Dan Hecht (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-5142 EventSequence displays negative elapsed time.
Date Wed, 25 Oct 2017 16:46:56 GMT
Dan Hecht has posted comments on this change. ( http://gerrit.cloudera.org:8080/8215 )

Change subject: IMPALA-5142 EventSequence displays negative elapsed time.
......................................................................


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/8215/2/be/src/util/runtime-profile.cc
File be/src/util/runtime-profile.cc:

http://gerrit.cloudera.org:8080/#/c/8215/2/be/src/util/runtime-profile.cc@658
PS2, Line 658:       event_sequence.second->GetEvents(&events);
             :       if (last == 0 && events.size() > 0) last = events.back().second;
that looks like it could be inconsistent with the sorted order


http://gerrit.cloudera.org:8080/#/c/8215/2/be/src/util/runtime-profile.cc@672
PS2, Line 672: 
runtime-profile-test.cc has a test case that expects GetEvents() to be in order. that case
isn't correct if we are saying that GetEvents() returns events unsorted.

Also, the comment for events_ says the events are in sorted order.

I think we should either:
(a) maintain events in sorted order, or
(b) do this sort inside GetEvents() 

either way the property is that GetEvents() always returns events in increasing time order,
which seems like the most straightforward API.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8c944396d96473b17b453da3e913ffc56680a896
Gerrit-Change-Number: 8215
Gerrit-PatchSet: 2
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Dan Hecht <dhecht@cloudera.com>
Gerrit-Reviewer: Pranay Singh
Gerrit-Comment-Date: Wed, 25 Oct 2017 16:46:56 +0000
Gerrit-HasComments: Yes

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