[Impala-ASF-CR] IMPALA-10129 Data race in MemTracker::GetTopNQueriesAndUpdatePoolStats
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/16408 ) Change subject: IMPALA-10129 Data race in MemTracker::GetTopNQueriesAndUpdatePoolStats .. IMPALA-10129 Data race in MemTracker::GetTopNQueriesAndUpdatePoolStats This work addresses a data race condition in admission controller by providing the initializing values for two data members ( is_query_mem_tracker_ and query_id_) in a constructor for the MemTracker class. Without doing so, the two data members are set, without lock protection, after the object is constructed, which allows other threads to modify either of them at the same time. Testing: 1. Ran the python admission controller test successfully with a tsan build. Data race was not observed with the enhancement. Data race was observed without the enhancement. 2. Ran the core test. Change-Id: I9c4ffe8064d3e099a525cc48c218ef73112fb67b Reviewed-on: http://gerrit.cloudera.org:8080/16408 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M be/src/runtime/mem-tracker.cc M be/src/runtime/mem-tracker.h 2 files changed, 8 insertions(+), 7 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/16408 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I9c4ffe8064d3e099a525cc48c218ef73112fb67b Gerrit-Change-Number: 16408 Gerrit-PatchSet: 7 Gerrit-Owner: Qifan Chen Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Qifan Chen Gerrit-Reviewer: Sahil Takiar
[Impala-ASF-CR] IMPALA-10129 Data race in MemTracker::GetTopNQueriesAndUpdatePoolStats
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16408 ) Change subject: IMPALA-10129 Data race in MemTracker::GetTopNQueriesAndUpdatePoolStats .. Patch Set 6: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/16408 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9c4ffe8064d3e099a525cc48c218ef73112fb67b Gerrit-Change-Number: 16408 Gerrit-PatchSet: 6 Gerrit-Owner: Qifan Chen Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Qifan Chen Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Wed, 09 Sep 2020 04:23:16 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10129 Data race in MemTracker::GetTopNQueriesAndUpdatePoolStats
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16408 ) Change subject: IMPALA-10129 Data race in MemTracker::GetTopNQueriesAndUpdatePoolStats .. Patch Set 6: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/16408 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9c4ffe8064d3e099a525cc48c218ef73112fb67b Gerrit-Change-Number: 16408 Gerrit-PatchSet: 6 Gerrit-Owner: Qifan Chen Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Qifan Chen Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Tue, 08 Sep 2020 23:06:28 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10129 Data race in MemTracker::GetTopNQueriesAndUpdatePoolStats
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16408 ) Change subject: IMPALA-10129 Data race in MemTracker::GetTopNQueriesAndUpdatePoolStats .. Patch Set 6: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/6411/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/16408 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9c4ffe8064d3e099a525cc48c218ef73112fb67b Gerrit-Change-Number: 16408 Gerrit-PatchSet: 6 Gerrit-Owner: Qifan Chen Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Qifan Chen Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Tue, 08 Sep 2020 23:06:29 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10129 Data race in MemTracker::GetTopNQueriesAndUpdatePoolStats
Sahil Takiar has posted comments on this change. ( http://gerrit.cloudera.org:8080/16408 ) Change subject: IMPALA-10129 Data race in MemTracker::GetTopNQueriesAndUpdatePoolStats .. Patch Set 5: Code-Review+2 Hit IMPALA-9351 -- To view, visit http://gerrit.cloudera.org:8080/16408 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9c4ffe8064d3e099a525cc48c218ef73112fb67b Gerrit-Change-Number: 16408 Gerrit-PatchSet: 5 Gerrit-Owner: Qifan Chen Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Qifan Chen Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Tue, 08 Sep 2020 23:06:05 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10129 Data race in MemTracker::GetTopNQueriesAndUpdatePoolStats
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16408 ) Change subject: IMPALA-10129 Data race in MemTracker::GetTopNQueriesAndUpdatePoolStats .. Patch Set 5: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/6407/ -- To view, visit http://gerrit.cloudera.org:8080/16408 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9c4ffe8064d3e099a525cc48c218ef73112fb67b Gerrit-Change-Number: 16408 Gerrit-PatchSet: 5 Gerrit-Owner: Qifan Chen Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Qifan Chen Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Tue, 08 Sep 2020 20:25:39 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10129 Data race in MemTracker::GetTopNQueriesAndUpdatePoolStats
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16408 ) Change subject: IMPALA-10129 Data race in MemTracker::GetTopNQueriesAndUpdatePoolStats .. Patch Set 5: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/16408 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9c4ffe8064d3e099a525cc48c218ef73112fb67b Gerrit-Change-Number: 16408 Gerrit-PatchSet: 5 Gerrit-Owner: Qifan Chen Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Qifan Chen Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Tue, 08 Sep 2020 14:59:08 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10129 Data race in MemTracker::GetTopNQueriesAndUpdatePoolStats
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16408 ) Change subject: IMPALA-10129 Data race in MemTracker::GetTopNQueriesAndUpdatePoolStats .. Patch Set 5: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/6407/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/16408 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9c4ffe8064d3e099a525cc48c218ef73112fb67b Gerrit-Change-Number: 16408 Gerrit-PatchSet: 5 Gerrit-Owner: Qifan Chen Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Qifan Chen Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Tue, 08 Sep 2020 14:59:09 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10129 Data race in MemTracker::GetTopNQueriesAndUpdatePoolStats
Sahil Takiar has posted comments on this change. ( http://gerrit.cloudera.org:8080/16408 ) Change subject: IMPALA-10129 Data race in MemTracker::GetTopNQueriesAndUpdatePoolStats .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/16408 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9c4ffe8064d3e099a525cc48c218ef73112fb67b Gerrit-Change-Number: 16408 Gerrit-PatchSet: 4 Gerrit-Owner: Qifan Chen Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Qifan Chen Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Tue, 08 Sep 2020 14:58:46 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10129 Data race in MemTracker::GetTopNQueriesAndUpdatePoolStats
Qifan Chen has posted comments on this change. ( http://gerrit.cloudera.org:8080/16408 ) Change subject: IMPALA-10129 Data race in MemTracker::GetTopNQueriesAndUpdatePoolStats .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/16408/4/be/src/runtime/mem-tracker.h File be/src/runtime/mem-tracker.h: http://gerrit.cloudera.org:8080/#/c/16408/4/be/src/runtime/mem-tracker.h@97 PS4, Line 97: bool is_query_mem_tracker = false, const TUniqueId& query_id = TUniqueId()); > is it necessary for these parameters to all have default values? creating a In the enhancement, only two data members are getting the initial values from the initializing list. Without doing so, the MemTracker object will be constructed in two steps which allows another thread to modify the object (a race) in between. On paper, TUniqueId() is exactly the same object for query_id_ upon the construction of MemTracker, when no query_id object is passed in the initializing list. The addition of it allows a known query_id to be passed to the cstr. -- To view, visit http://gerrit.cloudera.org:8080/16408 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9c4ffe8064d3e099a525cc48c218ef73112fb67b Gerrit-Change-Number: 16408 Gerrit-PatchSet: 4 Gerrit-Owner: Qifan Chen Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Qifan Chen Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Thu, 03 Sep 2020 18:05:50 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10129 Data race in MemTracker::GetTopNQueriesAndUpdatePoolStats
Sahil Takiar has posted comments on this change. ( http://gerrit.cloudera.org:8080/16408 ) Change subject: IMPALA-10129 Data race in MemTracker::GetTopNQueriesAndUpdatePoolStats .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/16408/4/be/src/runtime/mem-tracker.h File be/src/runtime/mem-tracker.h: http://gerrit.cloudera.org:8080/#/c/16408/4/be/src/runtime/mem-tracker.h@97 PS4, Line 97: bool is_query_mem_tracker = false, const TUniqueId& query_id = TUniqueId()); is it necessary for these parameters to all have default values? creating a default query_id of just TUniqueId() seems dangerous -- To view, visit http://gerrit.cloudera.org:8080/16408 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9c4ffe8064d3e099a525cc48c218ef73112fb67b Gerrit-Change-Number: 16408 Gerrit-PatchSet: 4 Gerrit-Owner: Qifan Chen Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Thu, 03 Sep 2020 15:08:38 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10129 Data race in MemTracker::GetTopNQueriesAndUpdatePoolStats
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16408 ) Change subject: IMPALA-10129 Data race in MemTracker::GetTopNQueriesAndUpdatePoolStats .. Patch Set 4: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/7086/ : 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/16408 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9c4ffe8064d3e099a525cc48c218ef73112fb67b Gerrit-Change-Number: 16408 Gerrit-PatchSet: 4 Gerrit-Owner: Qifan Chen Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Thu, 03 Sep 2020 13:52:16 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10129 Data race in MemTracker::GetTopNQueriesAndUpdatePoolStats
Qifan Chen has uploaded a new patch set (#4). ( http://gerrit.cloudera.org:8080/16408 ) Change subject: IMPALA-10129 Data race in MemTracker::GetTopNQueriesAndUpdatePoolStats .. IMPALA-10129 Data race in MemTracker::GetTopNQueriesAndUpdatePoolStats This work addresses a data race condition in admission controller by providing the initializing values for two data members ( is_query_mem_tracker_ and query_id_) in a constructor for the MemTracker class. Without doing so, the two data members are set, without lock protection, after the object is constructed, which allows other threads to modify either of them at the same time. Testing: 1. Ran the python admission controller test successfully with a tsan build. Data race was not observed with the enhancement. Data race was observed without the enhancement. 2. Ran the core test. Change-Id: I9c4ffe8064d3e099a525cc48c218ef73112fb67b --- M be/src/runtime/mem-tracker.cc M be/src/runtime/mem-tracker.h 2 files changed, 8 insertions(+), 7 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/08/16408/4 -- To view, visit http://gerrit.cloudera.org:8080/16408 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I9c4ffe8064d3e099a525cc48c218ef73112fb67b Gerrit-Change-Number: 16408 Gerrit-PatchSet: 4 Gerrit-Owner: Qifan Chen Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sahil Takiar
[Impala-ASF-CR] IMPALA-10129 Data race in MemTracker::GetTopNQueriesAndUpdatePoolStats
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16408 ) Change subject: IMPALA-10129 Data race in MemTracker::GetTopNQueriesAndUpdatePoolStats .. Patch Set 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/7082/ : 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/16408 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9c4ffe8064d3e099a525cc48c218ef73112fb67b Gerrit-Change-Number: 16408 Gerrit-PatchSet: 3 Gerrit-Owner: Qifan Chen Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Thu, 03 Sep 2020 04:18:10 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10129 Data race in MemTracker::GetTopNQueriesAndUpdatePoolStats
Qifan Chen has uploaded this change for review. ( http://gerrit.cloudera.org:8080/16408 Change subject: IMPALA-10129 Data race in MemTracker::GetTopNQueriesAndUpdatePoolStats .. IMPALA-10129 Data race in MemTracker::GetTopNQueriesAndUpdatePoolStats This work addresses the data race by properly initializing two data member is_query_mem_tracker_ and query_id_ in a constructor for the MemTracker class. Without doing so, the two data members are set after the object is constructed. This creates a race condition for other threads to modify either of them at the same time. Testing: 1. Ran the python admission controller test successfully with a tsan build. Data race was not observed with the enhancement. Data race was observed without the enhancement. 2. Ran the core test. Change-Id: I9c4ffe8064d3e099a525cc48c218ef73112fb67b --- M be/src/runtime/mem-tracker.cc M be/src/runtime/mem-tracker.h 2 files changed, 8 insertions(+), 7 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/08/16408/3 -- To view, visit http://gerrit.cloudera.org:8080/16408 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I9c4ffe8064d3e099a525cc48c218ef73112fb67b Gerrit-Change-Number: 16408 Gerrit-PatchSet: 3 Gerrit-Owner: Qifan Chen
