Alexey Serbin has posted comments on this change.

Change subject: [client] performance optimizations
......................................................................


Patch Set 2:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/4385/2/src/kudu/client/batcher.cc
File src/kudu/client/batcher.cc:

Line 481
> can you move this warning to the ctor? (or to the caller of the ctor)
Not sure I can track that condition in the ctor or caller of the ctor if not 
introducing an additional timeout parameter.

But what is the idea behind having this warning?  Why is it so crucial to have 
it at all, especially only every 1000th time of flushing a batcher or adding a 
write op?  I just want to understand the reasoning.


http://gerrit.cloudera.org:8080/#/c/4385/2/src/kudu/client/client.cc
File src/kudu/client/client.cc:

Line 761:     : data_(new KuduSession::Data(client, client->data_->messenger_)) 
{
> spurious change
That was style-related. OK, will revert.


http://gerrit.cloudera.org:8080/#/c/4385/2/src/kudu/client/session-internal.cc
File src/kudu/client/session-internal.cc:

Line 446:     // Thread-safety note: the buffer_bytes_limit_ and
> nit: rewrap to use whole lines
Done


Line 448:     // from any other thread of control since no thread-safety is 
advertised
> what's a "thread of control"?
it's just a thread -- will update the wording


Line 450:     // So, no protection while accessing those members.
> Actually can you move this (and the comment on loc 338) to where the field 
Done


http://gerrit.cloudera.org:8080/#/c/4385/2/src/kudu/client/session-internal.h
File src/kudu/client/session-internal.h:

Line 160:   sp::weak_ptr<KuduSession> session_;
> doc
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I4b57fc7355f9f673f30861ec30cb6b48cdf656d2
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <aser...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dral...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <t...@apache.org>
Gerrit-HasComments: Yes

Reply via email to