Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate and ignore gateway_implementation flag to vtgate. #9482

Merged
merged 4 commits into from
Jan 18, 2022

Conversation

deepthi
Copy link
Member

@deepthi deepthi commented Jan 7, 2022

Description

VTGate's gateway_implementation flag is now deprecated, and any value provided is ignored. tabletgateway is now the only supported implementation.

Related Issue(s)

#9218

Checklist

  • Should this PR be backported? NO
  • Tests were added or are not required
  • Documentation was added or is not required - release note has been added. Website PR will be created

Deployment Notes

Scripts used to run vtgate will need to be updated to drop this flag. It will be removed in a future release (14.0)

Signed-off-by: deepthi <deepthi@planetscale.com>
Signed-off-by: deepthi <deepthi@planetscale.com>
@deepthi deepthi added Component: Query Serving release notes (needs details) This PR needs to be listed in the release notes in a dedicated section (deprecation notice, etc...) Type: Internal Cleanup labels Jan 7, 2022
// GatewayImplementation allows you to choose which gateway to use for vtgate routing. Defaults to tabletgateway, other option is discoverygateway
GatewayImplementation = flag.String("gateway_implementation", "tabletgateway", "Allowed values: discoverygateway (deprecated), tabletgateway (default)")
_ = flag.String("gateway_implementation", "", "Deprecated. Only tabletgateway is now supported, discoverygateway is no longer available")
GatewayImplementation = new(string)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just make this GatewayImplementation = &tabletGatewayImplementation instead of assigning to it from an init function.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That doesn't work because tabletGatewayImplementation is declared as a const.

go/vt/vtgate/gateway.go:39:26: cannot take the address of tabletGatewayImplementation

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could change the type of GatewayImplementation from string* to string and change all references, but that will touch lots of files. I'm looking for the minimum necessary change.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@deepthi I see, in that case the minimal change would be making a temp assignment right next to it.

var defaultGatewayImplementation = tabletGatewayImplementation
var GatewayImplementation = &defaultGatewayImplementation

That's the minimal amount of changes and doesn't allocate on heap or use init,

Signed-off-by: deepthi <deepthi@planetscale.com>
Signed-off-by: deepthi <deepthi@planetscale.com>
@deepthi deepthi merged commit 20df931 into vitessio:main Jan 18, 2022
@deepthi deepthi deleted the ds-ignore-gateway-flag branch January 18, 2022 02:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Query Serving release notes (needs details) This PR needs to be listed in the release notes in a dedicated section (deprecation notice, etc...) Type: Internal Cleanup
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants