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

Set reconnect: true in config/database.yml to avoid random timeouts du… #1023

Closed
wants to merge 1 commit into from

Conversation

svogt0511
Copy link
Contributor

@svogt0511 svogt0511 commented Oct 18, 2023

…ring db queries.

Purpose

Reconnect on DB query timeouts. This is related to the timeout that happened during a db migration operation in staging using the Departure gem (w/ percona toolset). The migration was completed but on updating the schema_migrations table to show that the migration had been done, the query timed out. This setting would allow the mysql2 client to reconnect and try the op again.

UPDATE - What are the side-effects of allowing reconnects like this. Is it safe? Would increasing the timeout period be a good alternative solution?

Approach

Open Questions and Pre-Merge TODOs

Learning

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

  • New feature (non-breaking change which adds functionality)

  • Breaking change (fix or feature that would cause existing functionality to change)

Reviewer, please remember our guidelines:

  • Be humble in the language and feedback you give, ask don't tell.
  • Consider using positive language as opposed to neutral when offering feedback. This is to avoid the negative bias that can occur with neutral language appearing negative.
  • Offer suggestions on how to improve code e.g. simplification or expanding clarity.
  • Ensure you give reasons for the changes you are proposing.

@wendelfabianchinsamy
Copy link
Contributor

wendelfabianchinsamy commented Oct 19, 2023

Some further reading suggest that this feature is deprecated in the most recent versions of MySQL https://dev.mysql.com/doc/c-api/8.0/en/c-api-auto-reconnect.html#:~:text=If%20auto%2Dreconnect%20is%20enabled%2C%20mysql_ping()%20performs%20a%20reconnect,used%20to%20suppress%20this%20behavior.

Furthermore, the reconnect option is not transaction safe and rolls back any active transactions and resets the autocommit mode (removes the implicit start transaction...commit statement around each statement that is not part of a transaction).

More info and side effects can be found on the link I've provided.

Two suggestions I've found is either upping to pool size or increasing the timeout (like @svogt051 has previously suggested).

@svogt0511 thank you for getting me to pause and think about this for a bit.

@jrhoads
Copy link
Contributor

jrhoads commented Oct 20, 2023

From @wendelfabianchinsamy comment, it seams like this PR is a no-go.
Should we go ahead an close and approach the problem another way? @svogt0511

@richardhallett
Copy link
Contributor

Ye seems like not the right way to go about this. I had thought this was a rails reconnect flag, not mysql specific.
I don't think the timeout is a huge concern for the rollout of the new column, if we check it was successfully manually.

@svogt0511
Copy link
Contributor Author

svogt0511 commented Oct 24, 2023

Closing this without merging due to the discussion in today's Development group meeting and the above comments. It was the general consensus that using the 'reconnect' option to mysql was not safe, and with the departure gem, probably unnecessary.

@svogt0511 svogt0511 closed this Oct 24, 2023
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.

4 participants