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.

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

VReplication: use db_filtered user for vstreams #10080

Merged
merged 5 commits into from
Apr 20, 2022

Conversation

mattlord
Copy link
Contributor

@mattlord mattlord commented Apr 12, 2022

Description

When performing VReplication workflows — such as MoveTables — we need to stream table rows, query results, and binary log events from the source in order to copy data and changes to the target.

The com_binlog_dump_gtid command is used on the source side in order to stream the binary log events (vstreamer->binlog streamer) from the source tablet's mysqld to the vttablet for filtering before sending to the target vttablets (vcopier->vplayer). In order to execute this command the user needs the REPLICATION SLAVE privilege.

A big issue this PR addresses is that the db_app user was used for this. Not only is this unexpected (non-intuitive and not documented), but it requires that the application user has replication privileges which is not the norm. In this PR we:

  1. Use the db_filtered user — which already has the REPLICATION SLAVE privilege — for streaming the binlog events (binlog streamer)
  2. Use the db_filtered user for all other base vstream types: row streamer and results streamer
  3. We remove the REPLICATION SLAVE privilege from the default db_app user

Related Issue(s)

Checklist

@mattlord mattlord added Component: VReplication Type: Enhancement Logical improvement (somewhere between a bug and feature) release notes labels Apr 12, 2022
@mattlord mattlord force-pushed the vrepl_binlogstream_vt_repl_user branch 2 times, most recently from fd3cf07 to e50c970 Compare April 12, 2022 19:33
@mattlord mattlord changed the title VReplication: use replication user when streaming binlogs VReplication: use vt_filtered user when streaming binlogs Apr 12, 2022
@mattlord mattlord force-pushed the vrepl_binlogstream_vt_repl_user branch 2 times, most recently from 579b889 to f1774d6 Compare April 12, 2022 20:14
@mattlord mattlord marked this pull request as ready for review April 12, 2022 21:49
@mattlord mattlord requested review from rohit-nayak-ps and removed request for systay, harshit-gangal and frouioui April 12, 2022 21:49
@mattlord mattlord changed the title VReplication: use vt_filtered user when streaming binlogs VReplication: use db_filtered user when streaming binlogs Apr 12, 2022
@mattlord mattlord force-pushed the vrepl_binlogstream_vt_repl_user branch 3 times, most recently from 81eee77 to 999bd11 Compare April 13, 2022 07:25
Copy link
Contributor

@rohit-nayak-ps rohit-nayak-ps left a comment

Choose a reason for hiding this comment

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

LGTM
nice catch

@mattlord mattlord requested a review from deepthi as a code owner April 13, 2022 17:18
@mattlord mattlord force-pushed the vrepl_binlogstream_vt_repl_user branch 5 times, most recently from 75ca067 to 1312af6 Compare April 13, 2022 18:53
@mattlord mattlord marked this pull request as draft April 13, 2022 21:27
@mattlord mattlord force-pushed the vrepl_binlogstream_vt_repl_user branch from 1877058 to bbdb16a Compare April 14, 2022 02:29
@mattlord mattlord changed the title VReplication: use db_filtered user when streaming binlogs VReplication: use db_filtered user for vstreams Apr 14, 2022
@mattlord mattlord requested a review from rohit-nayak-ps April 14, 2022 02:39
@mattlord mattlord marked this pull request as ready for review April 14, 2022 03:06
@mattlord
Copy link
Contributor Author

mattlord commented Apr 14, 2022

LGTM
nice catch

I increased the scope a bit to use the vt_filtered user for all base vstreamers: row, results, binlog. I believe that:

  1. This simplifies future work for VReplication as we lock down the MySQL accounts used (for vstreams at least, it's all db_filtered)
  2. This simplifies operations as VReplication is using a single MySQL user account for vstreams
  3. The product then aligns with the docs: https://vitess.io/docs/14.0/reference/vreplication/faq/

That's why I requested a re-review. Sorry. It was a long round about adventure to find out why the PITR recovery tests (which use rippled) were failing (dbconfig parts of bbdb16a).

@mattlord mattlord force-pushed the vrepl_binlogstream_vt_repl_user branch from ce06996 to 8eb2d8f Compare April 14, 2022 03:18
@mattlord mattlord changed the title VReplication: use db_filtered user for vstreams VReplication: use db_filtered user for base vstreams Apr 14, 2022
@mattlord mattlord changed the title VReplication: use db_filtered user for base vstreams VReplication: use db_filtered user for vstreams Apr 14, 2022
@mattlord mattlord requested a review from derekperkins April 14, 2022 03:37
@mattlord
Copy link
Contributor Author

@derekperkins please let me know if you have any messaging related questions or concerns. Thanks!

@mattlord mattlord force-pushed the vrepl_binlogstream_vt_repl_user branch from 8eb2d8f to fd090a2 Compare April 15, 2022 16:51
This way vreplication is using the same user on the source (streamer) and
target (player).

Signed-off-by: Matt Lord <[email protected]>
And set the filtered user config in the binlog server streamer and
the tabletmanager restore.

Signed-off-by: Matt Lord <[email protected]>
@mattlord mattlord force-pushed the vrepl_binlogstream_vt_repl_user branch from fd090a2 to f25b649 Compare April 17, 2022 16:12
Copy link
Contributor

@rohit-nayak-ps rohit-nayak-ps left a comment

Choose a reason for hiding this comment

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

lgtm

@mattlord mattlord merged commit 93f4490 into vitessio:main Apr 20, 2022
@mattlord mattlord deleted the vrepl_binlogstream_vt_repl_user branch April 20, 2022 13:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: VReplication Type: Enhancement Logical improvement (somewhere between a bug and feature)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants