Skip to content

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

Open
woods-chen wants to merge 4 commits into
julien-duponchelle:mainfrom
woods-chen:main
Open

improve performance and documentation, skip the first 2 events after reconnected#363
woods-chen wants to merge 4 commits into
julien-duponchelle:mainfrom
woods-chen:main

Conversation

@woods-chen

Copy link
Copy Markdown
* 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

dongwook-chan commented Mar 9, 2022

Copy link
Copy Markdown
Collaborator

@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
Copy Markdown
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
Copy Markdown
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