Fang-Yu Rao has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/17095 )

Change subject: IMPALA-9767: Do not cleanup filter while PublishFilter is 
ongoing
......................................................................


Patch Set 1: Code-Review+1

(10 comments)

Thanks Riza for working on this fix and also Joe for providing the analysis! I 
really appreciate it. :-)

I am able to reproduce the issue according to suggested steps at IMPALA-9767 by 
Riza. I have pushed the patch that reproduces the issue to 
https://gerrit.cloudera.org/c/17103/ so that it would be easier for people to 
reproduce the issue. Will abandon this patch after the fix by Riza has been 
merged.

Only TPC-DS Q1 with the tpcds/tpcds_parquet database are needed to reproduce 
the issue. Building Impala with the 'asan' flag is also required to reproduce 
the issue. It seems it is unlikely to crash the coordinator in the local 
environment without the 'asan' flag.

I also verified locally that Riza's patch could avoid that heap-use-after-free 
error in my local development environment.

I only have some minor comments on the commit message.

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: cleanup
nit: clean up


http://gerrit.cloudera.org:8080/#/c/17095/1//COMMIT_MSG@9
PS1, Line 9: has been occurence
nit: have been occurrences


http://gerrit.cloudera.org:8080/#/c/17095/1//COMMIT_MSG@10
PS1, Line 10: publish
nit: publishing


http://gerrit.cloudera.org:8080/#/c/17095/1//COMMIT_MSG@10
PS1, Line 10: bloom
nit: a Bloom


http://gerrit.cloudera.org:8080/#/c/17095/1//COMMIT_MSG@11
PS1, Line 11: KRPC still sending the filter
nit: the coordinator is still sending the aggregated filter via KRPC


http://gerrit.cloudera.org:8080/#/c/17095/1//COMMIT_MSG@12
PS1, Line 12: decouple
nit: decouples


http://gerrit.cloudera.org:8080/#/c/17095/1//COMMIT_MSG@12
PS1, Line 12: remove
nit: removes


http://gerrit.cloudera.org:8080/#/c/17095/1//COMMIT_MSG@13
PS1, Line 13: separate
nit: a separate


http://gerrit.cloudera.org:8080/#/c/17095/1//COMMIT_MSG@14
PS1, Line 14: assert
nit: asserts


http://gerrit.cloudera.org:8080/#/c/17095/1//COMMIT_MSG@15
PS1, Line 15: filter
nit: the filter



--
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: 1
Gerrit-Owner: Riza Suminto <riza.sumi...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fangyu....@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <joemcdonn...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <riza.sumi...@cloudera.com>
Gerrit-Comment-Date: Sun, 21 Feb 2021 21:13:20 +0000
Gerrit-HasComments: Yes

Reply via email to