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

replication: Improve COM_REGISTER_SLAVE #967

Merged
merged 2 commits into from
Jan 10, 2025
Merged

replication: Improve COM_REGISTER_SLAVE #967

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dd23731
replication: Improve COM_REGISTER_SLAVE
dveeden Jan 10, 2025
6cbab12
replication: Fix replica UUID registration
dveeden Jan 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 10 additions & 12 deletions replication/binlogsyncer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -348,14 +348,6 @@ func (b *BinlogSyncer) registerSlave() error {
}
}

if err = b.writeRegisterSlaveCommand(); err != nil {
return errors.Trace(err)
}

if _, err = b.c.ReadOKPacket(); err != nil {
return errors.Trace(err)
}

serverUUID, err := uuid.NewUUID()
if err != nil {
b.cfg.Logger.Errorf("failed to get new uuid %v", err)
Expand All @@ -366,6 +358,14 @@ func (b *BinlogSyncer) registerSlave() error {
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)
}

return nil
}

Expand Down Expand Up @@ -596,7 +596,7 @@ func (b *BinlogSyncer) writeRegisterSlaveCommand() error {
hostname := b.localHostname()

// This should be the name of slave host not the host we are connecting to.
data := make([]byte, 4+1+4+1+len(hostname)+1+len(b.cfg.User)+1+len(b.cfg.Password)+2+4+4)
data := make([]byte, 4+1+4+1+len(hostname)+1+len(b.cfg.User)+1+2+4+4)
pos := 4

data[pos] = COM_REGISTER_SLAVE
Expand All @@ -616,10 +616,8 @@ func (b *BinlogSyncer) writeRegisterSlaveCommand() error {
n = copy(data[pos:], b.cfg.User)
pos += n

data[pos] = uint8(len(b.cfg.Password))
data[pos] = uint8(0)
pos++
n = copy(data[pos:], b.cfg.Password)
Copy link
Collaborator

Choose a reason for hiding this comment

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

😂 Don't know why we bother to send password before. Does different version of server has different behaviour?

Will take a look soon

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh interesting, we don't need to send the password?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The password is only for use in the output of SHOW REPLICAS if --show-replica-auth-info is enabled. This makes it a bit of a security hazard and with the length limitation also quite useless.

pos += n

binary.LittleEndian.PutUint16(data[pos:], b.cfg.Port)
pos += 2
Expand Down
Loading