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

improve performance and documentation, skip the first 2 events after reconnected #363

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

woods-chen
Copy link

* sleep 5s after disconnected accidently and skip the first 2
  events after reconnected
* symplify BinLogStreamReader.fetchone
* improve documentation of BinLogStreamReader
* some cleanup and compacting works
* tests/test_data_type.py: fix syntax errors
* tests/test_basic.py: dynamic get an end position instead a fixed one

* symplify BinLogStreamReader.fetchone
* improve documentation of BinLogStreamReader
* tests/test_data_type.py: fix syntax errors
* tests/test_basic.py: dynamic get an end position instead a fixed one
* sleep 5s after disconnected accidently and skip the first 2
  events after reconnected
* some cleanup and compacting works
@dongwook-chan
Copy link
Collaborator

dongwook-chan commented Mar 9, 2022

@woods-chen
I love the * symplify BinLogStreamReader.fetchone feature.
I think it will greatly improve the performance of the module.
(though I haven't tested yet)

However, I'm a little concerned about * sleep 5s after disconnected accidently and skip the first 2 events after reconnected feature.
The very first event the module receives upon connection is RotateEvent, which triggers table_map to be flushed:
https://github.com/noplay/python-mysql-replication/blob/7fd706d1686da90cdb3991279c764f057e24a1f6/pymysqlreplication/binlogstream.py#L507-L519
Consider a scenario where reboot of MySQL caused disconnection.
mysql-replication will sleep for 5 seconds and skip the first RotateEvent.
Then move on without flushing table_map to be filled with table ids that is not valid any more after MySQL reboot.

Nice work with the optimization, but I think we should keep parsing the first RotateEvent for maintaining valid table_map.

@woods-chen
Copy link
Author

@dongwook-chan
Thank you for correcting me. I should have read the previous codes and comments in more detail before committing.
I only considered the situation that the network is disconnected, and sleeping 5 seconds is also for waiting for the network to be resumed.
And I pushed another commit which deleted those lines .

@dongwook-chan
Copy link
Collaborator

@woods-chen
Thank you for the correction.

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