You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org> on 2020/06/09 23:13:02 UTC
[Impala-ASF-CR] IMPALA-9840: Fix data race in InternalQueue
Bikramjeet Vig has uploaded this change for review. ( http://gerrit.cloudera.org:8080/16051
Change subject: IMPALA-9840: Fix data race in InternalQueue
......................................................................
IMPALA-9840: Fix data race in InternalQueue
This patch converts the remaining public methods in InternalQueue to
be thread safe. These methods were being used during multi threaded
execution and got flagged by thread sanitizer.
Performance:
Ran single node perf run with TPCH (scale factor 30) on my local to
make sure there was no perf impact.
+----------+-----------------------+---------+------------+------------+----------------+
| Workload | File Format | Avg (s) | Delta(Avg) | GeoMean(s) | Delta(GeoMean) |
+----------+-----------------------+---------+------------+------------+----------------+
| TPCH(30) | parquet / none / none | 6.17 | -1.59% | 4.33 | -0.30% |
+----------+-----------------------+---------+------------+------------+----------------+
Change-Id: Ied72c4573e5d23ba744964c3e8a90851d9c6b31c
---
M be/src/util/internal-queue.h
1 file changed, 18 insertions(+), 8 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/51/16051/2
--
To view, visit http://gerrit.cloudera.org:8080/16051
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ied72c4573e5d23ba744964c3e8a90851d9c6b31c
Gerrit-Change-Number: 16051
Gerrit-PatchSet: 2
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
[Impala-ASF-CR] IMPALA-9840: Fix data race in InternalQueue
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/16051 )
Change subject: IMPALA-9840: Fix data race in InternalQueue
......................................................................
IMPALA-9840: Fix data race in InternalQueue
This patch converts the remaining public methods in InternalQueue to
be thread safe. These methods were being used during multi threaded
execution and got flagged by thread sanitizer.
Performance:
Ran single node perf run with TPCH (scale factor 30) on my local to
make sure there was no perf impact.
+----------+-----------------------+---------+------------+------------+----------------+
| Workload | File Format | Avg (s) | Delta(Avg) | GeoMean(s) | Delta(GeoMean) |
+----------+-----------------------+---------+------------+------------+----------------+
| TPCH(30) | parquet / none / none | 6.17 | -1.59% | 4.33 | -0.30% |
+----------+-----------------------+---------+------------+------------+----------------+
Change-Id: Ied72c4573e5d23ba744964c3e8a90851d9c6b31c
Reviewed-on: http://gerrit.cloudera.org:8080/16051
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/util/internal-queue.h
1 file changed, 18 insertions(+), 8 deletions(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/16051
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ied72c4573e5d23ba744964c3e8a90851d9c6b31c
Gerrit-Change-Number: 16051
Gerrit-PatchSet: 5
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-9840: Fix data race in InternalQueue
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16051 )
Change subject: IMPALA-9840: Fix data race in InternalQueue
......................................................................
Patch Set 2: Code-Review+2
(2 comments)
http://gerrit.cloudera.org:8080/#/c/16051/2/be/src/util/internal-queue.h
File be/src/util/internal-queue.h:
http://gerrit.cloudera.org:8080/#/c/16051/2/be/src/util/internal-queue.h@296
PS2, Line 296: inline int sizeLocked() const { return size_; }
nit: SizeLocked() for C++ code.
http://gerrit.cloudera.org:8080/#/c/16051/2/be/src/util/internal-queue.h@297
PS2, Line 297: inline bool emptyLocked() const { return head_ == nullptr; }
nit: EmptyLocked() or IsEmptyLocked()
--
To view, visit http://gerrit.cloudera.org:8080/16051
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ied72c4573e5d23ba744964c3e8a90851d9c6b31c
Gerrit-Change-Number: 16051
Gerrit-PatchSet: 2
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 09 Jun 2020 23:38:30 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9840: Fix data race in InternalQueue
Posted by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org>.
Hello Sahil Takiar, Tim Armstrong, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/16051
to look at the new patch set (#3).
Change subject: IMPALA-9840: Fix data race in InternalQueue
......................................................................
IMPALA-9840: Fix data race in InternalQueue
This patch converts the remaining public methods in InternalQueue to
be thread safe. These methods were being used during multi threaded
execution and got flagged by thread sanitizer.
Performance:
Ran single node perf run with TPCH (scale factor 30) on my local to
make sure there was no perf impact.
+----------+-----------------------+---------+------------+------------+----------------+
| Workload | File Format | Avg (s) | Delta(Avg) | GeoMean(s) | Delta(GeoMean) |
+----------+-----------------------+---------+------------+------------+----------------+
| TPCH(30) | parquet / none / none | 6.17 | -1.59% | 4.33 | -0.30% |
+----------+-----------------------+---------+------------+------------+----------------+
Change-Id: Ied72c4573e5d23ba744964c3e8a90851d9c6b31c
---
M be/src/util/internal-queue.h
1 file changed, 18 insertions(+), 8 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/51/16051/3
--
To view, visit http://gerrit.cloudera.org:8080/16051
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ied72c4573e5d23ba744964c3e8a90851d9c6b31c
Gerrit-Change-Number: 16051
Gerrit-PatchSet: 3
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-9840: Fix data race in InternalQueue
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16051 )
Change subject: IMPALA-9840: Fix data race in InternalQueue
......................................................................
Patch Set 2:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/6261/ : 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/16051
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ied72c4573e5d23ba744964c3e8a90851d9c6b31c
Gerrit-Change-Number: 16051
Gerrit-PatchSet: 2
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 09 Jun 2020 23:33:34 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9840: Fix data race in InternalQueue
Posted by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org>.
Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/16051 )
Change subject: IMPALA-9840: Fix data race in InternalQueue
......................................................................
Patch Set 3: Code-Review+2
(2 comments)
Carrying over Tim's +2
http://gerrit.cloudera.org:8080/#/c/16051/2/be/src/util/internal-queue.h
File be/src/util/internal-queue.h:
http://gerrit.cloudera.org:8080/#/c/16051/2/be/src/util/internal-queue.h@296
PS2, Line 296: inline int SizeLocked() const { return size_; }
> nit: SizeLocked() for C++ code.
Done
http://gerrit.cloudera.org:8080/#/c/16051/2/be/src/util/internal-queue.h@297
PS2, Line 297: inline bool IsEmptyLocked() const { return head_ == nullptr; }
> nit: EmptyLocked() or IsEmptyLocked()
Done
--
To view, visit http://gerrit.cloudera.org:8080/16051
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ied72c4573e5d23ba744964c3e8a90851d9c6b31c
Gerrit-Change-Number: 16051
Gerrit-PatchSet: 3
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 09 Jun 2020 23:46:35 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9840: Fix data race in InternalQueue
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16051 )
Change subject: IMPALA-9840: Fix data race in InternalQueue
......................................................................
Patch Set 3:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/6262/ : 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/16051
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ied72c4573e5d23ba744964c3e8a90851d9c6b31c
Gerrit-Change-Number: 16051
Gerrit-PatchSet: 3
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 10 Jun 2020 00:30:59 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9840: Fix data race in InternalQueue
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16051 )
Change subject: IMPALA-9840: Fix data race in InternalQueue
......................................................................
Patch Set 4:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5978/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/16051
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ied72c4573e5d23ba744964c3e8a90851d9c6b31c
Gerrit-Change-Number: 16051
Gerrit-PatchSet: 4
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 09 Jun 2020 23:46:59 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9840: Fix data race in InternalQueue
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16051 )
Change subject: IMPALA-9840: Fix data race in InternalQueue
......................................................................
Patch Set 4: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/16051
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ied72c4573e5d23ba744964c3e8a90851d9c6b31c
Gerrit-Change-Number: 16051
Gerrit-PatchSet: 4
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 10 Jun 2020 05:05:05 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9840: Fix data race in InternalQueue
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16051 )
Change subject: IMPALA-9840: Fix data race in InternalQueue
......................................................................
Patch Set 4: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/16051
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ied72c4573e5d23ba744964c3e8a90851d9c6b31c
Gerrit-Change-Number: 16051
Gerrit-PatchSet: 4
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 09 Jun 2020 23:46:58 +0000
Gerrit-HasComments: No