Mailing-List: contact reviews-help@impala.incubator.apache.org; run by ezmlm
Precedence: bulk
Message-Id: <201707142243.v6EMhS0j020723@ip-10-146-233-104.ec2.internal>
Date: Fri, 14 Jul 2017 22:43:28 +0000
From: "Tim Armstrong (Code Review)" <gerrit@cloudera.org>
To: anujphadke <aphadke@cloudera.com>, impala-cr@cloudera.com,
        reviews@impala.incubator.apache.org
CC: Thomas Tauber-Marshall <tmarshall@cloudera.com>
Reply-To: tarmstrong@cloudera.com
Subject: =?UTF-8?Q?=5BImpala-ASF-CR=5D_IMPALA-5586=3A_Null-aware_anti-join_can_take_a_long_time_to_cancel=0A?=
In-Reply-To: <gerrit.1499735720000.I0800754d4ad31cbadbdfadc630c640963f3f6053@gerrit.cloudera.org>
References: <gerrit.1499735720000.I0800754d4ad31cbadbdfadc630c640963f3f6053@gerrit.cloudera.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Content-Disposition: inline
User-Agent: Gerrit/2.12.7
archived-at: Fri, 14 Jul 2017 22:43:41 -0000

Tim Armstrong has posted comments on this change.

Change subject: IMPALA-5586: Null-aware anti-join can take a long time to cancel
......................................................................


Patch Set 3:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/7393/3/be/src/exec/partitioned-hash-join-node.cc
File be/src/exec/partitioned-hash-join-node.cc:

Line 1018:   // For each row, iterate over all rows in the build table.
The need for RETURN_IF_CANCELLED is a little subtle. A one-line comment would be useful, e.g. "This loop can run for a long time. Periodically check for cancellation."


Line 1023:     for (int j = 0; j < build_rows->num_rows(); ++j) {
This loop can also potentially run for a long time if build_rows is large. It will be a bit expensive to check every iteration but maybe check every 1024 rows or something like that (I'm suggesting a power-of-two because computing % is cheap for a constant power-of-two).

  if (j % 1024 == 0) RETURN_IF_CANCELLED(state);


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0800754d4ad31cbadbdfadc630c640963f3f6053
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke <aphadke@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tmarshall@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-Reviewer: anujphadke <aphadke@cloudera.com>
Gerrit-HasComments: Yes