Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/9155 )
Change subject: IMPALA-6450: fix EventSequence::Start() ...................................................................... Patch Set 4: (1 comment) > Patch Set 3: Code-Review+2 > > Also not for this patch, but Lars, is there some test we are missing that > would have caught the offset_ omission? We could try adding a flag to spread out / delay the startup of local fragment instances, then start a query with multiple instances on a single worker, then assert that the event sequences of the delayed instances reflect the delay in increased timestamps. However, I'm not sure it's wort the effort. Alternatively, we could add a unit test as part of IMPALA-6469 (see my inline comment). http://gerrit.cloudera.org:8080/#/c/9155/2/be/src/util/runtime-profile-counters.h File be/src/util/runtime-profile-counters.h: http://gerrit.cloudera.org:8080/#/c/9155/2/be/src/util/runtime-profile-counters.h@304 PS2, Line 304: offset_ = MonotonicStopWatch::Now() - start_time_ns; : // TODO: IMPALA-4631: Occasionally we see MonotonicStopWatch::Now() return : // (start_time_ns - 1), even though 'start_time_ns' was obtained using : // MonotonicStopWatch::Now(). : DCHECK_GE(offset_, -1); : sw_.Start(); > Not for this change, but I had to dig around to understand what this offset I created IMPALA-6469 to track this. I don't fully understand how a) solves the issue though, since we don't have a sw_ of which we could take the start time. -- To view, visit http://gerrit.cloudera.org:8080/9155 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I62317149cb8428f7d29e945a538f6cc8dd45342f Gerrit-Change-Number: 9155 Gerrit-PatchSet: 4 Gerrit-Owner: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Reviewer: Dan Hecht <dhe...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker <l...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Reviewer: anujphadke <apha...@cloudera.com> Gerrit-Comment-Date: Thu, 01 Feb 2018 01:09:44 +0000 Gerrit-HasComments: Yes
