[Impala-ASF-CR] IMPALA-6741: Add timestamp of fragment instance's status updates

2018-12-03 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12000 )

Change subject: IMPALA-6741: Add timestamp of fragment instance's status updates
..


Patch Set 7: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iae3dcddc292d694d7003d10ed0caccfceed7d8fa
Gerrit-Change-Number: 12000
Gerrit-PatchSet: 7
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Zoram Thanga 
Gerrit-Comment-Date: Tue, 04 Dec 2018 01:51:45 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6741: Add timestamp of fragment instance's status updates

2018-12-03 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/12000 )

Change subject: IMPALA-6741: Add timestamp of fragment instance's status updates
..

IMPALA-6741: Add timestamp of fragment instance's status updates

Currently, the profile of a running query doesn't contain any
timestamps for the last updates from the fragment instances.
This makes it hard to differentiate between when a fragment
instance failed to send status reports to the coordinator
for various reasons (e.g. IMPALA-2990) or a truly stuck
fragment instance.

This change adds a timestamp to a fragment instance's profile
to record the time when the coordinator last received a status
update from it. Note that it's possible that there is delay
between when the status was created on the executor and when
it arrived at the coordinator. Given that the clocks are not
necessarily synchronized across all executors, the receiving
time of the update at the coordinator seems easier to make sense of.

Sample output:

Fragment F01:
  Instance 494d948d3235441a:23eae1790001 (host=???):(Total: 15.099ms, 
non-child: 263.951us, % non-child: 1.75%)
Last report received time: 2018-11-27 16:57:30.014
Hdfs split stats (:<# splits>/): 0:1/1.58 KB
Fragment Instance Lifecycle Event Timeline: 15.622ms
   - Prepare Finished: 1.026ms (1.026ms)
   - Open Finished: 1.137ms (110.297us)
   - First Batch Produced: 15.010ms (13.873ms)
   - First Batch Sent: 15.080ms (70.715us)
   - ExecInternal Finished: 15.622ms (541.181us)

Change-Id: Iae3dcddc292d694d7003d10ed0caccfceed7d8fa
Reviewed-on: http://gerrit.cloudera.org:8080/12000
Reviewed-by: Impala Public Jenkins 
Tested-by: Impala Public Jenkins 
---
M be/src/runtime/coordinator-backend-state.cc
M be/src/runtime/coordinator-backend-state.h
M be/src/util/runtime-profile.cc
M be/src/util/runtime-profile.h
M tests/query_test/test_observability.py
5 files changed, 66 insertions(+), 11 deletions(-)

Approvals:
  Impala Public Jenkins: Looks good to me, approved; Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Iae3dcddc292d694d7003d10ed0caccfceed7d8fa
Gerrit-Change-Number: 12000
Gerrit-PatchSet: 8
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Zoram Thanga 


[Impala-ASF-CR] IMPALA-6741: Add timestamp of fragment instance's status updates

2018-12-03 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12000 )

Change subject: IMPALA-6741: Add timestamp of fragment instance's status updates
..


Patch Set 6:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/1517/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iae3dcddc292d694d7003d10ed0caccfceed7d8fa
Gerrit-Change-Number: 12000
Gerrit-PatchSet: 6
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Zoram Thanga 
Gerrit-Comment-Date: Mon, 03 Dec 2018 22:11:10 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6741: Add timestamp of fragment instance's status updates

2018-12-03 Thread Michael Ho (Code Review)
Hello Lars Volker, Balazs Jeszenszky, Zoram Thanga, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-6741: Add timestamp of fragment instance's status updates
..

IMPALA-6741: Add timestamp of fragment instance's status updates

Currently, the profile of a running query doesn't contain any
timestamps for the last updates from the fragment instances.
This makes it hard to differentiate between when a fragment
instance failed to send status reports to the coordinator
for various reasons (e.g. IMPALA-2990) or a truly stuck
fragment instance.

This change adds a timestamp to a fragment instance's profile
to record the time when the coordinator last received a status
update from it. Note that it's possible that there is delay
between when the status was created on the executor and when
it arrived at the coordinator. Given that the clocks are not
necessarily synchronized across all executors, the receiving
time of the update at the coordinator seems easier to make sense of.

Sample output:

