[pull] master from github:master #1
Open
Conversation
This file contains hidden or 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
Upgrade codeQL action to v2
* pin stretch image * update stretch repo * Update Dockerfile.test
Co-authored-by: Tim Vaillancourt <[email protected]>
Signed-off-by: Tim Vaillancourt <[email protected]>
* WIP * Pass entire sql.UniqueKey * newline for limit * Rename var --------- Co-authored-by: meiji163 <[email protected]>
* Include git commit in version Signed-off-by: Tim Vaillancourt <[email protected]> * Make --version output change less-breaking --------- Signed-off-by: Tim Vaillancourt <[email protected]>
* Cleanup whitespace in SQL query text * cleanup * Add indent * Update unit tests * Update unit tests, pt 2 * Fix tweaks * Fix merge conflict resolution Signed-off-by: Tim Vaillancourt <[email protected]> --------- Signed-off-by: Tim Vaillancourt <[email protected]>
* Fix `--version` output Signed-off-by: Tim Vaillancourt <[email protected]> * Always fallback version/commit if undef Signed-off-by: Tim Vaillancourt <[email protected]> --------- Signed-off-by: Tim Vaillancourt <[email protected]>
* go1.21 + bullseye Signed-off-by: Tim Vaillancourt <[email protected]> * go1.21 + bullseye pt 2 Signed-off-by: Tim Vaillancourt <[email protected]> * checkout before setup-go Signed-off-by: Tim Vaillancourt <[email protected]> * go fmt Signed-off-by: Tim Vaillancourt <[email protected]> * Use golangci-lint 1.54.2 to support go1.21 Signed-off-by: Tim Vaillancourt <[email protected]> * stop using io/ioutil to make linter happy Signed-off-by: Tim Vaillancourt <[email protected]> * Fix typo Signed-off-by: Tim Vaillancourt <[email protected]> * Lint Signed-off-by: Tim Vaillancourt <[email protected]> * revert replica-tests CI to ubuntu 20 due to linker errors Signed-off-by: Tim Vaillancourt <[email protected]> * Update ensure-go-installed * use `ubuntu-latest` for `ci` job Signed-off-by: Tim Vaillancourt <[email protected]> * stretch -> bullseye Signed-off-by: Tim Vaillancourt <[email protected]> --------- Signed-off-by: Tim Vaillancourt <[email protected]> Signed-off-by: Tim Vaillancourt <[email protected]>
Signed-off-by: Tim Vaillancourt <[email protected]>
Signed-off-by: Tim Vaillancourt <[email protected]>
Remove `@dm-2` as I am no longer a maintainer for gh-ost.
Update CODEOWNERS
* Add cpu-profile interactive command * better doc markdown Signed-off-by: Tim Vaillancourt <[email protected]> * set block profile after isProfiling=1 Signed-off-by: Tim Vaillancourt <[email protected]> * improve test Signed-off-by: Tim Vaillancourt <[email protected]> * check isCPUProfiling later Signed-off-by: Tim Vaillancourt <[email protected]> * Cleanup Signed-off-by: Tim Vaillancourt <[email protected]> * Fix discrepancy Signed-off-by: Tim Vaillancourt <[email protected]> * move base64 to .applyServerCommand(...) Signed-off-by: Tim Vaillancourt <[email protected]> --------- Signed-off-by: Tim Vaillancourt <[email protected]> Co-authored-by: meiji163 <[email protected]>
* feat(feat-config-connection-charset): support config connection charset * feat(feat-config-connection-charset): feat-config-connection-charset The ability to specify character set and collation is supported. --------- Co-authored-by: shaohoukun <[email protected]>
Co-authored-by: meiji163 <[email protected]>
* Bump actions/upload-artifact from 1 to 4 Bumps [actions/upload-artifact](https://github.com/actions/upload-artifact) from 1 to 4. - [Release notes](https://github.com/actions/upload-artifact/releases) - [Commits](actions/upload-artifact@v1...v4) --- updated-dependencies: - dependency-name: actions/upload-artifact dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <[email protected]> * update codeQL action --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: meiji163 <[email protected]>
Bumps [actions/setup-go](https://github.com/actions/setup-go) from 4 to 5. - [Release notes](https://github.com/actions/setup-go/releases) - [Commits](actions/setup-go@v4...v5) --- updated-dependencies: - dependency-name: actions/setup-go dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: meiji163 <[email protected]>
Bumps [golangci/golangci-lint-action](https://github.com/golangci/golangci-lint-action) from 3 to 4. - [Release notes](https://github.com/golangci/golangci-lint-action/releases) - [Commits](golangci/golangci-lint-action@v3...v4) --- updated-dependencies: - dependency-name: golangci/golangci-lint-action dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
) * Update connection.DuplicateCredentials function to set correct ServerName property Ensure the ServerName TLS property matches the new connection instance key hostname to avoid TLS verify errors like the following: 2025-01-02 02:07:26 FATAL tls: failed to verify certificate: x509: certificate is valid for [old host], not [new host] This is only one part of the fix for this issue. The second part, registering TLS Config with the mysql driver, will come in subsequent commits. * Use connection.DuplicateCredentials in cases where the connection key changes * Extract TLS config key name generation to GetDBTLSConfigKey function * Extract function to register a connection's TLS config with the mysql driver This allows us to register TLS configuration is the various places where connection configs are created and before they're used. * Register TLS config when setting up master connection info This ensures that the master's TLS config has been registered with the mysql driver before any connections are attempted. This is the second part of resolving the following TLS verify error: 2025-01-02 02:07:26 FATAL tls: failed to verify certificate: x509: certificate is valid for [old host], not [new host] * Register TLS config when setting up the throttler's connection info This ensures that the throttler's TLS config has been registered with the mysql driver before any connections are attempted. This is the second part of resolving the following TLS verify error: 2025-01-02 02:07:26 FATAL tls: failed to verify certificate: x509: certificate is valid for [old host], not [new host] --------- Co-authored-by: meiji163 <[email protected]>
* Fix clear-text logging of connection config Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> * don't log connection configs --------- Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
* Add trigger support to gh-ost based on openark#30 * Add comprehensive test cases for trigger support functionality * Fix trigger-basic test by adding required --trigger-suffix parameter and remove fail-trigger-unsupported test * Update trigger suffix in local test configurations Modify extra_args files for trigger-complex, trigger-multiple, and trigger-self-reference tests to use different trigger suffixes * Add --remove-trigger-suffix-if-exists to local test configurations Update extra_args files for trigger tests to include the new --remove-trigger-suffix-if-exists option, ensuring consistent trigger handling across different test scenarios * Fix trigger-long-name test by reducing trigger name length to be valid * Standardize trigger drop statements in local test configurations Update create.sql files across trigger test scenarios to: - Consistently drop both original and ghost triggers - Ensure clean slate before creating triggers - Align with recent trigger suffix changes This change improves test reliability and consistency by explicitly dropping all potential trigger variations before test setup. * Consolidate and enhance trigger test configurations Refactor local test scenarios for triggers by: - Merging multiple trigger test configurations into comprehensive test cases - Updating create.sql files with more complex and diverse trigger scenarios - Standardizing trigger and table setup across different test configurations - Removing redundant test directories while preserving test coverage This change simplifies the trigger testing infrastructure and provides more robust test coverage for gh-ost's trigger handling capabilities. * Add debug logging for ghost trigger validation process Enhance ghost trigger existence check by adding detailed debug logging to: - Log the ghost trigger name being searched - Log the database schema and query details - Log when an existing ghost trigger is found This change improves visibility into the trigger validation process, making troubleshooting easier during migration scenarios. * Improve ghost trigger validation with enhanced logging and verification Modify validateGhostTriggersDontExist() to: - Add a direct query to log all triggers in the database schema - Refactor trigger existence check to use count-based query - Improve debug logging for trigger validation process - Provide more detailed error reporting for existing ghost triggers This change enhances the robustness and observability of the trigger validation mechanism in gh-ost's migration process. * Refactor ghost trigger validation to improve logging and error detection Simplify and enhance the validateGhostTriggersDontExist() method by: - Removing redundant direct query logging - Streamlining trigger existence check - Improving debug logging for trigger validation - Consolidating trigger existence detection logic The changes provide more concise and focused trigger validation with clearer error reporting. * Simplify ghost trigger validation query and reduce logging verbosity Refactor validateGhostTriggersDontExist() to: - Streamline trigger existence check query - Remove redundant debug logging statements - Use a more concise approach to detecting existing triggers The changes reduce code complexity while maintaining the core validation logic for ghost triggers. * Enhance ghost trigger validation query to include table name filter Modify validateGhostTriggersDontExist() to: - Add table name filter to trigger existence check query - Improve specificity of ghost trigger detection - Prevent false positives from similarly named triggers in different tables The change ensures more precise ghost trigger validation by incorporating the original table name into the query criteria. * Enhance trigger test configuration with advanced features and consolidated test scenarios Update trigger-advanced-features test configuration to: - Add new column 'color' and 'modified_count' to test table - Implement more complex trigger logic for color and count tracking - Consolidate trigger test scenarios with richer data transformations - Modify event to test both numeric and color-based updates - Remove redundant trigger-basic-features directory The changes provide a more comprehensive and nuanced test suite for gh-ost's trigger handling capabilities, demonstrating advanced trigger behaviors and self-referencing updates. * Fix lint errors * Update trigger test configuration with suffix change Modify the extra_args file to use '_ght' trigger suffix instead of '_gho', maintaining consistency with recent trigger test configuration updates. * Remove gh-ost-ci-env submodule Clean up repository by removing the gh-ost-ci-env submodule, which appears to be no longer needed in the project structure. * Update create.sql --------- Co-authored-by: Yakir Gibraltar <[email protected]>
* Bump golang.org/x/net in the go_modules group across 1 directory Bumps the go_modules group with 1 update in the / directory: [golang.org/x/net](https://github.com/golang/net). Updates `golang.org/x/net` from 0.33.0 to 0.36.0 - [Commits](golang/net@v0.33.0...v0.36.0) --- updated-dependencies: - dependency-name: golang.org/x/net dependency-type: direct:production dependency-group: go_modules ... Signed-off-by: dependabot[bot] <[email protected]> * go mod tidy * Remove toolchain directive from go.mod * fix go version * try update golangci * remove deprecated linters * remove unused nolint comment --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: meiji163 <[email protected]>
…ss (#1500) * First pass at raise_on_warnings * Update dev.yml * add CLI option, ignore expected warnings, check for row count discrepancy * Count rows in each insert range prepared query - log insert warnings always - terminate if row count doesn't match and the PanicOnWarnings flag is set * Lint comments - ensure errors are handled in show warnings * update TestBuildUniqueKeyRangeEndPreparedQuery * Localtests for PanicOnWarnings with data loss * Unwrap CTE (mysql 5) * Update localtests/panic-on-warnings-duplicate-unique-values-on-column-type-change/extra_args Co-authored-by: Bastian Bartmann <[email protected]> * limit BuildUniqueKeyRangeEndPreparedQueryViaOffset subquery properly * Update Applier to support all unique indices with PanicOnWarnings. Add test coverage. * Impl code review feedback for PanicOnWarnings - documentation - remove dev.yml - remove unused variable * bump golangci-lint for local dev * Support altering index names with PanicOnWarnings * Fix string matching for PanicOnWarnings to correctly suppress warnings when renaming unique keys Error message formats are different across mysql distributions and versions --------- Co-authored-by: Bastian Bartmann <[email protected]>
* fix: move successful cleanup into func * fix: ignore linter error * fix: inline onSuccessFunc function
…_modules group across 1 directory (#1513) * Bump github.com/containerd/containerd Bumps the go_modules group with 1 update in the / directory: [github.com/containerd/containerd](https://github.com/containerd/containerd). Updates `github.com/containerd/containerd` from 1.7.18 to 1.7.27 - [Release notes](https://github.com/containerd/containerd/releases) - [Changelog](https://github.com/containerd/containerd/blob/main/RELEASES.md) - [Commits](containerd/containerd@v1.7.18...v1.7.27) --- updated-dependencies: - dependency-name: github.com/containerd/containerd dependency-type: indirect dependency-group: go_modules ... Signed-off-by: dependabot[bot] <[email protected]> * Remove toolchain directive from go.mod --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: meiji163 <[email protected]>
* use docker for ci tests * set charset server * remove unusued cibuild-gh-ost-replica-tests * add doc for how to run localtests
Bumps the go_modules group with 1 update in the / directory: [golang.org/x/net](https://github.com/golang/net). Updates `golang.org/x/net` from 0.36.0 to 0.38.0 - [Commits](golang/net@v0.36.0...v0.38.0) --- updated-dependencies: - dependency-name: golang.org/x/net dependency-version: 0.38.0 dependency-type: direct:production dependency-group: go_modules ... Signed-off-by: dependabot[bot] <[email protected]>
…-bbb8b02913 Bump golang.org/x/net from 0.36.0 to 0.38.0 in the go_modules group across 1 directory
…roblems with --panic-on-warnings (#1557) * Fix CalculateNextIterationRangeEndValues order by. The split between Temptable and Offset query builders was originally introduced in #471. Then, both query builders have been changed to calculate actual chunk sizes in #1500. The updated implementation of the Offset query introduced a bug, where a missing `order by` clause in `select_osc_chunk` can result in end values potentially surpassing the chunk_size. This wasn't detected during our initial testing where the query just returned rows in their original order, but may happen in real-world scenarios in case the db returns data in an undefined order. An obvious fix would be to just add an `order by` to the Offset builder subquery, however since both builders use temptables now, it makes more sense to simplify and use only one of them. Alternatively, the builder could only use the less performant count query variants when `--panic-on-warnings` is enabled and otherwise use the simpler ones from pull/471. We decided to not follow this path for now, hoping `--panic-on-warnings` becomes an updated and safer default in the future. Co-authored-by: Bastian Bartmann <[email protected]> * Fix builder's chunk order and drop panic-on-warnings row count check - Removed count subqueries from range builders to restore the more performant approach from PR #471 - Modified panic-on-warnings logic to trigger errors based solely on SQL warnings, not on row count mismatches - This addresses potential race conditions where row count comparisons could produce false positives due to concurrent table modifications --------- Co-authored-by: Bastian Bartmann <[email protected]>
When migrating a table with a `binary` key in `-verbose` mode, different sequences of bytes may prevent execution output from being logged, or correctly logged, to the console. This happens because the `MigrationRange*Values` are printed to the screen without any type of encoding. One particularly problematic sequence is `0x27`, `Escape`, which causes the terminal to stop logging for the duration of the migration: ``` 2025-05-23 11:53:27 INFO Listening on unix socket file: /tmp/gh-ost.test.binfoo.sock 2025-05-23 11:53:27 INFO Intercepted changelog state ReadMigrationRangeValues 2025-05-23 11:53:27 INFO Handled changelog state ReadMigrationRangeValues 2025-05-23 11:53:27 INFO Migration min values: [ ``` This commit changes the to-string rendering for `binary` keys, rendering the values as a hex string rather than an unescaped series of bytes. Co-authored-by: meiji163 <[email protected]>
* add copier test, upgrade testcontainers * vendor * test composite PK
#1536) * Before the successful renaming, a session accessed the ghost table, which had already unlocked the original table. There is a very small probability that other sessions dml operations on the original table will occur, and this dml operation will appear in the original table after renaming, resulting in data loss. --------- Co-authored-by: dbking <[email protected]> Co-authored-by: meiji163 <[email protected]>
* Add postpone-cut-over-flag-file interactive command * Apply suggestions from code review Co-authored-by: Copilot <[email protected]> * fix import error --------- Co-authored-by: Copilot <[email protected]> Co-authored-by: meiji163 <[email protected]>
* Fix conversion bug when string data comes from binlog Fixes #1568 When using gh-ost to migrate a table from latin1 to utf8mb3 character encoding, the initial data copy works correctly, but new data with special characters inserted during the migration via binlog replication fails with "Incorrect string value" errors. The reason for this is that the data is a binary byte array when converted from the binlog, so the character set conversion is not applied. This fix updates the character set conversion logic to apply to both string and []uint8 types when the column has a character set conversion. I added a new test for latin1 input to this method and confirmed that the reproduction from the linked issue is fixed with this change. * Update whitespace formatting
* add sysbench localtest * fix table name * Apply suggestion from @Copilot Co-authored-by: Copilot <[email protected]> * ensure cleanup --------- Co-authored-by: Copilot <[email protected]>
Bumps [actions/setup-go](https://github.com/actions/setup-go) from 5 to 6. - [Release notes](https://github.com/actions/setup-go/releases) - [Commits](actions/setup-go@v5...v6) --- updated-dependencies: - dependency-name: actions/setup-go dependency-version: '6' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: meiji163 <[email protected]>
…ly (#1594) * Panic if InitiateHeartbeat exhausts retries to avoid looping infinitely. Based on experience, if the writer database fails inbeetween the copy & cutover stages (e.g. during cutover pause), the heartbeat writes will fail and stop, then leading to throttled state and an infinite loop of throttler.shouldThrottle(). Since this state is irrecoverable, make the heartbeat writer panic if retries are exhausted, so that the migration can fail and be restarted later. * Add sysbench localtest (#1590) * add sysbench localtest * fix table name * Apply suggestion from @Copilot Co-authored-by: Copilot <[email protected]> * ensure cleanup --------- Co-authored-by: Copilot <[email protected]> * add toxiproxy option for localtests (#1591) --------- Co-authored-by: Jan Grodowski <[email protected]>
* WIP: add GTID support * Cleanp * Add doc for flag * Rename GTIDSet var * Fix GTID SID parsing * Cleanup * Add to docs * Require enforce_gtid_consistency=ON * Rename validator func * simplify check in validateBinlogsAndGTID() * Only update GTIDSet if there was no err * Simplify GTIDEvent -> GTIDSet * Simplify GTIDEvent -> GTIDSet further, resolve go.uuid dep issue * Fix UUIDSet GNO * Add .ParseGTIDBinlogCoordinates() * Add missing smaller-than/equal logic * Comment-out WIP test * Fail on SID change * Fix import err * Add missing .Equals() check * Fix .SmallerThan()/.SmallerThanEquals() funcs and tests * Fix type change issues * Fix panics * Cleanup .SmallerThan() * Add missing check in .Equals() * Add large GTID sets to test * Simplify if cond * Add localtest/gtid * simplify localtest gtid_mode check * print gtid config * Fix var typo * support enforce_gtid_consistency=1 * fix test * Handle PreviousGTIDsEvent * gofmt * Create `validateGTIDConfig` func * Update copyrights * Update copyrights pt 2 * Copyrights again * Update docs * WIP * WIP * fix BinlogCoordinates interface usage * add binlog stream retry and toxiproxy test * fix max retry * use AddGTID instead of AddSet * remove previous GTID event handling * use AddSet * modify GTID coord tracking * fix last trx coords * re-enable Binlogsyncer retry * fix localtest arg * remove unneccesary coordinate check * rm unused funcs * add back ReplicaTermFor * Update localtests/test.sh Co-authored-by: Copilot <[email protected]> --------- Co-authored-by: Tim Vaillancourt <[email protected]> Co-authored-by: Tim Vaillancourt <[email protected]> Co-authored-by: Copilot <[email protected]>
Co-authored-by: meiji163 <[email protected]>
* add Checkpoint table and read/write funcs * handle no checkpoints returned * store min and max range values in checkpoint * resume from checkpoint * add checkpoint file * fix unique key args * update applier coordinates from _ghc heartbeat * fix test * fix linter * make checkpoint interval configurable * write checkpoint iteration number * store rows copied & dml applied * truncate column name if necessary * drop checkpoint table for final cleanup * add docs * add resume doc
* Parse binlog file numbers numerically instead of lexicographically to correctly order files like binlog.999999 < binlog.1000000. Would cause the stream to ignore all incoming events and render the gh-ost process stuck: https://github.com/github/gh-ost/blob/48b34bcbfde730b2548d598dee98e9c1f0d2fcce/go/binlog/gomysql_reader.go#L85-L88 Possibly remediated by 005043d too, which drops the SmallerThanOrEqual check from GoMySqlReader.handleRowsEvent * Remove unused fn FileBinlogCoordinates.FileSmallerThan
…n permissions (#1597) * Potential fix for code scanning alert no. 5: Workflow does not contain permissions As part of the organization's transition to default read-only permissions for the GITHUB_TOKEN, this pull request addresses a missing permission in the workflow that triggered a code scanning alert. This PR explicitly adds the required read permissions to align with the default read only permission and is part of a larger effort for this OKR github/security-services#455 Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> * Potential fix for code scanning alert no. 3: Workflow does not contain permissions adding to existing branch, existing PR for similar alert Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> --------- Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> Co-authored-by: Jason White <[email protected]> Co-authored-by: meiji163 <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
33 participants
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.
See Commits and Changes for more details.
Created by
pull[bot]
Can you help keep this open source service alive? 💖 Please sponsor : )