Bikramjeet Vig has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15663 )

Change subject: IMPALA-9422: Re-visit and improve join node and builder's 
counters
......................................................................


Patch Set 2:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/15663/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/15663/1//COMMIT_MSG@24
PS1, Line 24:
> This non-child time doesn't seem right, we should be counting some time aga
Thanks for pointing this out. It turned out to be a pre-existing bug and have 
fixed it in this patch itself


http://gerrit.cloudera.org:8080/#/c/15663/1/be/src/exec/partitioned-hash-join-builder.h
File be/src/exec/partitioned-hash-join-builder.h:

http://gerrit.cloudera.org:8080/#/c/15663/1/be/src/exec/partitioned-hash-join-builder.h@586
PS1, Line 586:   Status AddBatch(RowBatch* build_batch);
> I think a better name is possible based on what it actually does, e.g. AddB
Done


http://gerrit.cloudera.org:8080/#/c/15663/1/be/src/exec/partitioned-hash-join-builder.h@590
PS1, Line 590: FinalizeBuild(Ru
> Maybe FinalizeBuild()
Done


http://gerrit.cloudera.org:8080/#/c/15663/1/be/src/exec/partitioned-hash-join-builder.cc
File be/src/exec/partitioned-hash-join-builder.cc:

http://gerrit.cloudera.org:8080/#/c/15663/1/be/src/exec/partitioned-hash-join-builder.cc@743
PS1, Line 743: }
> Maybe we should be actually starting and stopping the inactive timer and th
The reason I started the inactive timer earlier (L718) is because 
probe_barrier_->Wait() can wait for a long time before CleanUpHashPartitions() 
is executed and during that time neither the probe nor the builder thread is 
executing



--
To view, visit http://gerrit.cloudera.org:8080/15663
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I604075a2c8efcff26705fb39672f29f309b2ed97
Gerrit-Change-Number: 15663
Gerrit-PatchSet: 2
Gerrit-Owner: Bikramjeet Vig <[email protected]>
Gerrit-Reviewer: Bikramjeet Vig <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Tim Armstrong <[email protected]>
Gerrit-Comment-Date: Thu, 09 Apr 2020 20:35:30 +0000
Gerrit-HasComments: Yes

Reply via email to