Add move-tables environment variables to hooks#1711
Conversation
danieljoos
added
the
feature-move-tables
There was a problem hiding this comment.
Pull request overview
This PR extends gh-ost’s hook execution contract to better support move-tables mode by exporting additional move-tables context as environment variables and by passing a “drain GTID” marker to the on-success hook.
Changes:
- Add move-tables-related environment variables for hook scripts (target host/db/table + move-tables mode flag).
- Add a move-tables-specific on-success hook path that passes
GH_OST_DRAIN_GTID. - Add
MigrationContext.GetTargetHostname()to consistently surface the “target” host across modes.
Show a summary per file
| File | Description |
|---|---|
| go/logic/migrator.go | Switch move-tables cutover to invoke move-tables-specific on-success hook method with drain GTID. |
| go/logic/hooks.go | Add new composite + executor hook method; extend hook env var set for move-tables context. |
| go/logic/hooks_test.go | Add tests validating new env vars and drain GTID behavior. |
| go/base/hooks.go | Extend exported base.Hooks interface with move-tables on-success callback. |
| go/base/context.go | Add GetTargetHostname() helper for safe target host retrieval. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 5/5 changed files
- Comments generated: 4
| OnBeforeCutOver() error | ||
| OnInteractiveCommand(command string) error | ||
| OnSuccess(instantDDL bool) error | ||
| OnSuccessMoveTables(drainGTID string) error |
There was a problem hiding this comment.
docs:
Don't introduce new hook types. The existing on-row-copy-complete, on-before-cut-over, and on-success hooks fire at exactly the protocol boundaries needed (P2, T0, T5).
Should we be using OnSuccess instead?
There was a problem hiding this comment.
yeah. probably best not to introduce a new interface method here.
It does call the on-success hook though. just with different env variables.
I'll work on it tomorrow
danieljoos
force-pushed
the
danieljoos-move-tables-hooks-env
branch
from
7ebd36b to
1533e8a
Compare
danieljoos
force-pushed
the
danieljoos-move-tables-hooks-env
branch
from
1533e8a to
b60e605
Compare
A Pull Request should be associated with an Issue.
Related issue (internal): https://github.com/github/database-infrastructure/issues/8211
Related issue (public): #1681
Description
This PR exposes move-tables information as environment variables to the hooks scripts:
GH_OST_TARGET_HOST: the target host, meaning the primary hosts that is written to. For move tables, that will be the primary host of the other cluster.GH_OST_MOVE_TABLES: holds"true"if gh-ost is operating in move-tables mode.GH_OST_TARGET_DATABASE_NAME: the target database name, meaning the schema/database name on the target cluster.GH_OST_TARGET_TABLE_NAME: similarly, the target table name, meaning the name of table written to.GH_OST_DRAIN_GTID: for the on-success hook in move-tables mode, this environment variable holds the last GTID written to the old table, before it was renamed.script/cibuildreturns with no formatting errors, build errors or unit test errors.