You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Sahil Takiar (Code Review)" <ge...@cloudera.org> on 2020/06/14 23:41:15 UTC
[Impala-ASF-CR] IMPALA-5904: (part 4) Fix more TSAN bugs
Sahil Takiar has uploaded this change for review. ( http://gerrit.cloudera.org:8080/16079
Change subject: IMPALA-5904: (part 4) Fix more TSAN bugs
......................................................................
IMPALA-5904: (part 4) Fix more TSAN bugs
Fixes the following TSAN data races that come up when running custom
cluster tests. The immediate goal is to fix all remaining data races in
custom cluster tests and then enable custom cluster tests in the TSAN
builds. This patch fixes about half of the remaining data races reported
during a TSAN build of custom cluster tests.
--
SUMMARY: ThreadSanitizer: data race util/stopwatch.h:186:9 in impala::MonotonicStopWatch::RunningTime() const
Read of size 8 at 0x7b580000dba8 by thread T342:
#0 impala::MonotonicStopWatch::RunningTime() const util/stopwatch.h:186:9
#1 impala::MonotonicStopWatch::Reset() util/stopwatch.h:136:20
#2 impala::StatestoreSubscriber::Heartbeat(impala::TUniqueId const&) statestore/statestore-subscriber.cc:358:35
Previous write of size 8 at 0x7b580000dba8 by thread T341:
#0 impala::MonotonicStopWatch::Reset() util/stopwatch.h:139:21 (impalad+0x1f744ab)
#1 impala::StatestoreSubscriber::Heartbeat(impala::TUniqueId const&) statestore/statestore-subscriber.cc:358:35
--
SUMMARY: ThreadSanitizer: data race status.h:220:10 in impala::Status::operator=(impala::Status&&)
Write of size 8 at 0x7b50002e01e0 by thread T341 (mutexes: write M17919):
#0 impala::Status::operator=(impala::Status&&) common/status.h:220:10
#1 impala::RuntimeState::SetQueryStatus(std::string const&) runtime/runtime-state.h:250
#2 impala_udf::FunctionContext::SetError(char const*) udf/udf.cc:423:47
Previous read of size 8 at 0x7b50002e01e0 by thread T342:
#0 impala::Status::ok() const common/status.h:236:42
#1 impala::RuntimeState::GetQueryStatus() runtime/runtime-state.h:15
#2 impala::HdfsScanner::CommitRows(int, impala::RowBatch*) exec/hdfs-scanner.cc:218:3
--
SUMMARY: ThreadSanitizer: data race hashtable.h:370:58
Read of size 8 at 0x7b2400091df8 by thread T338 (mutexes: write M106814410723061456):
...
#3 impala::MetricGroup::CMCompatibleCallback() util/metrics.cc:185:40
...
#9 impala::Webserver::RenderUrlWithTemplate() util/webserver.cc:801:3
#10 impala::Webserver::BeginRequestCallback(sq_connection*, sq_request_info*) util/webserver.cc:696:5
Previous write of size 8 at 0x7b2400091df8 by thread T364 (mutexes: write M600803201008047112, write M1046659357959855584):
...
#4 impala::AtomicMetric<(impala::TMetricKind::type)0>* impala::MetricGroup::RegisterMetric<> >() util/metrics.h:366:5
#5 impala::MetricGroup::AddGauge(std::string const&, long, std::string const&) util/metrics.h:384:12
#6 impala::AdmissionController::PoolStats::InitMetrics() scheduling/admission-controller.cc:1714:55
Testing:
* Ran core tests
* Re-ran TSAN tests and made sure issues were resolved
Change-Id: Id4244c9a7f971c96b8b8dc7d5262904a0a4b77c1
---
M be/src/runtime/runtime-state.h
M be/src/statestore/statestore-subscriber.cc
M be/src/statestore/statestore-subscriber.h
M be/src/util/metrics.cc
4 files changed, 25 insertions(+), 14 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/79/16079/1
--
To view, visit http://gerrit.cloudera.org:8080/16079
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Id4244c9a7f971c96b8b8dc7d5262904a0a4b77c1
Gerrit-Change-Number: 16079
Gerrit-PatchSet: 1
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
[Impala-ASF-CR] IMPALA-5904: (part 4) Fix more TSAN bugs
Posted by "Sahil Takiar (Code Review)" <ge...@cloudera.org>.
Sahil Takiar has posted comments on this change. ( http://gerrit.cloudera.org:8080/16079 )
Change subject: IMPALA-5904: (part 4) Fix more TSAN bugs
......................................................................
Patch Set 2:
(1 comment)
Re-ran core tests for good measure. Everything looks good.
http://gerrit.cloudera.org:8080/#/c/16079/2/be/src/runtime/runtime-state.h
File be/src/runtime/runtime-state.h:
http://gerrit.cloudera.org:8080/#/c/16079/2/be/src/runtime/runtime-state.h@155
PS2, Line 155: if (UNLIKELY(is_query_status_ok_.Load())) {
> Isn't this missing a !?
Done
--
To view, visit http://gerrit.cloudera.org:8080/16079
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id4244c9a7f971c96b8b8dc7d5262904a0a4b77c1
Gerrit-Change-Number: 16079
Gerrit-PatchSet: 2
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 19 Jun 2020 15:13:18 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5904: (part 4) Fix more TSAN bugs
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/16079 )
Change subject: IMPALA-5904: (part 4) Fix more TSAN bugs
......................................................................
IMPALA-5904: (part 4) Fix more TSAN bugs
Fixes the following TSAN data races that come up when running custom
cluster tests. The immediate goal is to fix all remaining data races in
custom cluster tests and then enable custom cluster tests in the TSAN
builds. This patch fixes about half of the remaining data races reported
during a TSAN build of custom cluster tests.
SUMMARY: ThreadSanitizer: data race util/stopwatch.h:186:9 in impala::MonotonicStopWatch::RunningTime() const
Read of size 8 at 0x7b580000dba8 by thread T342:
#0 impala::MonotonicStopWatch::RunningTime() const util/stopwatch.h:186:9
#1 impala::MonotonicStopWatch::Reset() util/stopwatch.h:136:20
#2 impala::StatestoreSubscriber::Heartbeat(impala::TUniqueId const&) statestore/statestore-subscriber.cc:358:35
Previous write of size 8 at 0x7b580000dba8 by thread T341:
#0 impala::MonotonicStopWatch::Reset() util/stopwatch.h:139:21 (impalad+0x1f744ab)
#1 impala::StatestoreSubscriber::Heartbeat(impala::TUniqueId const&) statestore/statestore-subscriber.cc:358:35
SUMMARY: ThreadSanitizer: data race status.h:220:10 in impala::Status::operator=(impala::Status&&)
Write of size 8 at 0x7b50002e01e0 by thread T341 (mutexes: write M17919):
#0 impala::Status::operator=(impala::Status&&) common/status.h:220:10
#1 impala::RuntimeState::SetQueryStatus(std::string const&) runtime/runtime-state.h:250
#2 impala_udf::FunctionContext::SetError(char const*) udf/udf.cc:423:47
Previous read of size 8 at 0x7b50002e01e0 by thread T342:
#0 impala::Status::ok() const common/status.h:236:42
#1 impala::RuntimeState::GetQueryStatus() runtime/runtime-state.h:15
#2 impala::HdfsScanner::CommitRows(int, impala::RowBatch*) exec/hdfs-scanner.cc:218:3
SUMMARY: ThreadSanitizer: data race hashtable.h:370:58
Read of size 8 at 0x7b2400091df8 by thread T338 (mutexes: write M106814410723061456):
...
#3 impala::MetricGroup::CMCompatibleCallback() util/metrics.cc:185:40
...
#9 impala::Webserver::RenderUrlWithTemplate() util/webserver.cc:801:3
#10 impala::Webserver::BeginRequestCallback(sq_connection*, sq_request_info*) util/webserver.cc:696:5
Previous write of size 8 at 0x7b2400091df8 by thread T364 (mutexes: write M600803201008047112, write M1046659357959855584):
...
#4 impala::AtomicMetric<(impala::TMetricKind::type)0>* impala::MetricGroup::RegisterMetric<> >() util/metrics.h:366:5
#5 impala::MetricGroup::AddGauge(std::string const&, long, std::string const&) util/metrics.h:384:12
#6 impala::AdmissionController::PoolStats::InitMetrics() scheduling/admission-controller.cc:1714:55
Testing:
* Ran core tests
* Re-ran TSAN tests and made sure issues were resolved
* Ran single_node_perf_run for workload TPC-H scale factor 30;
no regressions detected
+----------+-----------------------+---------+------------+------------+----------------+
| Workload | File Format | Avg (s) | Delta(Avg) | GeoMean(s) | Delta(GeoMean) |
+----------+-----------------------+---------+------------+------------+----------------+
| TPCH(30) | parquet / none / none | 7.36 | -1.77% | 5.01 | -1.61% |
+----------+-----------------------+---------+------------+------------+----------------+
Change-Id: Id4244c9a7f971c96b8b8dc7d5262904a0a4b77c1
Reviewed-on: http://gerrit.cloudera.org:8080/16079
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/runtime/runtime-state.cc
M be/src/runtime/runtime-state.h
M be/src/statestore/statestore-subscriber.cc
M be/src/statestore/statestore-subscriber.h
M be/src/util/metrics.cc
5 files changed, 37 insertions(+), 15 deletions(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/16079
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Id4244c9a7f971c96b8b8dc7d5262904a0a4b77c1
Gerrit-Change-Number: 16079
Gerrit-PatchSet: 5
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-5904: (part 4) Fix more TSAN bugs
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16079 )
Change subject: IMPALA-5904: (part 4) Fix more TSAN bugs
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/6319/ : 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/16079
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id4244c9a7f971c96b8b8dc7d5262904a0a4b77c1
Gerrit-Change-Number: 16079
Gerrit-PatchSet: 1
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Mon, 15 Jun 2020 00:08:26 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5904: (part 4) Fix more TSAN bugs
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16079 )
Change subject: IMPALA-5904: (part 4) Fix more TSAN bugs
......................................................................
Patch Set 2:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/6359/ : 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/16079
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id4244c9a7f971c96b8b8dc7d5262904a0a4b77c1
Gerrit-Change-Number: 16079
Gerrit-PatchSet: 2
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 18 Jun 2020 00:28:39 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5904: (part 4) Fix more TSAN bugs
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16079 )
Change subject: IMPALA-5904: (part 4) Fix more TSAN bugs
......................................................................
Patch Set 3: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/16079
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id4244c9a7f971c96b8b8dc7d5262904a0a4b77c1
Gerrit-Change-Number: 16079
Gerrit-PatchSet: 3
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 19 Jun 2020 15:56:46 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5904: (part 4) Fix more TSAN bugs
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16079 )
Change subject: IMPALA-5904: (part 4) Fix more TSAN bugs
......................................................................
Patch Set 4: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/16079
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id4244c9a7f971c96b8b8dc7d5262904a0a4b77c1
Gerrit-Change-Number: 16079
Gerrit-PatchSet: 4
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 19 Jun 2020 21:03:30 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5904: (part 4) Fix more TSAN bugs
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16079 )
Change subject: IMPALA-5904: (part 4) Fix more TSAN bugs
......................................................................
Patch Set 4: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/16079
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id4244c9a7f971c96b8b8dc7d5262904a0a4b77c1
Gerrit-Change-Number: 16079
Gerrit-PatchSet: 4
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 19 Jun 2020 16:05:37 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5904: (part 4) Fix more TSAN bugs
Posted by "Sahil Takiar (Code Review)" <ge...@cloudera.org>.
Sahil Takiar has posted comments on this change. ( http://gerrit.cloudera.org:8080/16079 )
Change subject: IMPALA-5904: (part 4) Fix more TSAN bugs
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/16079/1/be/src/runtime/runtime-state.h
File be/src/runtime/runtime-state.h:
http://gerrit.cloudera.org:8080/#/c/16079/1/be/src/runtime/runtime-state.h@155
PS1, Line 155: std::lock_guard<SpinLock> l(query_status_lock_);
> I think there might be some perf impact here, since this is called in a bun
Changed it to use an atomic variable. Ran single-node-perf as well. Didn't see any perf regression.
--
To view, visit http://gerrit.cloudera.org:8080/16079
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id4244c9a7f971c96b8b8dc7d5262904a0a4b77c1
Gerrit-Change-Number: 16079
Gerrit-PatchSet: 1
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 18 Jun 2020 00:02:00 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5904: (part 4) Fix more TSAN bugs
Posted by "Sahil Takiar (Code Review)" <ge...@cloudera.org>.
Hello Tim Armstrong, Joe McDonnell, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/16079
to look at the new patch set (#2).
Change subject: IMPALA-5904: (part 4) Fix more TSAN bugs
......................................................................
IMPALA-5904: (part 4) Fix more TSAN bugs
Fixes the following TSAN data races that come up when running custom
cluster tests. The immediate goal is to fix all remaining data races in
custom cluster tests and then enable custom cluster tests in the TSAN
builds. This patch fixes about half of the remaining data races reported
during a TSAN build of custom cluster tests.
SUMMARY: ThreadSanitizer: data race util/stopwatch.h:186:9 in impala::MonotonicStopWatch::RunningTime() const
Read of size 8 at 0x7b580000dba8 by thread T342:
#0 impala::MonotonicStopWatch::RunningTime() const util/stopwatch.h:186:9
#1 impala::MonotonicStopWatch::Reset() util/stopwatch.h:136:20
#2 impala::StatestoreSubscriber::Heartbeat(impala::TUniqueId const&) statestore/statestore-subscriber.cc:358:35
Previous write of size 8 at 0x7b580000dba8 by thread T341:
#0 impala::MonotonicStopWatch::Reset() util/stopwatch.h:139:21 (impalad+0x1f744ab)
#1 impala::StatestoreSubscriber::Heartbeat(impala::TUniqueId const&) statestore/statestore-subscriber.cc:358:35
SUMMARY: ThreadSanitizer: data race status.h:220:10 in impala::Status::operator=(impala::Status&&)
Write of size 8 at 0x7b50002e01e0 by thread T341 (mutexes: write M17919):
#0 impala::Status::operator=(impala::Status&&) common/status.h:220:10
#1 impala::RuntimeState::SetQueryStatus(std::string const&) runtime/runtime-state.h:250
#2 impala_udf::FunctionContext::SetError(char const*) udf/udf.cc:423:47
Previous read of size 8 at 0x7b50002e01e0 by thread T342:
#0 impala::Status::ok() const common/status.h:236:42
#1 impala::RuntimeState::GetQueryStatus() runtime/runtime-state.h:15
#2 impala::HdfsScanner::CommitRows(int, impala::RowBatch*) exec/hdfs-scanner.cc:218:3
SUMMARY: ThreadSanitizer: data race hashtable.h:370:58
Read of size 8 at 0x7b2400091df8 by thread T338 (mutexes: write M106814410723061456):
...
#3 impala::MetricGroup::CMCompatibleCallback() util/metrics.cc:185:40
...
#9 impala::Webserver::RenderUrlWithTemplate() util/webserver.cc:801:3
#10 impala::Webserver::BeginRequestCallback(sq_connection*, sq_request_info*) util/webserver.cc:696:5
Previous write of size 8 at 0x7b2400091df8 by thread T364 (mutexes: write M600803201008047112, write M1046659357959855584):
...
#4 impala::AtomicMetric<(impala::TMetricKind::type)0>* impala::MetricGroup::RegisterMetric<> >() util/metrics.h:366:5
#5 impala::MetricGroup::AddGauge(std::string const&, long, std::string const&) util/metrics.h:384:12
#6 impala::AdmissionController::PoolStats::InitMetrics() scheduling/admission-controller.cc:1714:55
Testing:
* Ran core tests
* Re-ran TSAN tests and made sure issues were resolved
* Ran single_node_perf_run for workload TPC-H scale factor 30;
no regressions detected
+----------+-----------------------+---------+------------+------------+----------------+
| Workload | File Format | Avg (s) | Delta(Avg) | GeoMean(s) | Delta(GeoMean) |
+----------+-----------------------+---------+------------+------------+----------------+
| TPCH(30) | parquet / none / none | 7.36 | -1.77% | 5.01 | -1.61% |
+----------+-----------------------+---------+------------+------------+----------------+
Change-Id: Id4244c9a7f971c96b8b8dc7d5262904a0a4b77c1
---
M be/src/runtime/runtime-state.cc
M be/src/runtime/runtime-state.h
M be/src/statestore/statestore-subscriber.cc
M be/src/statestore/statestore-subscriber.h
M be/src/util/metrics.cc
5 files changed, 37 insertions(+), 15 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/79/16079/2
--
To view, visit http://gerrit.cloudera.org:8080/16079
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id4244c9a7f971c96b8b8dc7d5262904a0a4b77c1
Gerrit-Change-Number: 16079
Gerrit-PatchSet: 2
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-5904: (part 4) Fix more TSAN bugs
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16079 )
Change subject: IMPALA-5904: (part 4) Fix more TSAN bugs
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/16079/2/be/src/runtime/runtime-state.h
File be/src/runtime/runtime-state.h:
http://gerrit.cloudera.org:8080/#/c/16079/2/be/src/runtime/runtime-state.h@155
PS2, Line 155: if (UNLIKELY(is_query_status_ok_.Load())) {
Isn't this missing a !?
--
To view, visit http://gerrit.cloudera.org:8080/16079
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id4244c9a7f971c96b8b8dc7d5262904a0a4b77c1
Gerrit-Change-Number: 16079
Gerrit-PatchSet: 2
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 18 Jun 2020 18:27:47 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5904: (part 4) Fix more TSAN bugs
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16079 )
Change subject: IMPALA-5904: (part 4) Fix more TSAN bugs
......................................................................
Patch Set 3:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/6378/ : 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/16079
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id4244c9a7f971c96b8b8dc7d5262904a0a4b77c1
Gerrit-Change-Number: 16079
Gerrit-PatchSet: 3
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 19 Jun 2020 15:42:03 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5904: (part 4) Fix more TSAN bugs
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16079 )
Change subject: IMPALA-5904: (part 4) Fix more TSAN bugs
......................................................................
Patch Set 4:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/6032/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/16079
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id4244c9a7f971c96b8b8dc7d5262904a0a4b77c1
Gerrit-Change-Number: 16079
Gerrit-PatchSet: 4
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 19 Jun 2020 16:05:38 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5904: (part 4) Fix more TSAN bugs
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16079 )
Change subject: IMPALA-5904: (part 4) Fix more TSAN bugs
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/16079/1/be/src/runtime/runtime-state.h
File be/src/runtime/runtime-state.h:
http://gerrit.cloudera.org:8080/#/c/16079/1/be/src/runtime/runtime-state.h@155
PS1, Line 155: std::lock_guard<SpinLock> l(query_status_lock_);
I think there might be some perf impact here, since this is called in a bunch of loops. I think we either need to validate the perf, or keep the previous pattern, but use an atomic variable for the lock-free check.
--
To view, visit http://gerrit.cloudera.org:8080/16079
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id4244c9a7f971c96b8b8dc7d5262904a0a4b77c1
Gerrit-Change-Number: 16079
Gerrit-PatchSet: 1
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 16 Jun 2020 16:33:41 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5904: (part 4) Fix more TSAN bugs
Posted by "Sahil Takiar (Code Review)" <ge...@cloudera.org>.
Hello Tim Armstrong, Joe McDonnell, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/16079
to look at the new patch set (#3).
Change subject: IMPALA-5904: (part 4) Fix more TSAN bugs
......................................................................
IMPALA-5904: (part 4) Fix more TSAN bugs
Fixes the following TSAN data races that come up when running custom
cluster tests. The immediate goal is to fix all remaining data races in
custom cluster tests and then enable custom cluster tests in the TSAN
builds. This patch fixes about half of the remaining data races reported
during a TSAN build of custom cluster tests.
SUMMARY: ThreadSanitizer: data race util/stopwatch.h:186:9 in impala::MonotonicStopWatch::RunningTime() const
Read of size 8 at 0x7b580000dba8 by thread T342:
#0 impala::MonotonicStopWatch::RunningTime() const util/stopwatch.h:186:9
#1 impala::MonotonicStopWatch::Reset() util/stopwatch.h:136:20
#2 impala::StatestoreSubscriber::Heartbeat(impala::TUniqueId const&) statestore/statestore-subscriber.cc:358:35
Previous write of size 8 at 0x7b580000dba8 by thread T341:
#0 impala::MonotonicStopWatch::Reset() util/stopwatch.h:139:21 (impalad+0x1f744ab)
#1 impala::StatestoreSubscriber::Heartbeat(impala::TUniqueId const&) statestore/statestore-subscriber.cc:358:35
SUMMARY: ThreadSanitizer: data race status.h:220:10 in impala::Status::operator=(impala::Status&&)
Write of size 8 at 0x7b50002e01e0 by thread T341 (mutexes: write M17919):
#0 impala::Status::operator=(impala::Status&&) common/status.h:220:10
#1 impala::RuntimeState::SetQueryStatus(std::string const&) runtime/runtime-state.h:250
#2 impala_udf::FunctionContext::SetError(char const*) udf/udf.cc:423:47
Previous read of size 8 at 0x7b50002e01e0 by thread T342:
#0 impala::Status::ok() const common/status.h:236:42
#1 impala::RuntimeState::GetQueryStatus() runtime/runtime-state.h:15
#2 impala::HdfsScanner::CommitRows(int, impala::RowBatch*) exec/hdfs-scanner.cc:218:3
SUMMARY: ThreadSanitizer: data race hashtable.h:370:58
Read of size 8 at 0x7b2400091df8 by thread T338 (mutexes: write M106814410723061456):
...
#3 impala::MetricGroup::CMCompatibleCallback() util/metrics.cc:185:40
...
#9 impala::Webserver::RenderUrlWithTemplate() util/webserver.cc:801:3
#10 impala::Webserver::BeginRequestCallback(sq_connection*, sq_request_info*) util/webserver.cc:696:5
Previous write of size 8 at 0x7b2400091df8 by thread T364 (mutexes: write M600803201008047112, write M1046659357959855584):
...
#4 impala::AtomicMetric<(impala::TMetricKind::type)0>* impala::MetricGroup::RegisterMetric<> >() util/metrics.h:366:5
#5 impala::MetricGroup::AddGauge(std::string const&, long, std::string const&) util/metrics.h:384:12
#6 impala::AdmissionController::PoolStats::InitMetrics() scheduling/admission-controller.cc:1714:55
Testing:
* Ran core tests
* Re-ran TSAN tests and made sure issues were resolved
* Ran single_node_perf_run for workload TPC-H scale factor 30;
no regressions detected
+----------+-----------------------+---------+------------+------------+----------------+
| Workload | File Format | Avg (s) | Delta(Avg) | GeoMean(s) | Delta(GeoMean) |
+----------+-----------------------+---------+------------+------------+----------------+
| TPCH(30) | parquet / none / none | 7.36 | -1.77% | 5.01 | -1.61% |
+----------+-----------------------+---------+------------+------------+----------------+
Change-Id: Id4244c9a7f971c96b8b8dc7d5262904a0a4b77c1
---
M be/src/runtime/runtime-state.cc
M be/src/runtime/runtime-state.h
M be/src/statestore/statestore-subscriber.cc
M be/src/statestore/statestore-subscriber.h
M be/src/util/metrics.cc
5 files changed, 37 insertions(+), 15 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/79/16079/3
--
To view, visit http://gerrit.cloudera.org:8080/16079
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id4244c9a7f971c96b8b8dc7d5262904a0a4b77c1
Gerrit-Change-Number: 16079
Gerrit-PatchSet: 3
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>