You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Mike Percy (Code Review)" <ge...@cloudera.org> on 2017/06/15 17:32:08 UTC

[kudu-CR] TabletReplica: Move Init() logic to Start()

Hello David Ribeiro Alves, Alexey Serbin,

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

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

to review the following change.

Change subject: TabletReplica: Move Init() logic to Start()
......................................................................

TabletReplica: Move Init() logic to Start()

This is a cleanup / refactor that will make it easier to implement
tombstoned voting. In this patch we merge Init() and Start() since they
aren't really logically different right now.

An unrelated change in this patch is a minor API cleanup in
TSTabletManager, where we now pass TabletReplica directly to
TSTabletManager::OpenTablet() instead of registering it in a map first
and then looking it up again later.

Change-Id: Ib762db8cfaac628325feee445490713b5c555c5a
---
M src/kudu/master/sys_catalog.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_replica-test.cc
M src/kudu/tablet/tablet_replica.cc
M src/kudu/tablet/tablet_replica.h
M src/kudu/tserver/tablet_copy_source_session-test.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/tserver/ts_tablet_manager.h
8 files changed, 131 insertions(+), 152 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib762db8cfaac628325feee445490713b5c555c5a
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>

[kudu-CR] TabletReplica: Move Init() logic to Start()

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

Change subject: TabletReplica: Move Init() logic to Start()
......................................................................


Patch Set 5:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7194/5/src/kudu/tablet/tablet_replica.cc
File src/kudu/tablet/tablet_replica.cc:

Line 113:       local_peer_pb_(std::move(local_peer_pb)),
> It seems we need to define LANG_CXX11 ?
Define it where? I'm not sure what you mean. Also, I find this error message surprising. :)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib762db8cfaac628325feee445490713b5c555c5a
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: Yes

[kudu-CR] TabletReplica: Move Init() logic to Start()

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

Change subject: TabletReplica: Move Init() logic to Start()
......................................................................


Patch Set 5:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7194/5/src/kudu/tablet/tablet_replica.cc
File src/kudu/tablet/tablet_replica.cc:

Line 113:       local_peer_pb_(std::move(local_peer_pb)),
> It seems to be saying that protobuf doesn't generate a move constructor. I'
SGTM


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib762db8cfaac628325feee445490713b5c555c5a
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: Yes

[kudu-CR] TabletReplica: Move Init() logic to Start()

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

Change subject: TabletReplica: Move Init() logic to Start()
......................................................................


Patch Set 4: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib762db8cfaac628325feee445490713b5c555c5a
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: No

[kudu-CR] TabletReplica: Move Init() logic to Start()

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

Change subject: TabletReplica: Move Init() logic to Start()
......................................................................


Patch Set 5: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7194/5/src/kudu/tablet/tablet_replica.cc
File src/kudu/tablet/tablet_replica.cc:

Line 113:       local_peer_pb_(std::move(local_peer_pb)),
> warning: passing result of std::move() as a const reference argument; no mo
It seems we need to define LANG_CXX11 ?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib762db8cfaac628325feee445490713b5c555c5a
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: Yes

[kudu-CR] TabletReplica: Move Init() logic to Start()

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

Change subject: TabletReplica: Move Init() logic to Start()
......................................................................


Patch Set 2:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/7194/2/src/kudu/tablet/tablet.h
File src/kudu/tablet/tablet.h:

PS2, Line 334: scoped_refptr<TabletMetadata> shared_metadata() const { return metadata_; }
> do we really need this? you can create a scoped_refptr from a raw pointer r
Yes but it's nice to consistently pass around the vehicle that makes it clear that TabletMetadata is refcounted.


http://gerrit.cloudera.org:8080/#/c/7194/2/src/kudu/tablet/tablet_replica-test.cc
File src/kudu/tablet/tablet_replica-test.cc:

PS2, Line 133: Peer
> nit rename these to StartReplica*
Done


PS2, Line 148: Status StartPeer(const ConsensusBootstrapInfo& info) {
> nit maybe rename the "regular" method to StartReplica() and this one to Sta
Done


http://gerrit.cloudera.org:8080/#/c/7194/2/src/kudu/tablet/tablet_replica.h
File src/kudu/tablet/tablet_replica.h:

Line 84:   Status Start(const consensus::ConsensusBootstrapInfo& info,
> warning: function 'kudu::tablet::TabletReplica::Start' has a definition wit
Done


http://gerrit.cloudera.org:8080/#/c/7194/2/src/kudu/tserver/ts_tablet_manager.h
File src/kudu/tserver/ts_tablet_manager.h:

PS2, Line 236: const scoped_refptr<TransitionInProgressDeleter>& deleter
> remove this if not used?
It's actually needed as a reference but not used directly. I'll add a comment.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib762db8cfaac628325feee445490713b5c555c5a
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: Yes

[kudu-CR] TabletReplica: Move Init() logic to Start()

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

Change subject: TabletReplica: Move Init() logic to Start()
......................................................................


Patch Set 6: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib762db8cfaac628325feee445490713b5c555c5a
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: No

[kudu-CR] TabletReplica: Move Init() logic to Start()

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

Change subject: TabletReplica: Move Init() logic to Start()
......................................................................


Patch Set 5:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7194/5/src/kudu/tablet/tablet_replica.cc
File src/kudu/tablet/tablet_replica.cc:

Line 113:       local_peer_pb_(std::move(local_peer_pb)),
> I would expect LANG_CXX11 to be an internal macro of the compiler, so ideal
It seems to be saying that protobuf doesn't generate a move constructor. I'll look into that soon but I'm going to commit this now.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib762db8cfaac628325feee445490713b5c555c5a
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: Yes

[kudu-CR] TabletReplica: Move Init() logic to Start()

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

Change subject: TabletReplica: Move Init() logic to Start()
......................................................................


Patch Set 5:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7194/5/src/kudu/tablet/tablet_replica.cc
File src/kudu/tablet/tablet_replica.cc:

Line 113:       local_peer_pb_(std::move(local_peer_pb)),
> Define it where? I'm not sure what you mean. Also, I find this error messag
I would expect LANG_CXX11 to be an internal macro of the compiler, so ideally we should not be messing around this.  But I haven't verified that -- may be it's not.  In the latter case, we need to add -DLANG_CXX11 to the compiler/preprocessor flags.

Yes, it's a bit surprising.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib762db8cfaac628325feee445490713b5c555c5a
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: Yes

[kudu-CR] TabletReplica: Move Init() logic to Start()

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

Change subject: TabletReplica: Move Init() logic to Start()
......................................................................


Patch Set 2:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/7194/2/src/kudu/tablet/tablet.h
File src/kudu/tablet/tablet.h:

PS2, Line 334: scoped_refptr<TabletMetadata> shared_metadata() const { return metadata_; }
do we really need this? you can create a scoped_refptr from a raw pointer right?


http://gerrit.cloudera.org:8080/#/c/7194/2/src/kudu/tablet/tablet_replica-test.cc
File src/kudu/tablet/tablet_replica-test.cc:

PS2, Line 133: Peer
nit rename these to StartReplica*


PS2, Line 148: Status StartPeer(const ConsensusBootstrapInfo& info) {
nit maybe rename the "regular" method to StartReplica() and this one to StartReplicaAndWaitUntilLeader


http://gerrit.cloudera.org:8080/#/c/7194/2/src/kudu/tablet/tablet_replica.h
File src/kudu/tablet/tablet_replica.h:

Line 84:   Status Start(const consensus::ConsensusBootstrapInfo& info,
> warning: function 'kudu::tablet::TabletReplica::Start' has a definition wit
address this?


http://gerrit.cloudera.org:8080/#/c/7194/2/src/kudu/tserver/ts_tablet_manager.h
File src/kudu/tserver/ts_tablet_manager.h:

PS2, Line 236: const scoped_refptr<TransitionInProgressDeleter>& deleter
remove this if not used?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib762db8cfaac628325feee445490713b5c555c5a
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: Yes

[kudu-CR] TabletReplica: Move Init() logic to Start()

Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,

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

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

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

Change subject: TabletReplica: Move Init() logic to Start()
......................................................................

TabletReplica: Move Init() logic to Start()

This is a cleanup / refactor that will make it easier to implement
tombstoned voting. In this patch we merge Init() and Start() since they
aren't really logically different right now.

An unrelated change in this patch is a minor API cleanup in
TSTabletManager, where we now pass TabletReplica directly to
TSTabletManager::OpenTablet() instead of registering it in a map first
and then looking it up again later.

Change-Id: Ib762db8cfaac628325feee445490713b5c555c5a
---
M src/kudu/master/sys_catalog.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_replica-test.cc
M src/kudu/tablet/tablet_replica.cc
M src/kudu/tablet/tablet_replica.h
M src/kudu/tserver/tablet_copy_source_session-test.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/tserver/ts_tablet_manager.h
8 files changed, 151 insertions(+), 166 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/94/7194/4
-- 
To view, visit http://gerrit.cloudera.org:8080/7194
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib762db8cfaac628325feee445490713b5c555c5a
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot

[kudu-CR] TabletReplica: Move Init() logic to Start()

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

Change subject: TabletReplica: Move Init() logic to Start()
......................................................................


Patch Set 4:

sadly, this is in dire need of a rebase

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib762db8cfaac628325feee445490713b5c555c5a
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: No

[kudu-CR] TabletReplica: Move Init() logic to Start()

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

Change subject: TabletReplica: Move Init() logic to Start()
......................................................................


Patch Set 4: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib762db8cfaac628325feee445490713b5c555c5a
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: No

[kudu-CR] TabletReplica: Move Init() logic to Start()

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

Change subject: TabletReplica: Move Init() logic to Start()
......................................................................


TabletReplica: Move Init() logic to Start()

This is a cleanup / refactor that will make it easier to implement
tombstoned voting. In this patch we merge Init() and Start() since they
aren't really logically different right now.

An unrelated change in this patch is a minor API cleanup in
TSTabletManager, where we now pass TabletReplica directly to
TSTabletManager::OpenTablet() instead of registering it in a map first
and then looking it up again later.

Change-Id: Ib762db8cfaac628325feee445490713b5c555c5a
Reviewed-on: http://gerrit.cloudera.org:8080/7194
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin <as...@cloudera.com>
---
M src/kudu/master/sys_catalog.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_replica-test.cc
M src/kudu/tablet/tablet_replica.cc
M src/kudu/tablet/tablet_replica.h
M src/kudu/tserver/tablet_copy_source_session-test.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/tserver/ts_tablet_manager.h
8 files changed, 166 insertions(+), 175 deletions(-)

Approvals:
  Alexey Serbin: Looks good to me, approved
  Kudu Jenkins: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Ib762db8cfaac628325feee445490713b5c555c5a
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot

[kudu-CR] TabletReplica: Move Init() logic to Start()

Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,

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

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

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

Change subject: TabletReplica: Move Init() logic to Start()
......................................................................

TabletReplica: Move Init() logic to Start()

This is a cleanup / refactor that will make it easier to implement
tombstoned voting. In this patch we merge Init() and Start() since they
aren't really logically different right now.

An unrelated change in this patch is a minor API cleanup in
TSTabletManager, where we now pass TabletReplica directly to
TSTabletManager::OpenTablet() instead of registering it in a map first
and then looking it up again later.

Change-Id: Ib762db8cfaac628325feee445490713b5c555c5a
---
M src/kudu/master/sys_catalog.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_replica-test.cc
M src/kudu/tablet/tablet_replica.cc
M src/kudu/tablet/tablet_replica.h
M src/kudu/tserver/tablet_copy_source_session-test.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/tserver/ts_tablet_manager.h
8 files changed, 133 insertions(+), 153 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib762db8cfaac628325feee445490713b5c555c5a
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot

[kudu-CR] TabletReplica: Move Init() logic to Start()

Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins,

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

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

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

Change subject: TabletReplica: Move Init() logic to Start()
......................................................................

TabletReplica: Move Init() logic to Start()

This is a cleanup / refactor that will make it easier to implement
tombstoned voting. In this patch we merge Init() and Start() since they
aren't really logically different right now.

An unrelated change in this patch is a minor API cleanup in
TSTabletManager, where we now pass TabletReplica directly to
TSTabletManager::OpenTablet() instead of registering it in a map first
and then looking it up again later.

Change-Id: Ib762db8cfaac628325feee445490713b5c555c5a
---
M src/kudu/master/sys_catalog.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_replica-test.cc
M src/kudu/tablet/tablet_replica.cc
M src/kudu/tablet/tablet_replica.h
M src/kudu/tserver/tablet_copy_source_session-test.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/tserver/ts_tablet_manager.h
8 files changed, 166 insertions(+), 175 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/94/7194/5
-- 
To view, visit http://gerrit.cloudera.org:8080/7194
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib762db8cfaac628325feee445490713b5c555c5a
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot

[kudu-CR] TabletReplica: Move Init() logic to Start()

Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,

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

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

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

Change subject: TabletReplica: Move Init() logic to Start()
......................................................................

TabletReplica: Move Init() logic to Start()

This is a cleanup / refactor that will make it easier to implement
tombstoned voting. In this patch we merge Init() and Start() since they
aren't really logically different right now.

An unrelated change in this patch is a minor API cleanup in
TSTabletManager, where we now pass TabletReplica directly to
TSTabletManager::OpenTablet() instead of registering it in a map first
and then looking it up again later.

Change-Id: Ib762db8cfaac628325feee445490713b5c555c5a
---
M src/kudu/master/sys_catalog.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_replica-test.cc
M src/kudu/tablet/tablet_replica.cc
M src/kudu/tablet/tablet_replica.h
M src/kudu/tserver/tablet_copy_source_session-test.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/tserver/ts_tablet_manager.h
8 files changed, 139 insertions(+), 155 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/94/7194/3
-- 
To view, visit http://gerrit.cloudera.org:8080/7194
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib762db8cfaac628325feee445490713b5c555c5a
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot