You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Dan Burkert (Code Review)" <ge...@cloudera.org> on 2016/08/24 18:16:24 UTC

[kudu-CR] [c++-client] reduce in-flight-op partition key lifetime 

Hello Adar Dembo, Todd Lipcon, Alexey Serbin,

I'd like you to do a code review.  Please visit

    http://gerrit.cloudera.org:8080/4114

to review the following change.

Change subject: [c++-client] reduce in-flight-op partition key lifetime
......................................................................

[c++-client] reduce in-flight-op partition key lifetime

This commit changes the C++ MetaCache to take the lookup partition key by value,
and updates Batcher to pass the partition key by move when calling the MetaCache.
Additionally, the partition key is no longer stored as a field in the InFlightOp. The
effect is that the InFlightOp is smaller by the size of a std::string, and the
lifetime of the partition key is reduced from when the InFlightOp is complete to
when the meta cache lookup is complete.

Change-Id: Ia949b65a9447979c9c5eff324448af307a1db1b7
---
M src/kudu/client/batcher.cc
M src/kudu/client/meta_cache.cc
M src/kudu/client/meta_cache.h
3 files changed, 9 insertions(+), 11 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/14/4114/1
-- 
To view, visit http://gerrit.cloudera.org:8080/4114
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia949b65a9447979c9c5eff324448af307a1db1b7
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] [c++-client] reduce in-flight-op partition key lifetime

Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change.

Change subject: [c++-client] reduce in-flight-op partition key lifetime
......................................................................


Patch Set 2:

Build Started http://104.196.14.100/job/kudu-gerrit/3052/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia949b65a9447979c9c5eff324448af307a1db1b7
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No

[kudu-CR] [c++-client] reduce in-flight-op partition key lifetime

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.

Change subject: [c++-client] reduce in-flight-op partition key lifetime
......................................................................


Patch Set 1: Code-Review+1

(1 comment)

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

Line 9: This commit changes the C++ MetaCache to take the lookup partition key by value,
Nit: could you re-format the message a bit so the lines would be no longer than 72 symbols?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia949b65a9447979c9c5eff324448af307a1db1b7
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes

[kudu-CR] [c++-client] reduce in-flight-op partition key lifetime

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.

Change subject: [c++-client] reduce in-flight-op partition key lifetime
......................................................................


Patch Set 1: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia949b65a9447979c9c5eff324448af307a1db1b7
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No

[kudu-CR] [c++-client] reduce in-flight-op partition key lifetime

Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.

Change subject: [c++-client] reduce in-flight-op partition key lifetime
......................................................................


Patch Set 1:

(1 comment)

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

Line 9: This commit changes the C++ MetaCache to take the lookup partition key by value,
> Nit: could you re-format the message a bit so the lines would be no longer 
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia949b65a9447979c9c5eff324448af307a1db1b7
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes

[kudu-CR] [c++-client] reduce in-flight-op partition key lifetime

Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Hello Todd Lipcon, Alexey Serbin, Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

    http://gerrit.cloudera.org:8080/4114

to look at the new patch set (#2).

Change subject: [c++-client] reduce in-flight-op partition key lifetime
......................................................................

[c++-client] reduce in-flight-op partition key lifetime

This commit changes the C++ MetaCache to take the lookup partition key
by value, and updates Batcher to pass the partition key by move when
calling the MetaCache. Additionally, the partition key is no longer
stored as a field in the InFlightOp. The effect is that the InFlightOp
is smaller by the size of a std::string, and the lifetime of the
partition key is reduced from when the InFlightOp is complete to when
the meta cache lookup is complete.

Change-Id: Ia949b65a9447979c9c5eff324448af307a1db1b7
---
M src/kudu/client/batcher.cc
M src/kudu/client/meta_cache.cc
M src/kudu/client/meta_cache.h
3 files changed, 9 insertions(+), 11 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/14/4114/2
-- 
To view, visit http://gerrit.cloudera.org:8080/4114
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia949b65a9447979c9c5eff324448af307a1db1b7
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] [c++-client] reduce in-flight-op partition key lifetime

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.

Change subject: [c++-client] reduce in-flight-op partition key lifetime
......................................................................


Patch Set 2: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia949b65a9447979c9c5eff324448af307a1db1b7
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No

[kudu-CR] [c++-client] reduce in-flight-op partition key lifetime

Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change.

Change subject: [c++-client] reduce in-flight-op partition key lifetime
......................................................................


Patch Set 1:

Build Started http://104.196.14.100/job/kudu-gerrit/3047/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia949b65a9447979c9c5eff324448af307a1db1b7
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No

[kudu-CR] [c++-client] reduce in-flight-op partition key lifetime

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.

Change subject: [c++-client] reduce in-flight-op partition key lifetime
......................................................................


Patch Set 2: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia949b65a9447979c9c5eff324448af307a1db1b7
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No

[kudu-CR] [c++-client] reduce in-flight-op partition key lifetime

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has submitted this change and it was merged.

Change subject: [c++-client] reduce in-flight-op partition key lifetime
......................................................................


[c++-client] reduce in-flight-op partition key lifetime

This commit changes the C++ MetaCache to take the lookup partition key
by value, and updates Batcher to pass the partition key by move when
calling the MetaCache. Additionally, the partition key is no longer
stored as a field in the InFlightOp. The effect is that the InFlightOp
is smaller by the size of a std::string, and the lifetime of the
partition key is reduced from when the InFlightOp is complete to when
the meta cache lookup is complete.

Change-Id: Ia949b65a9447979c9c5eff324448af307a1db1b7
Reviewed-on: http://gerrit.cloudera.org:8080/4114
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin <as...@cloudera.com>
Reviewed-by: Adar Dembo <ad...@cloudera.com>
---
M src/kudu/client/batcher.cc
M src/kudu/client/meta_cache.cc
M src/kudu/client/meta_cache.h
3 files changed, 9 insertions(+), 11 deletions(-)

Approvals:
  Adar Dembo: Looks good to me, approved
  Alexey Serbin: Looks good to me, but someone else must approve
  Kudu Jenkins: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Ia949b65a9447979c9c5eff324448af307a1db1b7
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>