Fragment F01:
  Instance 494d948d3235441a:23eae1790001 (host=???):(Total: 15.099ms, 
non-child: 263.951us, % non-child: 1.75%)
Last report received time: 2018-11-27 16:57:30.014
Hdfs split stats (:<# splits>/): 0:1/1.58 KB
Fragment Instance Lifecycle Event Timeline: 15.622ms
   - Prepare Finished: 1.026ms (1.026ms)
   - Open Finished: 1.137ms (110.297us)
   - First Batch Produced: 15.010ms (13.873ms)
   - First Batch Sent: 15.080ms (70.715us)
   - ExecInternal Finished: 15.622ms (541.181us)

Change-Id: Iae3dcddc292d694d7003d10ed0caccfceed7d8fa
---
M be/src/runtime/coordinator-backend-state.cc
M be/src/runtime/coordinator-backend-state.h
M be/src/util/runtime-profile.cc
M be/src/util/runtime-profile.h
M tests/query_test/test_observability.py
5 files changed, 66 insertions(+), 11 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/00/12000/6
--
To view, visit http://gerrit.cloudera.org:8080/12000
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iae3dcddc292d694d7003d10ed0caccfceed7d8fa
Gerrit-Change-Number: 12000
Gerrit-PatchSet: 6
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Zoram Thanga 


[Impala-ASF-CR] IMPALA-6741: Add timestamp of fragment instance's status updates

2018-12-03 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12000 )

Change subject: IMPALA-6741: Add timestamp of fragment instance's status updates
..


Patch Set 7: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iae3dcddc292d694d7003d10ed0caccfceed7d8fa
Gerrit-Change-Number: 12000
Gerrit-PatchSet: 7
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Zoram Thanga 
Gerrit-Comment-Date: Mon, 03 Dec 2018 21:50:46 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6741: Add timestamp of fragment instance's status updates

2018-12-03 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12000 )

Change subject: IMPALA-6741: Add timestamp of fragment instance's status updates
..


Patch Set 7:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3517/ 
DRY_RUN=false


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iae3dcddc292d694d7003d10ed0caccfceed7d8fa
Gerrit-Change-Number: 12000
Gerrit-PatchSet: 7
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Zoram Thanga 
Gerrit-Comment-Date: Mon, 03 Dec 2018 21:50:47 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6741: Add timestamp of fragment instance's status updates

2018-12-03 Thread Michael Ho (Code Review)
Michael Ho has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12000 )

Change subject: IMPALA-6741: Add timestamp of fragment instance's status updates
..


Patch Set 6: Code-Review+2

Carry Lars' +2.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iae3dcddc292d694d7003d10ed0caccfceed7d8fa
Gerrit-Change-Number: 12000
Gerrit-PatchSet: 6
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Zoram Thanga 
Gerrit-Comment-Date: Mon, 03 Dec 2018 21:49:57 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6741: Add timestamp of fragment instance's status updates

2018-12-03 Thread Michael Ho (Code Review)
Michael Ho has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12000 )

Change subject: IMPALA-6741: Add timestamp of fragment instance's status updates
..


Patch Set 5:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/12000/5/tests/query_test/test_observability.py
File tests/query_test/test_observability.py:

http://gerrit.cloudera.org:8080/#/c/12000/5/tests/query_test/test_observability.py@28
PS5, Line 28: =
> flake8: E225 missing whitespace around operator
Done


http://gerrit.cloudera.org:8080/#/c/12000/5/tests/query_test/test_observability.py@28
PS5, Line 28: MAX_WAIT
> Now that this is removed from the code, I suggest naming it something like
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iae3dcddc292d694d7003d10ed0caccfceed7d8fa
Gerrit-Change-Number: 12000
Gerrit-PatchSet: 5
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Zoram Thanga 
Gerrit-Comment-Date: Mon, 03 Dec 2018 21:49:42 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6741: Add timestamp of fragment instance's status updates

2018-12-03 Thread Lars Volker (Code Review)
Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12000 )

Change subject: IMPALA-6741: Add timestamp of fragment instance's status updates
..


Patch Set 5: Code-Review+2

(1 comment)

Had a minor comment but otherwise looks good to me.

http://gerrit.cloudera.org:8080/#/c/12000/5/tests/query_test/test_observability.py
File tests/query_test/test_observability.py:

http://gerrit.cloudera.org:8080/#/c/12000/5/tests/query_test/test_observability.py@28
PS5, Line 28: MAX_WAIT
Now that this is removed from the code, I suggest naming it something like 
"MAX_THRIFT_PROFILE_WAIT_S".



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iae3dcddc292d694d7003d10ed0caccfceed7d8fa
Gerrit-Change-Number: 12000
Gerrit-PatchSet: 5
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Zoram Thanga 
Gerrit-Comment-Date: Mon, 03 Dec 2018 21:13:12 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6741: Add timestamp of fragment instance's status updates

2018-12-03 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12000 )

Change subject: IMPALA-6741: Add timestamp of fragment instance's status updates
..


Patch Set 5:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/1513/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iae3dcddc292d694d7003d10ed0caccfceed7d8fa
Gerrit-Change-Number: 12000
Gerrit-PatchSet: 5
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Zoram Thanga 
Gerrit-Comment-Date: Mon, 03 Dec 2018 20:10:57 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6741: Add timestamp of fragment instance's status updates

2018-12-03 Thread Michael Ho (Code Review)
Michael Ho has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12000 )

Change subject: IMPALA-6741: Add timestamp of fragment instance's status updates
..


Patch Set 5:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/12000/4/tests/query_test/test_observability.py
File tests/query_test/test_observability.py:

http://gerrit.cloudera.org:8080/#/c/12000/4/tests/query_test/test_observability.py@19
PS4, Line 19: from tests.common.impala_cluster import ImpalaCluster
> flake8: F401 'math.ceil' imported but unused
Done


http://gerrit.cloudera.org:8080/#/c/12000/4/tests/query_test/test_observability.py@74
PS4, Line 74:
> The other test below uses 300 seconds here. I suggest to use the same time,
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iae3dcddc292d694d7003d10ed0caccfceed7d8fa
Gerrit-Change-Number: 12000
Gerrit-PatchSet: 5
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Zoram Thanga 
Gerrit-Comment-Date: Mon, 03 Dec 2018 19:38:17 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6741: Add timestamp of fragment instance's status updates

2018-12-03 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12000 )

Change subject: IMPALA-6741: Add timestamp of fragment instance's status updates
..


Patch Set 5:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/12000/5/tests/query_test/test_observability.py
File tests/query_test/test_observability.py:

http://gerrit.cloudera.org:8080/#/c/12000/5/tests/query_test/test_observability.py@28
PS5, Line 28: =
flake8: E225 missing whitespace around operator



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iae3dcddc292d694d7003d10ed0caccfceed7d8fa
Gerrit-Change-Number: 12000
Gerrit-PatchSet: 5
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Zoram Thanga 
Gerrit-Comment-Date: Mon, 03 Dec 2018 19:38:13 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6741: Add timestamp of fragment instance's status updates

2018-12-03 Thread Michael Ho (Code Review)
Hello Lars Volker, Balazs Jeszenszky, Zoram Thanga, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-6741: Add timestamp of fragment instance's status updates
..

IMPALA-6741: Add timestamp of fragment instance's status updates

Currently, the profile of a running query doesn't contain any
timestamps for the last updates from the fragment instances.
This makes it hard to differentiate between when a fragment
instance failed to send status reports to the coordinator
for various reasons (e.g. IMPALA-2990) or a truly stuck
fragment instance.

This change adds a timestamp to a fragment instance's profile
to record the time when the coordinator last received a status
update from it. Note that it's possible that there is delay
between when the status was created on the executor and when
it arrived at the coordinator. Given that the clocks are not
necessarily synchronized across all executors, the receiving
time of the update at the coordinator seems easier to make sense of.

Sample output:

Fragment F01:
  Instance 494d948d3235441a:23eae1790001 (host=???):(Total: 15.099ms, 
non-child: 263.951us, % non-child: 1.75%)
Last report received time: 2018-11-27 16:57:30.014
Hdfs split stats (:<# splits>/): 0:1/1.58 KB
Fragment Instance Lifecycle Event Timeline: 15.622ms
   - Prepare Finished: 1.026ms (1.026ms)
   - Open Finished: 1.137ms (110.297us)
   - First Batch Produced: 15.010ms (13.873ms)
   - First Batch Sent: 15.080ms (70.715us)
   - ExecInternal Finished: 15.622ms (541.181us)

Change-Id: Iae3dcddc292d694d7003d10ed0caccfceed7d8fa
---
M be/src/runtime/coordinator-backend-state.cc
M be/src/runtime/coordinator-backend-state.h
M be/src/util/runtime-profile.cc
M be/src/util/runtime-profile.h
M tests/query_test/test_observability.py
5 files changed, 63 insertions(+), 9 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/00/12000/5
--
To view, visit http://gerrit.cloudera.org:8080/12000
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iae3dcddc292d694d7003d10ed0caccfceed7d8fa
Gerrit-Change-Number: 12000
Gerrit-PatchSet: 5
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Zoram Thanga 


[Impala-ASF-CR] IMPALA-6741: Add timestamp of fragment instance's status updates

2018-12-03 Thread Lars Volker (Code Review)
Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12000 )

