Skip to content

Why does binlog syncer need to register as a slave? #966

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

Closed
Tang8330 opened this issue Jan 10, 2025 · 8 comments · Fixed by #967
Closed

Why does binlog syncer need to register as a slave? #966

Tang8330 opened this issue Jan 10, 2025 · 8 comments · Fixed by #967

Comments

@Tang8330
Copy link
Contributor

Tang8330 commented Jan 10, 2025

Why does it need to register as a slave? Code ref

By making this additional network call, we are running into a MySQL password limitation as outlined here.

@Tang8330
Copy link
Contributor Author

I was able to bypass the 32 char limitation by commenting out register slave and replication worked as expected still.

@lance6716
Copy link
Collaborator

Oh, do you mean we can directly send COM_BINLOG_DUMP/COM_BINLOG_DUMP_GTID without COM_REGISTER_SLAVE? I didn't think of it before. Will take a look soon.

@Tang8330
Copy link
Contributor Author

Yes exactly. @lance6716

@dveeden
Copy link
Collaborator

dveeden commented Jan 10, 2025

  1. It seems like COM_REGISTER_SLAVE is required and/or expected by the protocol before COM_BINLOG_DUMP/COM_BINLOG_DUMP_GTID. As the protocol doesn't have a formal specification there isn't really a good way to see if not doing this is allowed or not.
  2. I think there might be another way of fixing the password length issue.
  3. I thought that not running COM_REGISTER_SLAVE would not register the host in the output of SHOW REPLICAS, but that doesn't seem to be the case.
  4. Not registering as a replica might skip the duplicate server-id/uuid checks for the replica. This should be tested.

If we can safely skip COM_REGISTER_SLAVE and/or leave the decision to the application that uses the library that might be good.

Maybe we should change the title of this issue to "replication user password limited to 32 characters"?

@dveeden
Copy link
Collaborator

dveeden commented Jan 10, 2025

This seems to work:

diff --git a/replication/binlogsyncer.go b/replication/binlogsyncer.go
index ccbfb9f..161fd69 100644
--- a/replication/binlogsyncer.go
+++ b/replication/binlogsyncer.go
@@ -348,13 +348,13 @@ func (b *BinlogSyncer) registerSlave() error {
                }
        }
 
-       if err = b.writeRegisterSlaveCommand(); err != nil {
-               return errors.Trace(err)
-       }
+       // if err = b.writeRegisterSlaveCommand(); err != nil {
+       //      return errors.Trace(err)
+       // }
 
-       if _, err = b.c.ReadOKPacket(); err != nil {
-               return errors.Trace(err)
-       }
+       // if _, err = b.c.ReadOKPacket(); err != nil {
+       //      return errors.Trace(err)
+       // }
 
        serverUUID, err := uuid.NewUUID()
        if err != nil {

Test for this issue:

mysql-9.1.0> create user 'repl' identified by 'replreplreplreplreplreplreplreplrepl';
Query OK, 0 rows affected (0.02 sec)

mysql-9.1.0> grant all on *.* to 'repl'@'%';
Query OK, 0 rows affected (0.01 sec)

mysql-9.1.0> select char_length('replreplreplreplreplreplreplreplrepl');
+-----------------------------------------------------+
| char_length('replreplreplreplreplreplreplreplrepl') |
+-----------------------------------------------------+
|                                                  36 |
+-----------------------------------------------------+
1 row in set (0.00 sec)
$ ./bin/go-mysqlbinlog -user repl -password replreplreplreplreplreplreplreplrepl
[2025/01/10 07:49:31] [info] binlogsyncer.go:191 create BinlogSyncer with config {ServerID:101 Flavor:mysql Host:127.0.0.1 Port:3306 User:repl Password: Localhost: Charset: SemiSyncEnabled:false RawModeEnabled:false TLSConfig:<nil> ParseTime:false TimestampStringLocation:UTC UseDecimal:true RecvBufferSize:0 HeartbeatPeriod:0s ReadTimeout:0s MaxReconnectAttempts:10 DisableRetrySync:false VerifyChecksum:false DumpCommandFlag:0 Option:<nil> Logger:0xc0000a6300 Dialer:0x6d2120 RowsEventDecodeFunc:<nil> TableMapOptionalMetaDecodeFunc:<nil> DiscardGTIDSet:false EventCacheCount:10240 SynchronousEventHandler:<nil>}
[2025/01/10 07:49:31] [info] binlogsyncer.go:443 begin to sync binlog from position (, 4)
Start sync error: ERROR 1105 (HY000): Failed to register replica; too long 'report-password'
github.com/pingcap/errors.AddStack
	/home/dvaneeden/go/pkg/mod/github.com/pingcap/[email protected]/errors.go:174
github.com/pingcap/errors.Trace
	/home/dvaneeden/go/pkg/mod/github.com/pingcap/[email protected]/juju_adaptor.go:15
github.com/go-mysql-org/go-mysql/replication.(*BinlogSyncer).registerSlave
	/home/dvaneeden/dev/go-mysql/replication/binlogsyncer.go:356
github.com/go-mysql-org/go-mysql/replication.(*BinlogSyncer).prepare
	/home/dvaneeden/dev/go-mysql/replication/binlogsyncer.go:401
github.com/go-mysql-org/go-mysql/replication.(*BinlogSyncer).prepareSyncPos
	/home/dvaneeden/dev/go-mysql/replication/binlogsyncer.go:692
github.com/go-mysql-org/go-mysql/replication.(*BinlogSyncer).StartSync
	/home/dvaneeden/dev/go-mysql/replication/binlogsyncer.go:452
main.main
	/home/dvaneeden/dev/go-mysql/cmd/go-mysqlbinlog/main.go:86
runtime.main
	/usr/local/go/src/runtime/proc.go:272
runtime.goexit
	/usr/local/go/src/runtime/asm_amd64.s:1700

@dveeden
Copy link
Collaborator

dveeden commented Jan 10, 2025

So it COM_REGISTER_SLAVE sends properties that are shown in SHOW REPLICAS.

With MySQL this contains:

This information is NOT used for authenticating to the replication source.

These things can be useful for things like pt-online-schema-change that want to discover replicas (with SHOW REPLICAS) and then connect to them to check for replication delay.

Note that SHOW REPLICAS doesn't show the password by default as --show-replica-auth-info is OFF by default.

@dveeden
Copy link
Collaborator

dveeden commented Jan 10, 2025

Testing shows that mysqlbinlog --read-from-remote-server ... doesn't send COM_REGISTER_SLAVE, so not doing this should be fine. However I don't think we should skip this to make sure we appear in SHOW REPLICAS output.

@dveeden
Copy link
Collaborator

dveeden commented Jan 10, 2025

Test with this in my my.cnf:

report_user='fakeuser'
report_host='fakehost'
report_password='fakepwd'
report_port=123

And with a replication source (master) running with --show-replica-auth-info:

mysql-9.1.0> show replicas;
+-----------+----------+----------+----------+------+-----------+--------------------------------------+
| Server_Id | Host     | User     | Password | Port | Source_Id | Replica_UUID                         |
+-----------+----------+----------+----------+------+-----------+--------------------------------------+
|         1 | fakehost | fakeuser | fakepwd  |  123 |       124 | 896e7882-18fe-11ef-ab88-22222d34d411 |
+-----------+----------+----------+----------+------+-----------+--------------------------------------+
1 row in set (0.00 sec)

From Wireshark:
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants