You are viewing a plain text version of this content. The canonical link for it is here.

Posted to reviews@impala.apache.org by "Joe McDonnell (Code Review)" <ge...@cloudera.org> on 2018/12/10 21:12:43 UTC

[Impala-ASF-CR] IMPALA-7946: Use original timeout in THREAD POOL TASK TIMED OUT message

Joe McDonnell has uploaded this change for review. ( http://gerrit.cloudera.org:8080/12062

Change subject: IMPALA-7946: Use original timeout in THREAD_POOL_TASK_TIMED_OUT message
......................................................................

IMPALA-7946: Use original timeout in THREAD_POOL_TASK_TIMED_OUT message

When SynchronousThreadPool::SynchronousOffer() times out, it can
sometimes print the wrong time out in the error message. This happens
because it is enforcing a total timeout across multiple operations.
For example, if there is a total timeout of 5 seconds and the first
step takes 1 second, the remaining step is given a 4 second timeout
to enforce the total timeout. However, this 4 second timeout should
not be expressed in the THREAD_POOL_TASK_TIMED_OUT error message
if the task times out. Instead, SynchronousOffer() should always use
the original timeout as the internal time out is unimportant to users.

This changes the code to make the error message always use the
original timeout.

Change-Id: Ib7bc31f58a8d29abfdc24959dc2730a0ae24ec56
---
M be/src/util/thread-pool.h
1 file changed, 15 insertions(+), 13 deletions(-)

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib7bc31f58a8d29abfdc24959dc2730a0ae24ec56
Gerrit-Change-Number: 12062
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>

[Impala-ASF-CR] IMPALA-7946: Use original timeout in THREAD POOL TASK TIMED OUT message

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.

Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12062 )

Change subject: IMPALA-7946: Use original timeout in THREAD_POOL_TASK_TIMED_OUT message
......................................................................


Patch Set 3:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib7bc31f58a8d29abfdc24959dc2730a0ae24ec56
Gerrit-Change-Number: 12062
Gerrit-PatchSet: 3
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Comment-Date: Thu, 20 Dec 2018 19:37:41 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7946: Use original timeout in THREAD POOL TASK TIMED OUT message

Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.

Joe McDonnell has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/12062 )

Change subject: IMPALA-7946: Use original timeout in THREAD_POOL_TASK_TIMED_OUT message
......................................................................

IMPALA-7946: Use original timeout in THREAD_POOL_TASK_TIMED_OUT message

When SynchronousThreadPool::SynchronousOffer() times out, it can
sometimes print the wrong time out in the error message. This happens
because it is enforcing a total timeout across multiple operations.
For example, if there is a total timeout of 5 seconds and the first
step takes 1 second, the remaining step is given a 4 second timeout
to enforce the total timeout. However, this 4 second timeout should
not be expressed in the THREAD_POOL_TASK_TIMED_OUT error message
if the task times out. Instead, SynchronousOffer() should always use
the original timeout as the internal time out is unimportant to users.

This changes the code to make the error message always use the
original timeout.

Change-Id: Ib7bc31f58a8d29abfdc24959dc2730a0ae24ec56
Reviewed-on: http://gerrit.cloudera.org:8080/12062
Reviewed-by: Joe McDonnell <jo...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/util/thread-pool.h
1 file changed, 15 insertions(+), 13 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ib7bc31f58a8d29abfdc24959dc2730a0ae24ec56
Gerrit-Change-Number: 12062
Gerrit-PatchSet: 4
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>

[Impala-ASF-CR] IMPALA-7946: Use original timeout in THREAD POOL TASK TIMED OUT message

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.

Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12062 )

Change subject: IMPALA-7946: Use original timeout in THREAD_POOL_TASK_TIMED_OUT message
......................................................................


Patch Set 3: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib7bc31f58a8d29abfdc24959dc2730a0ae24ec56
Gerrit-Change-Number: 12062
Gerrit-PatchSet: 3
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Comment-Date: Thu, 20 Dec 2018 23:28:03 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7946: Use original timeout in THREAD POOL TASK TIMED OUT message

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.

Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12062 )

Change subject: IMPALA-7946: Use original timeout in THREAD_POOL_TASK_TIMED_OUT message
......................................................................


Patch Set 1:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/1568/ : 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/12062
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib7bc31f58a8d29abfdc24959dc2730a0ae24ec56
Gerrit-Change-Number: 12062
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Mon, 10 Dec 2018 21:43:01 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7946: Use original timeout in THREAD POOL TASK TIMED OUT message

Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.

Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/12062 )

Change subject: IMPALA-7946: Use original timeout in THREAD_POOL_TASK_TIMED_OUT message
......................................................................


Patch Set 3: Code-Review+2

Rebased, carrying +2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib7bc31f58a8d29abfdc24959dc2730a0ae24ec56
Gerrit-Change-Number: 12062
Gerrit-PatchSet: 3
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Comment-Date: Thu, 20 Dec 2018 19:37:18 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7946: Use original timeout in THREAD POOL TASK TIMED OUT message

Posted by "Philip Zeyliger (Code Review)" <ge...@cloudera.org>.

Philip Zeyliger has posted comments on this change. ( http://gerrit.cloudera.org:8080/12062 )

Change subject: IMPALA-7946: Use original timeout in THREAD_POOL_TASK_TIMED_OUT message
......................................................................


Patch Set 2: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib7bc31f58a8d29abfdc24959dc2730a0ae24ec56
Gerrit-Change-Number: 12062
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Comment-Date: Thu, 20 Dec 2018 18:26:55 +0000
Gerrit-HasComments: No