Change subject: IMPALA-6741: Add timestamp of fragment instance's status updates
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/12000/4/tests/query_test/test_observability.py
File tests/query_test/test_observability.py:

http://gerrit.cloudera.org:8080/#/c/12000/4/tests/query_test/test_observability.py@74
PS4, Line 74: 30
The other test below uses 300 seconds here. I suggest to use the same time, 
maybe even make it a file-level constant, to reduce the risk of flaky failures 
under load (e.g. IMPALA-6399)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iae3dcddc292d694d7003d10ed0caccfceed7d8fa
Gerrit-Change-Number: 12000
Gerrit-PatchSet: 4
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Zoram Thanga 
Gerrit-Comment-Date: Mon, 03 Dec 2018 16:52:05 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6741: Add timestamp of fragment instance's status updates

2018-11-30 Thread Michael Ho (Code Review)
Michael Ho has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12000 )

Change subject: IMPALA-6741: Add timestamp of fragment instance's status updates
..


Patch Set 4:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/12000/3/be/src/runtime/coordinator-backend-state.cc
File be/src/runtime/coordinator-backend-state.cc:

http://gerrit.cloudera.org:8080/#/c/12000/3/be/src/runtime/coordinator-backend-state.cc@571
PS3, Line 571:
> Maybe clamp this at 0 to make sure we won't return a negative value if some
Done


http://gerrit.cloudera.org:8080/#/c/12000/3/tests/query_test/test_observability.py
File tests/query_test/test_observability.py:

http://gerrit.cloudera.org:8080/#/c/12000/3/tests/query_test/test_observability.py@83
PS3, Line 83: rt_time_str, '%Y-%
> get_thrift_profile takes a timeout, so you can call it once before the loop
Done


http://gerrit.cloudera.org:8080/#/c/12000/3/tests/query_test/test_observability.py@90
PS3, Line 90:
> Can you make this a constant and add a comment. Otherwise this test will fa
Iterate through all the nodes in the new PS.


http://gerrit.cloudera.org:8080/#/c/12000/3/tests/query_test/test_observability.py@108
PS3, Line 108: assert result.exec_summary[scan_idx]['detail'] == 
'functional_kudu.alltypestiny'
> I found this part tricky to read. Why is it 8 seconds? Can you elaborate in
The test in new PS is simplified to not check for monotonicity of the timestamp 
as that's not part of the scope.


http://gerrit.cloudera.org:8080/#/c/12000/3/tests/query_test/test_observability.py@112
PS3, Line 112: result = self.execute_query(quer
> Should we just call close_query() here?
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iae3dcddc292d694d7003d10ed0caccfceed7d8fa
Gerrit-Change-Number: 12000
Gerrit-PatchSet: 4
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Zoram Thanga 
Gerrit-Comment-Date: Sat, 01 Dec 2018 00:01:23 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6741: Add timestamp of fragment instance's status updates

2018-11-30 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12000 )

Change subject: IMPALA-6741: Add timestamp of fragment instance's status updates
..


Patch Set 4:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/1487/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iae3dcddc292d694d7003d10ed0caccfceed7d8fa
Gerrit-Change-Number: 12000
Gerrit-PatchSet: 4
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Zoram Thanga 
Gerrit-Comment-Date: Fri, 30 Nov 2018 23:49:22 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6741: Add timestamp of fragment instance's status updates

2018-11-30 Thread Michael Ho (Code Review)
Hello Lars Volker, Balazs Jeszenszky, Zoram Thanga, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-6741: Add timestamp of fragment instance's status updates
..

IMPALA-6741: Add timestamp of fragment instance's status updates

Currently, the profile of a running query doesn't contain any
timestamps for the last updates from the fragment instances.
This makes it hard to differentiate between when a fragment
instance failed to send status reports to the coordinator
for various reasons (e.g. IMPALA-2990) or a truly stuck
fragment instance.

This change adds a timestamp to a fragment instance's profile
to record the time when the coordinator last received a status
update from it. Note that it's possible that there is delay
between when the status was created on the executor and when
it arrived at the coordinator. Given that the clocks are not
necessarily synchronized across all executors, the receiving
time of the update at the coordinator seems easier to make sense of.

Sample output:

Fragment F01:
  Instance 494d948d3235441a:23eae1790001 (host=???):(Total: 15.099ms, 
non-child: 263.951us, % non-child: 1.75%)
Last report received time: 2018-11-27 16:57:30.014
Hdfs split stats (:<# splits>/): 0:1/1.58 KB
Fragment Instance Lifecycle Event Timeline: 15.622ms
   - Prepare Finished: 1.026ms (1.026ms)
   - Open Finished: 1.137ms (110.297us)
   - First Batch Produced: 15.010ms (13.873ms)
   - First Batch Sent: 15.080ms (70.715us)
   - ExecInternal Finished: 15.622ms (541.181us)

Change-Id: Iae3dcddc292d694d7003d10ed0caccfceed7d8fa
---
M be/src/runtime/coordinator-backend-state.cc
M be/src/runtime/coordinator-backend-state.h
M be/src/util/runtime-profile.cc
M be/src/util/runtime-profile.h
M tests/query_test/test_observability.py
5 files changed, 62 insertions(+), 8 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/00/12000/4
--
To view, visit http://gerrit.cloudera.org:8080/12000
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iae3dcddc292d694d7003d10ed0caccfceed7d8fa
Gerrit-Change-Number: 12000
Gerrit-PatchSet: 4
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Zoram Thanga 


[Impala-ASF-CR] IMPALA-6741: Add timestamp of fragment instance's status updates

2018-11-30 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12000 )

Change subject: IMPALA-6741: Add timestamp of fragment instance's status updates
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/12000/4/tests/query_test/test_observability.py
File tests/query_test/test_observability.py:

http://gerrit.cloudera.org:8080/#/c/12000/4/tests/query_test/test_observability.py@19
PS4, Line 19: from math import ceil
flake8: F401 'math.ceil' imported but unused



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iae3dcddc292d694d7003d10ed0caccfceed7d8fa
Gerrit-Change-Number: 12000
Gerrit-PatchSet: 4
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Zoram Thanga 
Gerrit-Comment-Date: Fri, 30 Nov 2018 23:19:49 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6741: Add timestamp of fragment instance's status updates

2018-11-29 Thread Lars Volker (Code Review)
Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12000 )

Change subject: IMPALA-6741: Add timestamp of fragment instance's status updates
..


Patch Set 3:

(5 comments)

Apologies for being late, I only had some small comments.

http://gerrit.cloudera.org:8080/#/c/12000/3/be/src/runtime/coordinator-backend-state.cc
File be/src/runtime/coordinator-backend-state.cc:

http://gerrit.cloudera.org:8080/#/c/12000/3/be/src/runtime/coordinator-backend-state.cc@571
PS3, Line 571: UnixMillis() - last_report_time_ms_
Maybe clamp this at 0 to make sure we won't return a negative value if someone 
refreshes the page at the wrong time?


http://gerrit.cloudera.org:8080/#/c/12000/3/tests/query_test/test_observability.py
File tests/query_test/test_observability.py:

http://gerrit.cloudera.org:8080/#/c/12000/3/tests/query_test/test_observability.py@83
PS3, Line 83: get_thrift_profile
get_thrift_profile takes a timeout, so you can call it once before the loop to 
wait until the profile shows up.


http://gerrit.cloudera.org:8080/#/c/12000/3/tests/query_test/test_observability.py@90
PS3, Line 90: 2:
Can you make this a constant and add a comment. Otherwise this test will fail 
if someone adds a new profile node in front of them? Alternatively you could 
make the search a bit more generic and iterate over all nodes.


http://gerrit.cloudera.org:8080/#/c/12000/3/tests/query_test/test_observability.py@108
PS3, Line 108: # Minimum NTP poll period is 8 seconds.
I found this part tricky to read. Why is it 8 seconds? Can you elaborate in the 
comment what the intent here is?


http://gerrit.cloudera.org:8080/#/c/12000/3/tests/query_test/test_observability.py@112
PS3, Line 112: self.client.fetch(query, handle)
Should we just call close_query() here?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iae3dcddc292d694d7003d10ed0caccfceed7d8fa
Gerrit-Change-Number: 12000
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Zoram Thanga 
Gerrit-Comment-Date: Fri, 30 Nov 2018 00:58:20 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6741: Add timestamp of fragment instance's status updates

