Skip to content

vrepl: import from multiple external sources#6103

Merged
rafael merged 16 commits intovitessio:masterfrom
planetscale:ss-vrepl-external
May 5, 2020
Merged

vrepl: import from multiple external sources#6103
rafael merged 16 commits intovitessio:masterfrom
planetscale:ss-vrepl-external

Conversation

@sougou
Copy link
Copy Markdown
Contributor

@sougou sougou commented Apr 22, 2020

This change builds on top of the tabletserver componentization changes. With this change, you can add a section to the tablet_config yaml that configures external sources, like this:

externalConnections:
  product:
    socket: /home/sougou/dev/src/vitess.io/vitess/vtdataroot/vtroot_15201/vt_0000000622/mysql.sock
    dbName: vt_product
    app:
      user: vt_app
    dba:
      user: vt_dba
  customer:
    flavor: FilePos
    socket: /home/sougou/dev/src/vitess.io/vitess/vtdataroot/vtroot_15201/vt_0000000620/mysql.sock
    dbName: vt_customer
    app:
      user: vt_app
    dba:
      user: vt_dba

You can then create replication streams from these external sources with these inserts:

VReplicationExec: insert into _vt.vreplication (workflow, db_name, source, pos, max_tps, max_replication_lag, tablet_types, time_updated, transaction_timestamp, state) values('product', 'vt_commerce', 'filter:<rules:<match:\"product\" > > external_mysql:\"product\" ', '', 9999, 9999, 'master', 0, 0, 'Running')
VReplicationExec: insert into _vt.vreplication (workflow, db_name, source, pos, max_tps, max_replication_lag, tablet_types, time_updated, transaction_timestamp, state) values('customer', 'vt_commerce', 'filter:<rules:<match:\"customer\" > > external_mysql:\"customer\" ', '', 9999, 9999, 'master', 0, 0, 'Running')
VReplicationExec: insert into _vt.vreplication (workflow, db_name, source, pos, max_tps, max_replication_lag, tablet_types, time_updated, transaction_timestamp, state) values('orders', 'vt_commerce', 'filter:<rules:<match:\"orders\" > > external_mysql:\"customer\" ', '', 9999, 9999, 'master', 0, 0, 'Running')

@sougou sougou requested review from harshit-gangal and rafael April 22, 2020 19:54
@harshit-gangal
Copy link
Copy Markdown
Member

Does it have to be via insert only or the workflow can be run with vtctl command as well?

@sougou
Copy link
Copy Markdown
Contributor Author

sougou commented Apr 23, 2020

Does it have to be via insert only or the workflow can be run with vtctl command as well?

It's actually a vtctl VReplicationExec command. I just pasted the insert part of it.

@morgo morgo modified the milestone: v6.0 Apr 27, 2020
Comment on lines +352 to +355
te.txPool.Open(te.env.Config().DB.AppWithDB(), te.env.Config().DB.DbaWithDB(), te.env.Config().DB.AppDebugWithDB())

if te.twopcEnabled && te.state == AcceptingReadAndWrite {
te.twoPC.Open(te.env.DBConfigs())
te.twoPC.Open(te.env.Config().DB)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I am not liking these kind of calls Config().DB.AppWithDB. I like it before with DBConfig().func

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I can add a convenience function that shortcuts Config().DB to DBConfigs() keeping it backward compatible. Will that work?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

That will work

@harshit-gangal
Copy link
Copy Markdown
Member

@rohit-nayak-ps , interested in looking at vreplication changes?

Copy link
Copy Markdown
Contributor Author

@sougou sougou left a comment

Choose a reason for hiding this comment

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

@rafael will also be reviewing this.

Comment on lines +352 to +355
te.txPool.Open(te.env.Config().DB.AppWithDB(), te.env.Config().DB.DbaWithDB(), te.env.Config().DB.AppDebugWithDB())

if te.twopcEnabled && te.state == AcceptingReadAndWrite {
te.twoPC.Open(te.env.DBConfigs())
te.twoPC.Open(te.env.Config().DB)
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I can add a convenience function that shortcuts Config().DB to DBConfigs() keeping it backward compatible. Will that work?

Copy link
Copy Markdown
Member

@rafael rafael left a comment

Choose a reason for hiding this comment

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

@sougou this looks really good. I reviewed and tested this PR in our environment.

I only have one comment around the tests. Let me know your thoughts.

config.DB = config.DB.Init(socketFile)
config.DB.Init(socketFile)
for _, cfg := range config.ExternalConnections {
cfg.Init("")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit - Maybe it would be clearer to have InitWithSocket and Init() I found it a bit confusing that this is initialized with empty string without any context on why that's the case.

}

func startExternalVReplication(t *testing.T, bls *binlogdatapb.BinlogSource) (cancelr func()) {
query := binlogplayer.CreateVReplication("test", bls, "", 9223372036854775807, 9223372036854775807, 0, vrepldb)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It seems that we are only testing vcopy in this test. I think we should also test a externalVReplication with a starting position provided. That will ensure that we go through the main methods of the interface (VStream and VStreamRows). I think this got lost when deleting the old tests.

sougou added 16 commits May 5, 2020 10:58
There are upcoming use cases where we'll create new named
subcomponents of tabletserver along with the global unnamed
one. Specifically for vreplication external mysql.

This change will tolerate such usage and we'll just reuse the
vars created by the unnamed tabletserver.

Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
The binlog dump was using dba privileges, but it's better to use
app, which is also expected to have privileges to request binlogs.

Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
@sougou sougou force-pushed the ss-vrepl-external branch from ce7cf75 to 4948935 Compare May 5, 2020 17:59
@rafael
Copy link
Copy Markdown
Member

rafael commented May 5, 2020

LGTM

@rafael rafael merged commit 80e6295 into vitessio:master May 5, 2020
@sougou sougou deleted the ss-vrepl-external branch May 11, 2020 22:33
@deepthi deepthi added this to the v7.0 milestone Jul 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants