Cleanup TODOs targeting v25 or earlier

[timvaillancourt]

Description

Cleans up 10 x TODO/deprecation items in go/ that target a Vitess version of v25 or earlier. We're now building v25, so old compat shims, debug helpers and deprecated flags are eligible for removal

What was removed

1. grpctmclient v23 compat shim in validateTablet() (simulated a missing TabletShutdownTime field via a Hostname/MysqlHostname/PortMap-nil heuristic) + the paired test case

2. --watch-replication-stream flag and the WatchReplication field in tabletenv/config.go. References cleaned up across 18 x e2e test files and config/tablet/default.yaml

3. --snapshot-topology-interval flag and the entire snapshot-topology feature in VTOrc — flag/getter, tick branch, SnapshotTopologies() and its test, the database_instance_topology_history SQLite table, e2e config field + sub-test, plus references in examples/common/vtorc/config.yaml and flags/endtoend/vtorc.txt

4. VTOrc --cell is now required — validateCell() previously logged a warning and continued; it now returns a FAILED_PRECONDITION error if --cell is empty

5. 5 x SimulatedNull non-slice checks in rpc_vreplication.go (OnDdl, State ×2, Message, StopPosition). The comments target "v22+" so they're overdue

6. Pre-v21 codes.Unimplemented branch in validateShardsHaveVReplicationPermissions() (go/vt/vtctl/workflow/server.go) + updated unit test, dropped now-unused codes/status imports

7. printCallerDetails() debug helper in go/vt/sidecardb/sidecardb.go (was tagged remove in v17 😅)

The go/vt/sidecardb/doc.go note about removing IF NOT EXISTS from the 21 x sidecar schema CREATE TABLE statements is deferred — that's a real schema change with upgrade implications and deserves its own issue/PR

Related Issue(s)

Resolves: #20047

Checklist

• "Backport to:" labels have been added if this change should be back-ported to release branches
• If this change is to be back-ported to previous releases, a justification is included in the PR description
• Tests were added or are not required
• Did the new or modified tests pass consistently locally and on CI?
• Documentation was added or is not required

Deployment Notes

3 x user-visible behaviour changes that need release-note callouts:

1. --watch-replication-stream removed — startup will fail if still passed (the flag has been deprecated and ignored for a while)
2. --snapshot-topology-interval removed and the snapshot-topology feature entirely deleted, including the database_instance_topology_history SQLite table
3. VTOrc --cell is now required — previously a warning, now a hard FAILED_PRECONDITION error if missing

AI Disclosure

This PR was written primarily by Claude Code — I provided direction on scope and reviewed. Claude also prepared this PR summary

[vitess-bot]

Review Checklist

Hello reviewers! 👋 Please follow this checklist when reviewing this Pull Request.

General

• Ensure that the Pull Request has a descriptive title.
• Ensure there is a link to an issue (except for internal cleanup and flaky test fixes), new features should have an RFC that documents use cases and test cases.

Tests

• Bug fixes should have at least one unit or end-to-end test, enhancement and new features should have a sufficient number of tests.

Documentation

• Apply the release notes (needs details) label if users need to know about this change.
• New features should be documented.
• There should be some code comments as to why things are implemented the way they are.
• There should be a comment at the top of each new or modified test to explain what the test does.

New flags

• Is this flag really necessary?
• Flag names must be clear and intuitive, use dashes (-), and have a clear help text.

If a workflow is added or modified:

• Each item in Jobs should be named in order to mark it as required.
• If the workflow needs to be marked as required, the maintainer team must be notified.

Backward compatibility

• Protobuf changes should be wire-compatible.
• Changes to _vt tables and RPCs need to be backward compatible.
• RPC changes should be compatible with vitess-operator
• If a flag is removed, then it should also be removed from vitess-operator and arewefastyet, if used there.
• vtctl command output order should be stable and awk-able.

[Copilot]

Pull request overview

This PR removes several long-expired TODOs and deprecation/compatibility shims that targeted Vitess v25 or earlier, simplifying the go/ codebase now that v25 is being built and older behaviors are no longer supported.

Changes:

• Removed deprecated/ignored flags and config fields (--watch-replication-stream, --snapshot-topology-interval) and updated related configs/tests/docs.
• Deleted VTOrc snapshot-topology functionality (logic path, DAO function/test, and associated SQLite schema entries).
• Removed outdated compatibility branches/debug helpers (grpctmclient shutdown heuristic, vreplication SimulatedNull non-slice checks, vtctl workflow pre-v21 Unimplemented handling, sidecardb debug helper) and made VTOrc --cell required.

Reviewed changes

Copilot reviewed 38 out of 38 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
go/vt/vttablet/tabletserver/tabletenv/config.go	Removes deprecated --watch-replication-stream flag wiring and WatchReplication config field.
go/vt/vttablet/tabletmanager/rpc_vreplication.go	Removes legacy non-slice SimulatedNull handling and applies minor formatting-only callsite changes.
go/vt/vttablet/grpctmclient/client.go	Drops v23-era shutdown-heuristic compatibility branch in validateTablet().
go/vt/vttablet/grpctmclient/client_test.go	Removes the test case that covered the deleted pre-v24 shutdown heuristic.
go/vt/vtorc/server/discovery.go	Makes empty --cell a hard error (FAILED_PRECONDITION).
go/vt/vtorc/logic/vtorc.go	Removes the deprecated snapshot-topology tick branch from continuous discovery.
go/vt/vtorc/inst/instance_dao.go	Deletes SnapshotTopologies() implementation.
go/vt/vtorc/inst/instance_dao_test.go	Removes the unit test for the deleted snapshot-topology feature.
go/vt/vtorc/db/generate_base.go	Removes the database_instance_topology_history table from the generated base schema.
go/vt/vtorc/config/config.go	Removes --snapshot-topology-interval config/flag plumbing and updates --cell help text.
go/vt/vtctl/workflow/server.go	Removes pre-v21 Unimplemented special-casing branch for vreplication permission validation.
go/vt/vtctl/workflow/server_test.go	Removes the corresponding “unimplemented error” test case and now-unused imports.
go/vt/sidecardb/sidecardb.go	Removes old printCallerDetails() debug helper and its call.
go/test/endtoend/vtorc/api/config_test.go	Removes e2e coverage for the deleted dynamic snapshot-topology-interval config.
go/test/endtoend/vtgate/foreignkey/stress/fk_stress_test.go	Removes deprecated --watch-replication-stream usage from e2e args.
go/test/endtoend/vault/vault_test.go	Removes deprecated --watch-replication-stream usage from e2e args.
go/test/endtoend/topoconncache/main_test.go	Removes deprecated --watch-replication-stream usage from e2e args.
go/test/endtoend/tabletmanager/throttler_topo/throttler_test.go	Removes deprecated --watch-replication-stream usage from e2e args.
go/test/endtoend/tabletmanager/tablegc/tablegc_test.go	Removes deprecated --watch-replication-stream usage from e2e args.
go/test/endtoend/tabletmanager/primary/tablet_test.go	Removes deprecated --watch-replication-stream usage from e2e args.
go/test/endtoend/tabletmanager/main_test.go	Removes deprecated --watch-replication-stream usage from e2e args.
go/test/endtoend/schemadiff/vrepl/schemadiff_vrepl_suite_test.go	Removes deprecated --watch-replication-stream usage from e2e args.
go/test/endtoend/recovery/unshardedrecovery/recovery.go	Removes deprecated --watch-replication-stream usage from e2e args.
go/test/endtoend/onlineddl/vrepl/onlineddl_vrepl_test.go	Removes deprecated --watch-replication-stream usage from e2e args.
go/test/endtoend/onlineddl/vrepl_suite/onlineddl_vrepl_suite_test.go	Removes deprecated --watch-replication-stream usage from e2e args.
go/test/endtoend/onlineddl/vrepl_stress/onlineddl_vrepl_mini_stress_test.go	Removes deprecated --watch-replication-stream usage from e2e args.
go/test/endtoend/onlineddl/vrepl_stress_suite/onlineddl_vrepl_stress_suite_test.go	Removes deprecated --watch-replication-stream usage from e2e args.
go/test/endtoend/onlineddl/scheduler/onlineddl_scheduler_test.go	Removes deprecated --watch-replication-stream usage from e2e args.
go/test/endtoend/onlineddl/revert/onlineddl_revert_test.go	Removes deprecated --watch-replication-stream usage from e2e args.
go/test/endtoend/onlineddl/flow/onlineddl_flow_test.go	Removes deprecated --watch-replication-stream usage from e2e args.
go/test/endtoend/cluster/vtorc_process.go	Removes snapshot-topology-interval from VTOrc e2e configuration struct.
go/test/endtoend/cellalias/cell_alias_test.go	Removes deprecated --watch-replication-stream usage from e2e args.
go/test/endtoend/backup/vtctlbackup/backup_utils.go	Removes deprecated --watch-replication-stream usage from e2e args.
go/test/endtoend/backup/vtbackup/main_test.go	Removes deprecated --watch-replication-stream usage from e2e args.
go/test/endtoend/backup/clone/main_test.go	Removes deprecated --watch-replication-stream usage from e2e args.
go/flags/endtoend/vtorc.txt	Removes the deleted --snapshot-topology-interval from the end-to-end flags list.
examples/common/vtorc/config.yaml	Removes snapshot-topology-interval from example VTOrc config.
config/tablet/default.yaml	Removes watchReplication from default tablet config.

[codecov]

Codecov Report

❌ Patch coverage is 44.82759% with 16 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.12%. Comparing base (70c7a72) to head (84a4512).
⚠️ Report is 245 commits behind head on main.

Files with missing lines	Patch %	Lines
go/vt/vttablet/tabletmanager/rpc_vreplication.go	47.82%	12 Missing ⚠️
go/vt/vtorc/config/config.go	0.00%	3 Missing ⚠️
go/vt/vtorc/server/discovery.go	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #20048      +/-   ##
==========================================
+ Coverage   69.67%   72.12%   +2.45%     
==========================================
  Files        1614     1349     -265     
  Lines      216793   204776   -12017     
==========================================
- Hits       151044   147696    -3348     
+ Misses      65749    57080    -8669     

Flag	Coverage Δ
partial	72.12% <44.82%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[Copilot]

Pull request overview

Copilot reviewed 39 out of 39 changed files in this pull request and generated 2 comments.

[mattlord]

Now that the SimulatedNull guard is removed (which is done here in this PR), this path in go/vt/vttablet/tabletmanager/rpc_vreplication.go:608-610 treats any non-nil req.State as valid and maps it through VReplicationWorkflowState_name[int32(*req.State)]. Unknown enum values map to the empty string, so a malformed or stale client request can persist state = '' into _vt.vreplication. The bulk update path already checks the map ok result and returns INVALID_ARGUMENT; this path should do the same before building the stream update query. We can copy that existing handling:

		state, ok := binlogdatapb.VReplicationWorkflowState_name[int32(req.GetState())]
		if !ok {
			return "", vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "invalid state value: %v", req.GetState())
		}

Approving now, but please do that before merging. Thanks!

[Copilot]

Pull request overview

Copilot reviewed 45 out of 45 changed files in this pull request and generated no new comments.