2018-11-29 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12000 )

Change subject: IMPALA-6741: Add timestamp of fragment instance's status updates
..


Patch Set 3:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3507/ 
DRY_RUN=false


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iae3dcddc292d694d7003d10ed0caccfceed7d8fa
Gerrit-Change-Number: 12000
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Zoram Thanga 
Gerrit-Comment-Date: Thu, 29 Nov 2018 23:09:35 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6741: Add timestamp of fragment instance's status updates

2018-11-29 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12000 )

Change subject: IMPALA-6741: Add timestamp of fragment instance's status updates
..


Patch Set 3: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iae3dcddc292d694d7003d10ed0caccfceed7d8fa
Gerrit-Change-Number: 12000
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Zoram Thanga 
Gerrit-Comment-Date: Thu, 29 Nov 2018 23:09:34 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6741: Add timestamp of fragment instance's status updates

2018-11-29 Thread Zoram Thanga (Code Review)
Zoram Thanga has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12000 )

Change subject: IMPALA-6741: Add timestamp of fragment instance's status updates
..


Patch Set 2: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iae3dcddc292d694d7003d10ed0caccfceed7d8fa
Gerrit-Change-Number: 12000
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Zoram Thanga 
Gerrit-Comment-Date: Thu, 29 Nov 2018 19:16:22 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6741: Add timestamp of fragment instance's status updates

2018-11-29 Thread Balazs Jeszenszky (Code Review)
Balazs Jeszenszky has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12000 )

Change subject: IMPALA-6741: Add timestamp of fragment instance's status updates
..


Patch Set 2: Code-Review+1

LGTM


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iae3dcddc292d694d7003d10ed0caccfceed7d8fa
Gerrit-Change-Number: 12000
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Zoram Thanga 
Gerrit-Comment-Date: Thu, 29 Nov 2018 12:52:25 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6741: Add timestamp of fragment instance's status updates

2018-11-28 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12000 )

Change subject: IMPALA-6741: Add timestamp of fragment instance's status updates
..


Patch Set 2:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/1461/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iae3dcddc292d694d7003d10ed0caccfceed7d8fa
Gerrit-Change-Number: 12000
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Zoram Thanga 
Gerrit-Comment-Date: Thu, 29 Nov 2018 00:39:32 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6741: Add timestamp of fragment instance's status updates

2018-11-28 Thread Zoram Thanga (Code Review)
Zoram Thanga has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12000 )

Change subject: IMPALA-6741: Add timestamp of fragment instance's status updates
..


Patch Set 2: Code-Review+1

LGTM.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iae3dcddc292d694d7003d10ed0caccfceed7d8fa
Gerrit-Change-Number: 12000
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Zoram Thanga 
Gerrit-Comment-Date: Thu, 29 Nov 2018 00:07:03 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6741: Add timestamp of fragment instance's status updates

2018-11-28 Thread Michael Ho (Code Review)
Michael Ho has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12000 )

Change subject: IMPALA-6741: Add timestamp of fragment instance's status updates
..


Patch Set 2:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/12000/1/be/src/runtime/coordinator-backend-state.cc
File be/src/runtime/coordinator-backend-state.cc:

http://gerrit.cloudera.org:8080/#/c/12000/1/be/src/runtime/coordinator-backend-state.cc@47
PS1, Line 47: "Last report received time";
> I am nit-picking somewhat, but this is really 'Last received time' from the
How about "Last report received time". "received time" by itself may not be 
very clear on what was received.


http://gerrit.cloudera.org:8080/#/c/12000/1/tests/query_test/test_observability.py
File tests/query_test/test_observability.py:

http://gerrit.cloudera.org:8080/#/c/12000/1/tests/query_test/test_observability.py@95
PS1, Line 95:   if report_time < report_time_dict.get(node.name, 
datetime.min):
> What happens here the first time through, when node.name does not yet exist
The second argument (datetime.min) is the default value if the key doesn't 
exist.


http://gerrit.cloudera.org:8080/#/c/12000/1/tests/query_test/test_observability.py@111
PS1, Line 111: assert num_time_backward <= ceil(elapsed_time / 
MIN_NTP_POLL_PERIOD)
> Should this be num_time_backward? time_backward is a boolean, and shouldn't
Nice catch. Fixed.


