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 2019/04/02 23:12:20 UTC

[Impala-ASF-CR] IMPALA-8360: Fix race conditions in thread-pool-test

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


Change subject: IMPALA-8360: Fix race conditions in thread-pool-test
......................................................................

IMPALA-8360: Fix race conditions in thread-pool-test

There are race conditions in thread-pool-test between the caller
thread and the worker thread. Specifically, in some cases, the
worker thread seems to be slow in freeing resources. This can
lead to asserts failing because the work item has not been
freed or because the submit of a task failed when it should
not have.

This fixes the race conditions:
 - It breaks up the existing SynchronousThreadPoolTest into two
   smaller tests so that the two can't interfere with each other.
 - It adds the ability to sleep and recheck that the work item
   has been freed.

Testing:
 - Ran thread-pool-test in a loop for 20k iterations

Change-Id: Id2d5f8b677e475d8e9d6b4512e990b20bfbefaf1
---
M be/src/util/thread-pool-test.cc
1 file changed, 16 insertions(+), 1 deletion(-)



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

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

[Impala-ASF-CR] IMPALA-8360: Fix race conditions in thread-pool-test

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12916 )

Change subject: IMPALA-8360: Fix race conditions in thread-pool-test
......................................................................


Patch Set 2: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id2d5f8b677e475d8e9d6b4512e990b20bfbefaf1
Gerrit-Change-Number: 12916
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 04 Apr 2019 21:13:26 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8360: Fix race conditions in thread-pool-test

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12916 )

Change subject: IMPALA-8360: Fix race conditions in thread-pool-test
......................................................................


Patch Set 2:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id2d5f8b677e475d8e9d6b4512e990b20bfbefaf1
Gerrit-Change-Number: 12916
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 04 Apr 2019 21:13:27 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8360: Fix race conditions in thread-pool-test

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12916 )

Change subject: IMPALA-8360: Fix race conditions in thread-pool-test
......................................................................


Patch Set 1:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id2d5f8b677e475d8e9d6b4512e990b20bfbefaf1
Gerrit-Change-Number: 12916
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 02 Apr 2019 23:57:06 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8360: Fix race conditions in thread-pool-test

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/12916 )

Change subject: IMPALA-8360: Fix race conditions in thread-pool-test
......................................................................


Patch Set 1: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id2d5f8b677e475d8e9d6b4512e990b20bfbefaf1
Gerrit-Change-Number: 12916
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 04 Apr 2019 20:52:13 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8360: Fix race conditions in thread-pool-test

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/12916 )

Change subject: IMPALA-8360: Fix race conditions in thread-pool-test
......................................................................

IMPALA-8360: Fix race conditions in thread-pool-test

There are race conditions in thread-pool-test between the caller
thread and the worker thread. Specifically, in some cases, the
worker thread seems to be slow in freeing resources. This can
lead to asserts failing because the work item has not been
freed or because the submit of a task failed when it should
not have.

This fixes the race conditions:
 - It breaks up the existing SynchronousThreadPoolTest into two
   smaller tests so that the two can't interfere with each other.
 - It adds the ability to sleep and recheck that the work item
   has been freed.

Testing:
 - Ran thread-pool-test in a loop for 20k iterations

Change-Id: Id2d5f8b677e475d8e9d6b4512e990b20bfbefaf1
Reviewed-on: http://gerrit.cloudera.org:8080/12916
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/util/thread-pool-test.cc
1 file changed, 16 insertions(+), 1 deletion(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Id2d5f8b677e475d8e9d6b4512e990b20bfbefaf1
Gerrit-Change-Number: 12916
Gerrit-PatchSet: 3
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR] IMPALA-8360: Fix race conditions in thread-pool-test

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12916 )

Change subject: IMPALA-8360: Fix race conditions in thread-pool-test
......................................................................


Patch Set 2: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id2d5f8b677e475d8e9d6b4512e990b20bfbefaf1
Gerrit-Change-Number: 12916
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 05 Apr 2019 01:52:47 +0000
Gerrit-HasComments: No