Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

tabletmanager revamp: refactor initialization #6311

Merged
merged 32 commits into from
Jun 24, 2020
Merged

tabletmanager revamp: refactor initialization #6311

merged 32 commits into from
Jun 24, 2020

Conversation

sougou
Copy link
Contributor

@sougou sougou commented Jun 15, 2020 •
edited
Loading

This work is for #6131.

This change mainly focuses on refactoring how vttablet comes up. This change seems to have improved the flakyness of the endtoend tests, most likely because the new code is more aggressive about computing the mysql port, which makes everything else faster. Here's a summary of the changes:

  • Initializations parameters: init_keyspace, init_shard and init_tablet_type are now mandatory.
  • The demote_master_type flag has been deprecated, and we use the init_tablet_type in place of it.
  • ActionAgent has been renamed to TabletManager. There will not be much action left by the time we're done with the refactor.
  • TabletManager initialization uses a unified code path for all: vttablet, vtcombo and tests. You have to build the tablet record, initialize the necessary public members of TabletManager, and invoke Start.
  • InitTablet and the old Start have been broken out into smaller functions that perform focused and isolated work. All those actions are performed by the new Start function.
  • VREngine and UpdateStream follow the two-phase initialization. They are created outside tm, but the Start function will invoke their InitDBConfig function after computing the db name. They are also optional, which makes it convenient for tests.
  • action_agent.go and init_tablet.go are replaced by tm_init.go.
  • The mysql port is computed by a standalone goroutine that terminates once it's saved it into the tablet record. There are no use cases where the port will change. So, this should only one-time.
  • UpdateStream is always enabled. There was no reason to ever disable it. This is how VStream works, and it's been working fine.
  • A race has been fixed in shard sync where the sync loop may end up trying to close a nil channel.
  • Removed legacy (3.0) code that uses PortMap to store and retrieve the mysql host and port.
  • New unit tests cover all the critical code paths.

sougou added 30 commits June 12, 2020 09:00
@sougou
tm revamp: remove deprecated code
4eabc0c 
mysql host and port have been first class proto fields for a long
time now.

Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
@sougou
tm revamp: simplify topo update of mysql port
8339adf 
The previous scheme was unnecessarily complex and elaborate.
The new code launches a simple standalone goroutine that
polls and exits after a successful update.

Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
@sougou
tm revamp: fix e2e tabletmanager test
ff3145b 
Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
@sougou
tm revamp: deprecate demoteMasterType
7d8d899 
No one knows this exists, and no one uses it. The intuitive tablet
type is the one from init_tablet_type, which everybody already sets
while starting a vttablet.

Because this is the only use case in real life, this change also
requires init_tablet_type to be always specified.

Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
@sougou
tm revamp: fix reparent test
0a33587 
Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
@sougou
tm revamp: move mysql check inside Start
65084c3 
Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
@sougou
tm revamp: break InitTablet into smaller parts
3411d62 
We'll recombine them differently later.

Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
@sougou
tm revamp: checkMysql function
52633d9 
Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
@sougou
tm revamp: fix race conditions
56e4a05 
Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
@sougou
tm revamp: updateStream initialization refactored
ada175d 
Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
@sougou
tm revamp: break up Start into smaller parts
a347ebd 
Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
@sougou
tm revamp: refreshState does not reload tablet
6608b8c 
Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
@sougou
tm revamp: pre-populate shardInfo and srvKeyspace
553d2d6 
Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
@sougou
tm revamp: fix tests after rebase
68627af 
Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
@sougou
tm revamp: remove agent._masterTermStartTime
105f995 
Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
@sougou
tm revamp: ActionAgent -> TabletManager
aa7484a 
Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
@sougou
tm revamp: Agent -> TM
3331e9f 
Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
@sougou
tm revamp: agent -> tm
be8cc28 
Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
@sougou
tm revamp: rename 'New' functions
ac077d9 
Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
@sougou
tm revamp: make some members private
2d3641a 
Preparing for a struct based initialization.

Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
@sougou
tm revamp: groundwork for different initialization
31cb5f8 
Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
@sougou
tm revamp: use new initialization method
9ed5de7 
Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
@sougou
tm revamp: make VREngine and UpdateStream optional
127e9f7 
Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
@sougou
tm revamp: unify codepaths through tm.Start
dbc25f0 
Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
@sougou
tm revamp: simplify some tm init tests
99f385f 
Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
@sougou
tm revamp: TestStartBuildTabletFromInput
fc52afe 
Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
@sougou
tm revamp: fix vtcombo healthcheck
f5f5e17 
Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
@sougou
tm revamp: TestStartCreateKeyspaceShard
9254e78 
Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
@sougou
tm revamp: TestCheckMastership
4442cd9 
Also fixed a race in shard sync.

Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
@sougou
tm recamp: test for multi-shard srvKeyspace
cab2687 
Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
@sougou
tm revamp: TestStartCheckMysql
7e8d2d4 
Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
@sougou sougou requested review from enisoc and deepthi June 15, 2020 01:25
deepthi
deepthi reviewed Jun 16, 2020
View reviewed changes
Copy link
Member

@deepthi deepthi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First pass review with minor nits

go/cmd/vtbackup/vtbackup.go Show resolved Hide resolved
go/cmd/vttablet/healthz.go Outdated Show resolved Hide resolved
go/cmd/vttablet/vttablet.go Outdated Show resolved Hide resolved
go/cmd/vttablet/vttablet.go Show resolved Hide resolved
go/test/endtoend/reparent/reparent_test.go Show resolved Hide resolved
go/vt/vtctl/vtctl.go Show resolved Hide resolved
deepthi
deepthi reviewed Jun 17, 2020
View reviewed changes
Copy link
Member

@deepthi deepthi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly looks good, a few questions/comments.

go/vt/topo/shard.go Show resolved Hide resolved
go/vt/vttablet/tabletmanager/tm_init_test.go Outdated Show resolved Hide resolved
go/vt/vttablet/tabletmanager/tm_init_test.go Show resolved Hide resolved
go/vt/vttablet/tabletmanager/tm_init.go Outdated Show resolved Hide resolved
go/vt/vttablet/tabletmanager/tm_init.go Show resolved Hide resolved
go/vt/vttablet/tabletmanager/shard_sync.go Show resolved Hide resolved
go/vt/vttablet/tabletmanager/shard_sync.go Show resolved Hide resolved
@sougou
tm revamp: address review comments
245d708 
Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
@deepthi deepthi merged commit 9ad7303 into vitessio:master Jun 24, 2020
@deepthi deepthi added this to the v7.0 milestone Jul 17, 2020
@deepthi deepthi mentioned this pull request Jul 27, 2020
Closed
@sougou sougou deleted the ss-tm-owned-record2 branch July 29, 2020 00:43
@ameetkotian ameetkotian mentioned this pull request Aug 20, 2020
Merged
@GuptaManan100 GuptaManan100 mentioned this pull request Dec 6, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@deepthi deepthi deepthi approved these changes

@enisoc enisoc Awaiting requested review from enisoc

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v7.0
Development

Successfully merging this pull request may close these issues.
2 participants
@sougou @deepthi