Skip to content

refine usage of MaxReconnectAttempts in BinlogSyncer #417

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

Merged

Conversation

amyangfei
Copy link
Contributor

@amyangfei amyangfei commented Sep 2, 2019

Refine binlog synchronization retry strategy, make it possible for user to disable retry sync or retry forever.
After this pr the default retry strategy for BinlogSyncer is no retry (if MaxReconnectAttempts is not configurated and use default value zero)

  • add DisableRetrySync option, the default behavior for retry strategy is not changed, and user also can disable retry by setting DisableRetrySync to true

@amyangfei
Copy link
Contributor Author

PTAL @siddontang @GregoryIan @csuzhangxc

@amyangfei amyangfei force-pushed the refine-sync-retry-strategy branch from a73b98a to eba6a01 Compare September 2, 2019 02:22
@IANTHEREAL
Copy link
Collaborator

LGTM. we need this DisableRetrySync to cover the incorrect case in caller program ref #414 , and we will push to fix this bug later

MaxReconnectAttempts int

// whether disable re-sync for broken connection
DisableRetrySync bool
Copy link
Collaborator

Choose a reason for hiding this comment

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

setting MaxReconnectAttempts = 0 is not enough?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I prefer changing the way of MaxReconnectAttempts

Copy link
Contributor Author

@amyangfei amyangfei Sep 2, 2019

Choose a reason for hiding this comment

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

then what about

  • MaxReconnectAttempts < 0, no retry
  • MaxReconnectAttempts = 0, infinite retry
  • MaxReconnectAttempts > 0, retry for MaxReconnectAttempts time

Copy link
Collaborator

Choose a reason for hiding this comment

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

ok, for < 0, we can change the value to MaxInt

@siddontang siddontang merged commit f52d30c into go-mysql-org:master Sep 2, 2019
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.

None yet

3 participants