Mailing-List: contact reviews-help@impala.incubator.apache.org; run by ezmlm
Precedence: bulk
Message-Id: <201610140145.u9E1jje2023367@ip-10-146-233-104.ec2.internal>
Date: Fri, 14 Oct 2016 01:45:45 +0000
From: "Todd Lipcon (Code Review)" <gerrit@cloudera.org>
To: Matthew Jacobs <mj@cloudera.com>, Todd Lipcon <todd@apache.org>,
        impala-cr@cloudera.com, reviews@impala.incubator.apache.org
CC: Mostafa Mokhtar <mmokhtar@cloudera.com>
Reply-To: todd@apache.org
Subject: =?UTF-8?Q?=5BImpala-ASF-CR=5D_IMPALA-4134=3A_Use_AUTO_FLUSH_BACKGROUND_for_Kudu_sink=0A?=
In-Reply-To: <gerrit.1475890777000.I665ce82d1caf64cf5bdf4bc0e15edbf9f1dc5ba0@gerrit.cloudera.org>
References: <gerrit.1475890777000.I665ce82d1caf64cf5bdf4bc0e15edbf9f1dc5ba0@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.2
archived-at: Fri, 14 Oct 2016 01:45:53 -0000

Todd Lipcon has posted comments on this change.

Change subject: IMPALA-4134: Use AUTO_FLUSH_BACKGROUND for Kudu sink
......................................................................


Patch Set 3:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/4670/3/be/src/exec/kudu-table-sink.cc
File be/src/exec/kudu-table-sink.cc:

Line 156:     DCHECK(sink_action_ == TSinkAction::DELETE) << "Sink type not supported: "
spurious change here


Line 163:   SCOPED_TIMER(profile()->total_time_counter());
I wonder if we should still keep some kind of timer which is specifically referring to the time spent in Kudu. We can't specifically measure the flushing, but we could do something like have a vector<unique_ptr<KuduWriteOperation>> which we fill up by looping over the row batch, and then in a SCOPED_TIMER(write_time_counter) apply them all? That way if we are writing faster than Kudu can absorb the writes, we'll see the blocking on the Apply() call register as significant time, and can still distinguish this time from time spent in output expr computation.


Line 301:   (*state->per_partition_status())[ROOT_PARTITION_KEY].num_appended_rows =
if we don't update this to the end, does the query summary page on the web UI still show the number of rows written by the sink? (I don't know if that counter was the output side or the input side of the sink, actually)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I665ce82d1caf64cf5bdf4bc0e15edbf9f1dc5ba0
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <todd@apache.org>
Gerrit-Reviewer: Matthew Jacobs <mj@cloudera.com>
Gerrit-Reviewer: Mostafa Mokhtar <mmokhtar@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <todd@apache.org>
Gerrit-HasComments: Yes