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

Expose host and port in BinlogRequest builder. #96

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

darnuria
Copy link

@darnuria darnuria commented Jul 20, 2023

Hello! Thanks for the crate!

The host and port were already exposed but unused in ComRegisterSlave, so I did a little patch to expose them in BinlogRequest, so use them with the async/not async crate.

Linked to blackbeam/mysql_async#250

It's used for reporting in command like

show slave hosts;
+-----------+------------+------+-----------+--------------------------------------+
| Server_id | Host       | Port | Master_id | Slave_UUID                           |
+-----------+------------+------+-----------+--------------------------------------+
| 42        | stage.test | 3306 |      4214 | 88cd28ee-ffff-11ed-b7a8-171e928e6282 |
+-----------+-------------------+-----------+--------------------------------------+

There is also master_heartbeat_period and slave_uuid but they are settable outside of the ComRegisterSlave packet with:

  • slave_uuid, it's settable with set @slave_uuid='88cd28ee-ffff-11ed-b7a8-171e928e6282'"
  • master_heartbeat_period set @master_heartbeat_period=<period in nanosec u64>

These two last are from mysql-internals if you are interested you can checkout how it's done in pymysqlreplication. I didn't included them since they are settable with queries.

I am writing a naive patch for mysql_async. :)

darnuria added a commit to darnuria/mysql_async that referenced this pull request Jul 20, 2023
Followup of blackbeam/rust_mysql_common#96
it's possible to inspect them with show slave hosts/ show replicas
The host and port were already exposed but unused in ComRegisterSlave.
@darnuria darnuria force-pushed the expose_more_replica_information branch from ce5a7d5 to 4f0fe61 Compare July 20, 2023 15:56
darnuria added a commit to darnuria/mysql_async that referenced this pull request Jul 20, 2023
Followup of blackbeam/rust_mysql_common#96
it's possible to inspect them with show slave hosts/ show replicas
@blackbeam
Copy link
Owner

Hi.

This seem to be unnecessary, because hostname and port is a part of COM_REGISTER_SLAVE rather then COM_BINLOG_DUMP[...] 🤔

@blackbeam
Copy link
Owner

Oh, I see the point.

I don't like the idea of putting this unrelated data into the BinlogRequest because, as stated in the docs, it's a COM_BINLOG_DUMP builder. I now see that I shouldn't have exposed BinlogRequest in the mysql_async API.

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.

2 participants