Remove v2 resharding fields

[arvind-murty]

Description

sharding_column_name and sharding_column_type are no longer used by resharding, so they can be deleted.

Related Issue(s)

Fixes #10378

Checklist

• "Backport me!" label has been added if this change should be backported
• Tests were added or are not required
• Documentation was added or is not required

Deployment Notes

[github-actions]

Review Checklist

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

General

• Ensure that the Pull Request has the correct release notes label. release notes none should only be used for PRs that are so trivial that they need not be included.
• If a new flag is being introduced, review whether it is really needed. The flag names should be clear and intuitive (as far as possible), and the flag's help should be descriptive.

Bug fixes

• There should be at least one unit or end-to-end test.
• The Pull Request description should either include a link to an issue that describes the bug OR an actual description of the bug and how to reproduce, along with a description of the fix.

Non-trivial changes

• There should be some code comments as to why things are implemented the way they are.

New/Existing features

• Should be documented, either by modifying the existing documentation or creating new documentation.
• New features should have a link to a feature request issue or an RFC that documents the use cases, corner cases and test cases.

Backward compatibility

• Protobuf changes should be wire-compatible.
• Changes to _vt tables and RPCs need to be backward compatible.
• vtctl command output order should be stable and awk-able.

[deepthi]

@rohit-nayak-ps vtctld_web_test is failing on this PR. Do we need to regenerate the rice-box.go for these changes? Is that still possible?

2022-06-02T00:53:12.6438684Z     vtctld_web_main_test.go:481: 
2022-06-02T00:53:12.6439138Z         	Error Trace:	vtctld_web_main_test.go:481
2022-06-02T00:53:12.6439701Z         	            				vtctld_web_test.go:127
2022-06-02T00:53:12.6440023Z         	Error:      	elements differ
2022-06-02T00:53:12.6440310Z         	            	
2022-06-02T00:53:12.6440745Z         	            	extra elements in list B:
2022-06-02T00:53:12.6441187Z         	            	([]interface {}) (len=2) {
2022-06-02T00:53:12.6442053Z         	            	 (string) (len=29) "-sharding_column_name=test_id",
2022-06-02T00:53:12.6442783Z         	            	 (string) (len=28) "-sharding_column_type=UINT64"
2022-06-02T00:53:12.6443104Z         	            	}
2022-06-02T00:53:12.6443368Z         	            	
2022-06-02T00:53:12.6443622Z         	            	
2022-06-02T00:53:12.6443910Z         	            	listA:
2022-06-02T00:53:12.6444308Z         	            	([]string) (len=3) {
2022-06-02T00:53:12.6444834Z         	            	 (string) (len=14) "CreateKeyspace",
2022-06-02T00:53:12.6445445Z         	            	 (string) (len=12) "-force=false",
2022-06-02T00:53:12.6445953Z         	            	 (string) (len=14) "test_keyspace3"
2022-06-02T00:53:12.6446254Z         	            	}
2022-06-02T00:53:12.6446515Z         	            	
2022-06-02T00:53:12.6447017Z         	            	
2022-06-02T00:53:12.6447309Z         	            	listB:
2022-06-02T00:53:12.6447705Z         	            	([]string) (len=5) {
2022-06-02T00:53:12.6448204Z         	            	 (string) (len=14) "CreateKeyspace",
2022-06-02T00:53:12.6448909Z         	            	 (string) (len=29) "-sharding_column_name=test_id",
2022-06-02T00:53:12.6449626Z         	            	 (string) (len=28) "-sharding_column_type=UINT64",
2022-06-02T00:53:12.6450216Z         	            	 (string) (len=12) "-force=false",
2022-06-02T00:53:12.6450744Z         	            	 (string) (len=14) "test_keyspace3"
2022-06-02T00:53:12.6451039Z         	            	}
2022-06-02T00:53:12.6451319Z         	Test:       	TestCreateKs

[notfelineit]

@arvind-murty did you run make web_build? That might help compile!

[notfelineit]

Hey @arvind-murty, I went ahead and created a branch that removes those fields from vtctld2, you can rebase off #10459 to get those changes onto your branch!

[deepthi]

@ajm188 what is the right way to deprecate these options?

[ajm188]

pflag has native support for this!! (vaguely gestures at "won't it be great to use pflag everywhere in the future?"):

CreateKeyspace.Flags().StringVar(&createKeyspaceOptions.ShardingColumnName, "sharding-column-name", ...)
CreateKeyspace.Flags().MarkDeprecated("sharding-column-name", "Keyspace.ShardingColumnName is no longer used for resharding") // feel free to tweak the deprecation message, i didn't give this much thought
// same pattern for ShardingColumnType

[deepthi]

Thank you. I'm thinking we should let this PR delete these flags on main (to go into v15) and do a separate PR to deprecate them in v14 (on release-14.0)

[ajm188]

makes sense to me!

[deepthi]

@ajm188 what is the right way to deprecate these?

[ajm188]

For this, we should definitely update the help message for these flags to begin with "(DEPRECATED) ", eg:

	shardingColumnName := subFlags.String("sharding_column_name", "", "(DEPRECATED) Specifies the column to use for sharding operations")

then we can either:

1. no-assign the flag, and completely ignore any values the user sets, e.g.

	_ = subFlags.String("sharding_column_name", "", "Specifies the column to use for sharding operations")

OR

2. we can do what @systay did for planner-version / planner_version here

[arvind-murty]

Hey @arvind-murty, I went ahead and created a branch that removes those fields from vtctld2, you can rebase off #10459 to get those changes onto your branch!

Thank you. Since your PR was merged, I just did a merge from main.

[deepthi]

Changes look fine.
I'm in two minds about whether we need to add this to the summary doc. v2 resharding (not the same as vreplication v2) is old enough that probably no one cares.
We are announcing the deprecation of these fields for the CLI commands in v14.
@ajm188 @frouioui do you have any opinions on this?

[ajm188]

I'm in two minds about whether we need to add this to the summary doc. v2 resharding (not the same as vreplication v2) is old enough that probably no one cares.
We are announcing the deprecation of these fields for the CLI commands in v14.
@ajm188 @frouioui do you have any opinions on this?

It's been marked as "legacy" since at least v13, so I think it's fine to just delete. If we feel a little iffy about it, we can include a "minor deletions/deprecations" section or something like that towards the bottom of the doc.