MySQL replication fixes #170
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
🦋 Changeset detectedLatest commit: 5666fb2 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
stevensJourney
approved these changes
Jan 1, 2025
Rentacookie
reviewed
Jan 6, 2025
| }); | ||
| // MAX_EXECUTION_TIME(0) hint disables execution timeout for this query | ||
| const query = connection.query(`SELECT /*+ MAX_EXECUTION_TIME(0) */ * FROM ${escapeMysqlTableName(table)}`); | ||
| const stream = query.stream(); |
There was a problem hiding this comment.
This is a nice simplification to the streaming process! 🥳
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes some issues with MySQL replication:
Timestamps: Timezone handling
Timezone in timestamps were not handled correctly if the MySQL server is configured with a timezone other than UTC. The issue is that MySQL always returns timestamps in the session timezone, without any timezone indicator. So there is no way to know what we should configure the client
timezonewith if we don't know the server timezone.The current tests also stored timestamps in the default/server timezone, but expected them to be returned in UTC timezone.
This changes:
+00:00timezone in literals.SET time_zone = "+00:00". This does not appear to be needed for zongji.Timestamps: Updates broken
When updating a row containing an update, the row was converted in-place to the SQLite format, then converted again. The first pass converted the timestamp to a string, then the second pass failed since it expected a Date. This results in an error
row[key].toISOString is not a function.This fixes the conversion to not modify the original row, and also avoids doing the conversion twice.
Timestamps: Handling invalid dates
With MySQL, it is possible to disable strict mode and then persist a timestamp such as
2024-00-00 00:00:00. This is not a valid Date in JavaScript, and results in an errorRangeError: Invalid time valuewhen callingtoISOString().This fixes the issue by converting these to null.
Initial replication: Improved error handling
The implementation of snapshot handling did not handle errors properly in all cases (such as the above issue) - it resulted in an unhandled promise rejection, which could cause either the process to crash, or replication to stall.
This changes the snapshot query to using a Readable stream, which does not have this issue.
Resuming replication
When replication is resumed after a restart, the list of tables were not loaded, resulting in an empty list of tables in
includeSchema, and no changes replicated. This now loads the list of tables when resuming replication.Quoting table names
This now properly quotes table names with backticks in snapshot queries, avoiding potential issues when a table name contains special characters for some reason. Unlike with Postgres, this is not required matching the table name case, but is still better to do either way.