[Impala-ASF-CR] IMPALA-9767: Do not clean up filter while PublishFilter is ongoing
Joe McDonnell has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/17095 ) Change subject: IMPALA-9767: Do not clean up filter while PublishFilter is ongoing .. IMPALA-9767: Do not clean up filter while PublishFilter is ongoing There have been occurrences of heap-use-after-free in ASAN build during runtime filter publishing. This issue happens because the memory consumed by an aggregated Bloom filter has been released while the coordinator is still sending the aggregated filter via KRPC to workers. This patch removes the offending cleanup routine. This patch also decouples the cleanup routine from FilterState::DisableAndRelease() into a separate method FilterState::Release() and asserts that no RPC is inflight while cleaning up the filter. Testing: - Reproduce the bug by instrumenting Coordinator::UpdateFilter(). - Manually verify that the bug does not happen anymore after the patch. - Pass core tests in ASAN build. Change-Id: I1c408bdedab83c4b9249e2c0c493cb0f894a3d08 Reviewed-on: http://gerrit.cloudera.org:8080/17095 Reviewed-by: Joe McDonnell Tested-by: Impala Public Jenkins --- M be/src/runtime/coordinator-backend-state.cc M be/src/runtime/coordinator-filter-state.h M be/src/runtime/coordinator.cc 3 files changed, 18 insertions(+), 20 deletions(-) Approvals: Joe McDonnell: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/17095 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I1c408bdedab83c4b9249e2c0c493cb0f894a3d08 Gerrit-Change-Number: 17095 Gerrit-PatchSet: 4 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Riza Suminto
[Impala-ASF-CR] IMPALA-9767: Do not clean up filter while PublishFilter is ongoing
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17095 ) Change subject: IMPALA-9767: Do not clean up filter while PublishFilter is ongoing .. Patch Set 3: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/17095 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1c408bdedab83c4b9249e2c0c493cb0f894a3d08 Gerrit-Change-Number: 17095 Gerrit-PatchSet: 3 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Tue, 23 Feb 2021 04:00:31 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9767: Do not clean up filter while PublishFilter is ongoing
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17095 ) Change subject: IMPALA-9767: Do not clean up filter while PublishFilter is ongoing .. Patch Set 3: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/6911/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/17095 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1c408bdedab83c4b9249e2c0c493cb0f894a3d08 Gerrit-Change-Number: 17095 Gerrit-PatchSet: 3 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Mon, 22 Feb 2021 22:07:08 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9767: Do not clean up filter while PublishFilter is ongoing
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/17095 ) Change subject: IMPALA-9767: Do not clean up filter while PublishFilter is ongoing .. Patch Set 3: Code-Review+2 This looks good to me. Thanks for taking this on! -- To view, visit http://gerrit.cloudera.org:8080/17095 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1c408bdedab83c4b9249e2c0c493cb0f894a3d08 Gerrit-Change-Number: 17095 Gerrit-PatchSet: 3 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Mon, 22 Feb 2021 18:47:30 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9767: Do not clean up filter while PublishFilter is ongoing
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/17095 ) Change subject: IMPALA-9767: Do not clean up filter while PublishFilter is ongoing .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/17095/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/17095/2//COMMIT_MSG@10 PS2, Line 10: the memory : consumed by > nit: Maybe it would be a bit clearer if we change it to the following. Done -- To view, visit http://gerrit.cloudera.org:8080/17095 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1c408bdedab83c4b9249e2c0c493cb0f894a3d08 Gerrit-Change-Number: 17095 Gerrit-PatchSet: 3 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Mon, 22 Feb 2021 16:39:18 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9767: Do not clean up filter while PublishFilter is ongoing
Hello Fang-Yu Rao, Joe McDonnell, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/17095 to look at the new patch set (#3). Change subject: IMPALA-9767: Do not clean up filter while PublishFilter is ongoing .. IMPALA-9767: Do not clean up filter while PublishFilter is ongoing There have been occurrences of heap-use-after-free in ASAN build during runtime filter publishing. This issue happens because the memory consumed by an aggregated Bloom filter has been released while the coordinator is still sending the aggregated filter via KRPC to workers. This patch removes the offending cleanup routine. This patch also decouples the cleanup routine from FilterState::DisableAndRelease() into a separate method FilterState::Release() and asserts that no RPC is inflight while cleaning up the filter. Testing: - Reproduce the bug by instrumenting Coordinator::UpdateFilter(). - Manually verify that the bug does not happen anymore after the patch. - Pass core tests in ASAN build. Change-Id: I1c408bdedab83c4b9249e2c0c493cb0f894a3d08 --- M be/src/runtime/coordinator-backend-state.cc M be/src/runtime/coordinator-filter-state.h M be/src/runtime/coordinator.cc 3 files changed, 18 insertions(+), 20 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/95/17095/3 -- To view, visit http://gerrit.cloudera.org:8080/17095 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I1c408bdedab83c4b9249e2c0c493cb0f894a3d08 Gerrit-Change-Number: 17095 Gerrit-PatchSet: 3 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Riza Suminto
[Impala-ASF-CR] IMPALA-9767: Do not clean up filter while PublishFilter is ongoing
Fang-Yu Rao has posted comments on this change. ( http://gerrit.cloudera.org:8080/17095 ) Change subject: IMPALA-9767: Do not clean up filter while PublishFilter is ongoing .. Patch Set 2: Code-Review+1 (1 comment) Thanks Riza! I only have a very minor comment on the commit message and do not have any more suggestion. http://gerrit.cloudera.org:8080/#/c/17095/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/17095/2//COMMIT_MSG@10 PS2, Line 10: a Bloom filter is : cleaning up nit: Maybe it would be a bit clearer if we change it to the following. the memory consumed by an aggregated Bloom filter has been released -- To view, visit http://gerrit.cloudera.org:8080/17095 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1c408bdedab83c4b9249e2c0c493cb0f894a3d08 Gerrit-Change-Number: 17095 Gerrit-PatchSet: 2 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Mon, 22 Feb 2021 04:39:46 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9767: Do not clean up filter while PublishFilter is ongoing
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/17095 ) Change subject: IMPALA-9767: Do not clean up filter while PublishFilter is ongoing .. Patch Set 2: (10 comments) Thanks Fang-Yu. I'm also rewording a bit. http://gerrit.cloudera.org:8080/#/c/17095/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/17095/1//COMMIT_MSG@7 PS1, Line 7: clean u > nit: clean up Done http://gerrit.cloudera.org:8080/#/c/17095/1//COMMIT_MSG@9 PS1, Line 9: have been occurren > nit: have been occurrences Done http://gerrit.cloudera.org:8080/#/c/17095/1//COMMIT_MSG@10 PS1, Line 10: filt > nit: a Bloom Done http://gerrit.cloudera.org:8080/#/c/17095/1//COMMIT_MSG@10 PS1, Line 10: publish > nit: publishing Done http://gerrit.cloudera.org:8080/#/c/17095/1//COMMIT_MSG@11 PS1, Line 11: is still sending the aggrega > nit: the coordinator is still sending the aggregated filter via KRPC Done http://gerrit.cloudera.org:8080/#/c/17095/1//COMMIT_MSG@12 PS1, Line 12: nup rout > nit: decouples Done http://gerrit.cloudera.org:8080/#/c/17095/1//COMMIT_MSG@12 PS1, Line 12: This > nit: removes Done http://gerrit.cloudera.org:8080/#/c/17095/1//COMMIT_MSG@13 PS1, Line 13: > nit: a separate Done http://gerrit.cloudera.org:8080/#/c/17095/1//COMMIT_MSG@14 PS1, Line 14: to a s > nit: asserts Done http://gerrit.cloudera.org:8080/#/c/17095/1//COMMIT_MSG@15 PS1, Line 15: :Relea > nit: the filter Done -- To view, visit http://gerrit.cloudera.org:8080/17095 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1c408bdedab83c4b9249e2c0c493cb0f894a3d08 Gerrit-Change-Number: 17095 Gerrit-PatchSet: 2 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Mon, 22 Feb 2021 01:39:53 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9767: Do not clean up filter while PublishFilter is ongoing
Hello Fang-Yu Rao, Joe McDonnell, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/17095 to look at the new patch set (#2). Change subject: IMPALA-9767: Do not clean up filter while PublishFilter is ongoing .. IMPALA-9767: Do not clean up filter while PublishFilter is ongoing There have been occurrences of heap-use-after-free in ASAN build during runtime filter publishing. This issue happens because a Bloom filter is cleaning up while the coordinator is still sending the aggregated filter via KRPC to workers. This patch removes the offending cleanup routine. This patch also decouples the cleanup routine from FilterState::DisableAndRelease() into a separate method FilterState::Release() and asserts that no RPC is inflight while cleaning up the filter. Testing: - Reproduce the bug by instrumenting Coordinator::UpdateFilter(). - Manually verify that the bug does not happen anymore after the patch. - Pass core tests in ASAN build. Change-Id: I1c408bdedab83c4b9249e2c0c493cb0f894a3d08 --- M be/src/runtime/coordinator-backend-state.cc M be/src/runtime/coordinator-filter-state.h M be/src/runtime/coordinator.cc 3 files changed, 18 insertions(+), 20 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/95/17095/2 -- To view, visit http://gerrit.cloudera.org:8080/17095 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I1c408bdedab83c4b9249e2c0c493cb0f894a3d08 Gerrit-Change-Number: 17095 Gerrit-PatchSet: 2 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Riza Suminto
