You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Alexey Serbin (Code Review)" <ge...@cloudera.org> on 2017/09/23 19:14:05 UTC
[kudu-CR] [tablet copy] end session before bootstrapping tablet
Alexey Serbin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/8133
Change subject: [tablet copy] end session before bootstrapping tablet
......................................................................
[tablet copy] end session before bootstrapping tablet
A minor clean-up: end both source and target tablet copy sessions
before bootstrapping the copied tablet.
Prior to this patch, the tablet copy sessions were ended only after
the tablet start-up process completed. However, there isn't any
explicit constraint which requires that behavior.
Change-Id: I20592007a40113d8409f121b226bcccca14e8300
---
M src/kudu/integration-tests/tablet_copy-itest.cc
M src/kudu/tserver/ts_tablet_manager.cc
2 files changed, 78 insertions(+), 77 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/33/8133/1
--
To view, visit http://gerrit.cloudera.org:8080/8133
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I20592007a40113d8409f121b226bcccca14e8300
Gerrit-Change-Number: 8133
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
[kudu-CR] [tablet copy] end session before bootstrapping tablet
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/8133 )
Change subject: [tablet copy] end session before bootstrapping tablet
......................................................................
Patch Set 1:
> Should this patch be abandoned?
I think I need to add the corresponding comment and then abandon the patch. I'll do that today.
--
To view, visit http://gerrit.cloudera.org:8080/8133
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I20592007a40113d8409f121b226bcccca14e8300
Gerrit-Change-Number: 8133
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
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-Comment-Date: Mon, 02 Oct 2017 22:24:08 +0000
Gerrit-HasComments: No
[kudu-CR] [tablet copy] end session before bootstrapping tablet
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/8133 )
Change subject: [tablet copy] end session before bootstrapping tablet
......................................................................
Patch Set 1:
> My understanding of it is the following. I would recommend that
> would browse the code to make sure this is still the case.
>
> because by the time the tablet boopststraps the leader might have
> ended up deleting all the logs that it needs to catch up to the
> leader.
> - say that you have wal segments 0-10 when tablet copy starts
> - tablet copy will anchor 0 until its destroyed, meaning the source
> wont delete it
> - when tablet copy is done the tablet still needs to boostrap which
> will take some time.
> - when tablet bootstrap is done the replica will need to continue
> catching up to the leader, this time through consensus. It needs
> segment 11 to be still available.
>
> We need the anchor to still be alive at this point, otherwise
> theres nothing preventing the leader from anchoring segment 11 and
> thus making the new replica unable to catch up.
>
> True that we're not optimal, we're anchoring 0 and we might only
> need to anchor 10/11 but no anchor at all is likely to cause
> replicas to start to fail copying.
Thank you for the explanation -- I will need to dig into that to get better understanding of this.
--
To view, visit http://gerrit.cloudera.org:8080/8133
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I20592007a40113d8409f121b226bcccca14e8300
Gerrit-Change-Number: 8133
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
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-Comment-Date: Mon, 25 Sep 2017 20:50:47 +0000
Gerrit-HasComments: No
[kudu-CR] [tablet copy] end session before bootstrapping tablet
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/8133 )
Change subject: [tablet copy] end session before bootstrapping tablet
......................................................................
Patch Set 1:
Should this patch be abandoned?
--
To view, visit http://gerrit.cloudera.org:8080/8133
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I20592007a40113d8409f121b226bcccca14e8300
Gerrit-Change-Number: 8133
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
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-Comment-Date: Mon, 02 Oct 2017 21:52:19 +0000
Gerrit-HasComments: No
[kudu-CR] [tablet copy] end session before bootstrapping tablet
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/8133 )
Change subject: [tablet copy] end session before bootstrapping tablet
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/8133/1/src/kudu/tserver/ts_tablet_manager.cc
File src/kudu/tserver/ts_tablet_manager.cc:
http://gerrit.cloudera.org:8080/#/c/8133/1/src/kudu/tserver/ts_tablet_manager.cc@a629
PS1, Line 629:
:
> We likely need an explanation here that states why this needs to survive un
I think it's a good idea. Let me dig in a bit to get a better understanding and then I'll update this changelist as needed (hope to do that later this evening).
--
To view, visit http://gerrit.cloudera.org:8080/8133
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I20592007a40113d8409f121b226bcccca14e8300
Gerrit-Change-Number: 8133
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
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-Comment-Date: Mon, 25 Sep 2017 21:39:00 +0000
Gerrit-HasComments: Yes
[kudu-CR] [tablet copy] end session before bootstrapping tablet
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/8133 )
Change subject: [tablet copy] end session before bootstrapping tablet
......................................................................
Patch Set 1:
> > Should this patch be abandoned?
>
> I think I need to add the corresponding comment and then abandon
> the patch. I'll do that today.
I meant I'll post a new patch to add comments on the necessity to keep the target session alive until the tablet successfully starts up.
--
To view, visit http://gerrit.cloudera.org:8080/8133
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I20592007a40113d8409f121b226bcccca14e8300
Gerrit-Change-Number: 8133
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
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-Comment-Date: Mon, 02 Oct 2017 22:26:51 +0000
Gerrit-HasComments: No
[kudu-CR] [tablet copy] end session before bootstrapping tablet
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/8133 )
Change subject: [tablet copy] end session before bootstrapping tablet
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/8133/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/8133/1//COMMIT_MSG@12
PS1, Line 12: Prior to this patch, the tablet copy sessions were ended only after
: the tablet start-up process completed. However, there isn't any
: explicit constraint which requires that behavior.
> I thought we did this to make sure that the leader (of whomever the replica
I'm curious why is that? I thought once all the data and logs are copied, we can delete the source, if it's scheduled for deletion. Why would we need to postpone that moment till the new tablet is started? If it fails, it would be necessary to start a new tablet copy session, wouldn't it?
--
To view, visit http://gerrit.cloudera.org:8080/8133
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I20592007a40113d8409f121b226bcccca14e8300
Gerrit-Change-Number: 8133
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
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-Comment-Date: Mon, 25 Sep 2017 20:13:18 +0000
Gerrit-HasComments: Yes
[kudu-CR] [tablet copy] end session before bootstrapping tablet
Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change. ( http://gerrit.cloudera.org:8080/8133 )
Change subject: [tablet copy] end session before bootstrapping tablet
......................................................................
Patch Set 1:
Looking at TabletCopySession it seems indeed that we allow the logs to be deleted at the source when it gets deleted. I would assume we need the client to remain alive (and to keep pinging the server) too.
Is that not the case?
--
To view, visit http://gerrit.cloudera.org:8080/8133
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I20592007a40113d8409f121b226bcccca14e8300
Gerrit-Change-Number: 8133
Gerrit-PatchSet: 1
Gerrit-Owner: 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-Comment-Date: Mon, 25 Sep 2017 19:55:36 +0000
Gerrit-HasComments: No
[kudu-CR] [tablet copy] end session before bootstrapping tablet
Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change. ( http://gerrit.cloudera.org:8080/8133 )
Change subject: [tablet copy] end session before bootstrapping tablet
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/8133/1/src/kudu/tserver/ts_tablet_manager.cc
File src/kudu/tserver/ts_tablet_manager.cc:
http://gerrit.cloudera.org:8080/#/c/8133/1/src/kudu/tserver/ts_tablet_manager.cc@a629
PS1, Line 629:
:
We likely need an explanation here that states why this needs to survive until OpenTablet is finished.
--
To view, visit http://gerrit.cloudera.org:8080/8133
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I20592007a40113d8409f121b226bcccca14e8300
Gerrit-Change-Number: 8133
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
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-Comment-Date: Mon, 25 Sep 2017 21:29:12 +0000
Gerrit-HasComments: Yes
[kudu-CR] [tablet copy] end session before bootstrapping tablet
Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change. ( http://gerrit.cloudera.org:8080/8133 )
Change subject: [tablet copy] end session before bootstrapping tablet
......................................................................
Patch Set 1:
My understanding of it is the following. I would recommend that would browse the code to make sure this is still the case.
because by the time the tablet boopststraps the leader might have ended up deleting all the logs that it needs to catch up to the leader.
- say that you have wal segments 0-10 when tablet copy starts
- tablet copy will anchor 0 until its destroyed, meaning the source wont delete it
- when tablet copy is done the tablet still needs to boostrap which will take some time.
- when tablet bootstrap is done the replica will need to continue catching up to the leader, this time through consensus. It needs segment 11 to be still available.
We need the anchor to still be alive at this point, otherwise theres nothing preventing the leader from anchoring segment 11 and thus making the new replica unable to catch up.
True that we're not optimal, we're anchoring 0 and we might only need to anchor 10/11 but no anchor at all is likely to cause replicas to start to fail copying.
--
To view, visit http://gerrit.cloudera.org:8080/8133
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I20592007a40113d8409f121b226bcccca14e8300
Gerrit-Change-Number: 8133
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
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-Comment-Date: Mon, 25 Sep 2017 20:21:33 +0000
Gerrit-HasComments: No
[kudu-CR] [tablet copy] end session before bootstrapping tablet
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has abandoned this change. ( http://gerrit.cloudera.org:8080/8133 )
Change subject: [tablet copy] end session before bootstrapping tablet
......................................................................
Abandoned
Abandoned in favor of http://gerrit.cloudera.org:8080/8197
--
To view, visit http://gerrit.cloudera.org:8080/8133
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: abandon
Gerrit-Change-Id: I20592007a40113d8409f121b226bcccca14e8300
Gerrit-Change-Number: 8133
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
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>
[kudu-CR] [tablet copy] end session before bootstrapping tablet
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/8133 )
Change subject: [tablet copy] end session before bootstrapping tablet
......................................................................
Patch Set 1:
(1 comment)
> Patch Set 1:
>
> My understanding of it is the following. I would recommend that would browse the code to make sure this is still the case.
>
> because by the time the tablet boopststraps the leader might have ended up deleting all the logs that it needs to catch up to the leader.
> - say that you have wal segments 0-10 when tablet copy starts
> - tablet copy will anchor 0 until its destroyed, meaning the source wont delete it
> - when tablet copy is done the tablet still needs to boostrap which will take some time.
> - when tablet bootstrap is done the replica will need to continue catching up to the leader, this time through consensus. It needs segment 11 to be still available.
>
> We need the anchor to still be alive at this point, otherwise theres nothing preventing the leader from anchoring segment 11 and thus making the new replica unable to catch up.
>
> True that we're not optimal, we're anchoring 0 and we might only need to anchor 10/11 but no anchor at all is likely to cause replicas to start to fail copying.
David is correct, we want to maintain the anchor until the replica starts up. That is not really sufficient since it would be better to wait until the leader connects but the intent is to try to prevent GC of logs before we can get the target replica connected to the leader and into its watermark tracking. We could make this more guaranteed than it is now, though.
http://gerrit.cloudera.org:8080/#/c/8133/1/src/kudu/tserver/ts_tablet_manager.cc
File src/kudu/tserver/ts_tablet_manager.cc:
http://gerrit.cloudera.org:8080/#/c/8133/1/src/kudu/tserver/ts_tablet_manager.cc@a629
PS1, Line 629:
:
> I think it's a good idea. Let me dig in a bit to get a better understandin
Agree w/ David
--
To view, visit http://gerrit.cloudera.org:8080/8133
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I20592007a40113d8409f121b226bcccca14e8300
Gerrit-Change-Number: 8133
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
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-Comment-Date: Mon, 25 Sep 2017 22:20:14 +0000
Gerrit-HasComments: Yes
[kudu-CR] [tablet copy] end session before bootstrapping tablet
Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change. ( http://gerrit.cloudera.org:8080/8133 )
Change subject: [tablet copy] end session before bootstrapping tablet
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/8133/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/8133/1//COMMIT_MSG@12
PS1, Line 12: Prior to this patch, the tablet copy sessions were ended only after
: the tablet start-up process completed. However, there isn't any
: explicit constraint which requires that behavior.
I thought we did this to make sure that the leader (of whomever the replica is pulling the data from) still has as the logs by the time the tablet is up and ready to start. Is that not the case? (or was that never the case?) I'll dig a bit too.
--
To view, visit http://gerrit.cloudera.org:8080/8133
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I20592007a40113d8409f121b226bcccca14e8300
Gerrit-Change-Number: 8133
Gerrit-PatchSet: 1
Gerrit-Owner: 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-Comment-Date: Mon, 25 Sep 2017 19:48:25 +0000
Gerrit-HasComments: Yes
[kudu-CR] [tablet copy] end session before bootstrapping tablet
Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change. ( http://gerrit.cloudera.org:8080/8133 )
Change subject: [tablet copy] end session before bootstrapping tablet
......................................................................
Patch Set 1:
oops, that last item should read:
"preventing the leader from _deleting_ segment 11"
--
To view, visit http://gerrit.cloudera.org:8080/8133
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I20592007a40113d8409f121b226bcccca14e8300
Gerrit-Change-Number: 8133
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
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-Comment-Date: Mon, 25 Sep 2017 21:24:53 +0000
Gerrit-HasComments: No