http://gerrit.cloudera.org:8080/#/c/12000/1/tests/query_test/test_observability.py@112
PS1, Line 112: s
> flake8: F841 local variable 'results' is assigned to but never used
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iae3dcddc292d694d7003d10ed0caccfceed7d8fa
Gerrit-Change-Number: 12000
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Zoram Thanga 
Gerrit-Comment-Date: Thu, 29 Nov 2018 00:04:39 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6741: Add timestamp of fragment instance's status updates

2018-11-28 Thread Michael Ho (Code Review)
Hello Balazs Jeszenszky, Zoram Thanga, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-6741: Add timestamp of fragment instance's status updates
..

IMPALA-6741: Add timestamp of fragment instance's status updates

Currently, the profile of a running query doesn't contain any
timestamps for the last updates from the fragment instances.
This makes it hard to differentiate between when a fragment
instance failed to send status reports to the coordinator
for various reasons (e.g. IMPALA-2990) or a truly stuck
fragment instance.

This change adds a timestamp to a fragment instance's profile
to record the time when the coordinator last received a status
update from it. Note that it's possible that there is delay
between when the status was created on the executor and when
it arrived at the coordinator. Given that the clocks are not
necessarily synchronized across all executors, the receiving
time of the update at the coordinator seems easier to make sense of.

Sample output:

Fragment F01:
  Instance 494d948d3235441a:23eae1790001 (host=???):(Total: 15.099ms, 
non-child: 263.951us, % non-child: 1.75%)
Last report received time: 2018-11-27 16:57:30.014
Hdfs split stats (:<# splits>/): 0:1/1.58 KB
Fragment Instance Lifecycle Event Timeline: 15.622ms
   - Prepare Finished: 1.026ms (1.026ms)
   - Open Finished: 1.137ms (110.297us)
   - First Batch Produced: 15.010ms (13.873ms)
   - First Batch Sent: 15.080ms (70.715us)
   - ExecInternal Finished: 15.622ms (541.181us)

Change-Id: Iae3dcddc292d694d7003d10ed0caccfceed7d8fa
---
M be/src/runtime/coordinator-backend-state.cc
M be/src/runtime/coordinator-backend-state.h
M be/src/util/runtime-profile.cc
M be/src/util/runtime-profile.h
M tests/query_test/test_observability.py
5 files changed, 85 insertions(+), 8 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/00/12000/2
--
To view, visit http://gerrit.cloudera.org:8080/12000
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iae3dcddc292d694d7003d10ed0caccfceed7d8fa
Gerrit-Change-Number: 12000
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Zoram Thanga 


[Impala-ASF-CR] IMPALA-6741: Add timestamp of fragment instance's status updates

2018-11-28 Thread Zoram Thanga (Code Review)
Zoram Thanga has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12000 )

Change subject: IMPALA-6741: Add timestamp of fragment instance's status updates
..


Patch Set 1:

(3 comments)

Thanks for doing this.

http://gerrit.cloudera.org:8080/#/c/12000/1/be/src/runtime/coordinator-backend-state.cc
File be/src/runtime/coordinator-backend-state.cc:

http://gerrit.cloudera.org:8080/#/c/12000/1/be/src/runtime/coordinator-backend-state.cc@47
PS1, Line 47: "Last report time";
I am nit-picking somewhat, but this is really 'Last received time' from the 
coordinator's PoV, right? I wonder if it might be better to rename it as such, 
so that we don't have to explain what the metric means. I don't feel too 
strongly about this, so I leave it to you.


http://gerrit.cloudera.org:8080/#/c/12000/1/tests/query_test/test_observability.py
File tests/query_test/test_observability.py:

http://gerrit.cloudera.org:8080/#/c/12000/1/tests/query_test/test_observability.py@95
PS1, Line 95:   if report_time < report_time_dict.get(node.name, 
datetime.min):
What happens here the first time through, when node.name does not yet exist in 
the dict?


http://gerrit.cloudera.org:8080/#/c/12000/1/tests/query_test/test_observability.py@111
PS1, Line 111: assert time_backward <= ceil(elapsed_time / 
MIN_NTP_POLL_PERIOD)
Should this be num_time_backward? time_backward is a boolean, and shouldn't be 
used outside of the above while loop.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iae3dcddc292d694d7003d10ed0caccfceed7d8fa
Gerrit-Change-Number: 12000
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Zoram Thanga 
Gerrit-Comment-Date: Wed, 28 Nov 2018 21:25:44 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6741: Add timestamp of fragment instance's status updates

2018-11-27 Thread Michael Ho (Code Review)
Michael Ho has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12000 )

Change subject: IMPALA-6741: Add timestamp of fragment instance's status updates
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/12000/1/be/src/runtime/coordinator-backend-state.cc
File be/src/runtime/coordinator-backend-state.cc:

http://gerrit.cloudera.org:8080/#/c/12000/1/be/src/runtime/coordinator-backend-state.cc@502
PS1, Line 502: ToStringFromUnixMillis(last_report_time_ms_)
An alternate approach would be to just store 'last_report_time_ms_' as string 
and convert it using ToStringFromUnixMillis() when exporting the profile as 
Thrift object or when we are pretty printing it.

That said, it's unclear whether the complication is worth it given this is not 
in the critical path of returning query results.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iae3dcddc292d694d7003d10ed0caccfceed7d8fa
Gerrit-Change-Number: 12000
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Zoram Thanga 
Gerrit-Comment-Date: Wed, 28 Nov 2018 01:24:49 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6741: Add timestamp of fragment instance's status updates

2018-11-27 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12000 )

Change subject: IMPALA-6741: Add timestamp of fragment instance's status updates
..


Patch Set 1:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/1449/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iae3dcddc292d694d7003d10ed0caccfceed7d8fa
Gerrit-Change-Number: 12000
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Zoram Thanga 
Gerrit-Comment-Date: Wed, 28 Nov 2018 01:46:35 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6741: Add timestamp of fragment instance's status updates

2018-11-27 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12000 )

Change subject: IMPALA-6741: Add timestamp of fragment instance's status updates
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/12000/1/tests/query_test/test_observability.py
File tests/query_test/test_observability.py:

http://gerrit.cloudera.org:8080/#/c/12000/1/tests/query_test/test_observability.py@112
PS1, Line 112: r
flake8: F841 local variable 'results' is assigned to but never used



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iae3dcddc292d694d7003d10ed0caccfceed7d8fa
Gerrit-Change-Number: 12000
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Wed, 28 Nov 2018 01:07:35 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6741: Add timestamp of fragment instance's status updates

2018-11-27 Thread Michael Ho (Code Review)
Michael Ho has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/12000


Change subject: IMPALA-6741: Add timestamp of fragment instance's status updates
..

IMPALA-6741: Add timestamp of fragment instance's status updates

Currently, the profile of a running query doesn't contain any
timestamps for the last updates from the fragment instances.
This makes it hard to differentiate between when a fragment
instance failed to send status reports to the coordinator
for various reasons (e.g. IMPALA-2990) or a truly stuck
fragment instance.

This change adds a timestamp to a fragment instance's profile
to record the time when the coordinator last received a status
update from it. Note that it's possible that there is delay
between when the status was created on the executor and when
it arrived at the coordinator. Given that the clocks are not
necessarily synchronized across all executors, the receiving
time of the update at the coordinator seems easier to make sense of.

Sample output:

Fragment F01:
  Instance 494d948d3235441a:23eae1790001 (host=???):(Total: 15.099ms, 
non-child: 263.951us, % non-child: 1.75%)
Last report time: 2018-11-27 16:57:30.014
Hdfs split stats (:<# splits>/): 0:1/1.58 KB
Fragment Instance Lifecycle Event Timeline: 15.622ms
   - Prepare Finished: 1.026ms (1.026ms)
   - Open Finished: 1.137ms (110.297us)
   - First Batch Produced: 15.010ms (13.873ms)
   - First Batch Sent: 15.080ms (70.715us)
   - ExecInternal Finished: 15.622ms (541.181us)

Change-Id: Iae3dcddc292d694d7003d10ed0caccfceed7d8fa
---
M be/src/runtime/coordinator-backend-state.cc
M be/src/runtime/coordinator-backend-state.h
M be/src/util/runtime-profile.cc
M be/src/util/runtime-profile.h
M tests/query_test/test_observability.py
5 files changed, 85 insertions(+), 8 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/00/12000/1
--
To view, visit http://gerrit.cloudera.org:8080/12000
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Iae3dcddc292d694d7003d10ed0caccfceed7d8fa
Gerrit-Change-Number: 12000
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Ho