Delete discovery gateway #9500

systay · 2022-01-11T10:25:46Z

Description

This pull request removes the DiscoveryGateway struct and related files. The DiscoveryGateway has been our legacy gateway since the implementation of the new tabletgateway through #6155.

Related Issue(s)

Contributes to Task: delete deprecated discoverygateway and legacy healthcheck code #9218

Checklist

Should this PR be backported? NO
Tests were added or are not required
Documentation was added or is not required

harshit-gangal · 2022-01-11T11:29:51Z

go/vt/vtgate/executor_framework_test.go

 _ = hc.AddTestTablet(cell, "e0-", 1, "TestExecutor", "e0-", topodatapb.TabletType_PRIMARY, true, 1, nil)
+ // Below is needed so that SendAnyWherePlan doesn't fail
+ _ = hc.AddTestTablet(cell, "e0-", 1, "TestXBadVSchema", "-20", topodatapb.TabletType_PRIMARY, true, 1, nil)


the problem seems to be related to hostname, if you change the hostname from e0- to random the test starts working.
The bug may be a test thing when the hostname is the same.

ah good point. we should change e0- to -20 to match the shard name and make it unique.

-20 also conflicts. I guess we should make it 0.
I tried that, the next error is

--- FAIL: TestExecutorMaxPayloadSizeExceeded (0.00s) executor_test.go:1992: Error Trace: executor_test.go:1992 Error: Not equal: expected: <nil>(<nil>) actual : *vterrors.fundamental(Code: NOT_FOUND tablet aa-0000000010 not found ) Test: TestExecutorMaxPayloadSizeExceeded Messages: err should be nil

harshit-gangal · 2022-01-11T13:24:27Z

go/vt/vtgate/executor_dml_test.go

@@ -409,7 +409,7 @@ func TestDeleteEqual(t *testing.T) {
 }

 func TestUpdateScatter(t *testing.T) {


for failing test, the issue is due to a global session named primarySession getting used.
instead create a local session for those tests and use exec or executorExecSession instead of executorExec

What I did instead is to not initialize primarySession when it is declared, but create it in each of the test env functions - createExecutorEnv etc. That seems to fix all the failures in this particular test.

go/vt/vtgate/gateway.go

systay · 2022-01-18T15:57:34Z

let's re-open when we have time to spend on it

Signed-off-by: deepthi <deepthi@planetscale.com>

… package Signed-off-by: deepthi <deepthi@planetscale.com>

…teway Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>

…t names Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>

Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>

frouioui · 2022-03-09T09:55:44Z

I will open a follow-up pull request rebased on top of this one to remove the Gateway interface introduced by #5992.

go/vt/discovery/fake_healthcheck.go

…TabletUID Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>

systay

I opened this PR, so I can't accept it, but it LGTM ⭐

Removing old code is the best ❤️

…teway Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>

frouioui · 2022-03-09T15:54:25Z

@deepthi in healthcheck.go you left a ToDo to deprecate some variables. I am not sure which ones you were talking about. Can you guide me?

Link to the corresponding ToDo:
https://cs.github.com/vitessio/vitess/blob/605beab9015ce69840f014cabd98a8ecae4d9b31/go/vt/discovery/healthcheck.go#L70

deepthi · 2022-03-09T16:37:44Z

@deepthi in healthcheck.go you left a ToDo to deprecate some variables. I am not sure which ones you were talking about. Can you guide me?

Link to the corresponding ToDo: https://cs.github.com/vitessio/vitess/blob/605beab9015ce69840f014cabd98a8ecae4d9b31/go/vt/discovery/healthcheck.go#L70

Not deprecate, just change from exported -> not. All 6 flags below that comment should be reviewed to see if they are still being used outside discovery package. If not, we can change them back.

Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>

harshit-gangal reviewed Jan 11, 2022

View reviewed changes

deepthi added Component: Query Serving release notes Type: Internal Cleanup labels Jan 11, 2022

deepthi reviewed Jan 13, 2022

View reviewed changes

go/vt/vtgate/gateway.go Show resolved Hide resolved

systay closed this Jan 18, 2022

systay reopened this Jan 18, 2022

deepthi added 3 commits January 19, 2022 11:16

delete discoverygateway

1823401

Signed-off-by: deepthi <deepthi@planetscale.com>

fix some executor tests

6d885cd

Signed-off-by: deepthi <deepthi@planetscale.com>

make retryCount unexported. It is no longer needed outside the vtgate…

266af64

… package Signed-off-by: deepthi <deepthi@planetscale.com>

deepthi force-pushed the delete-discovery-gateway branch from 3f5662e to 266af64 Compare January 19, 2022 19:18

frouioui self-assigned this Mar 3, 2022

Merge remote-tracking branch 'upstream/main' into delete-discovery-ga…

73d4af0

…teway Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>

frouioui mentioned this pull request Mar 7, 2022

Task: delete deprecated discoverygateway and legacy healthcheck code #9218

Closed

6 tasks

frouioui added 3 commits March 7, 2022 09:52

fix: use primarySession after its initialization and fix comments/tes…

6bb1866

…t names Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>

fix: use proper tablet alias and hostname in executor test

c157eed

Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>

fix: updated expected output in vtgate tx and executor tests

6c33bb0

Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>

frouioui force-pushed the delete-discovery-gateway branch from 777a4ba to faf0cd3 Compare March 8, 2022 14:12

refactore: remove gateway discovery test

2475fb3

Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>

frouioui force-pushed the delete-discovery-gateway branch from 3c976f2 to 2475fb3 Compare March 8, 2022 18:28

frouioui changed the title ~~delete discoverygateway~~ Delete discovery gateway Mar 9, 2022

fix: reset primarySession before TestSelectScatterPartial

5222127

Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>

frouioui marked this pull request as ready for review March 9, 2022 09:54

frouioui self-requested a review as a code owner March 9, 2022 09:54

frouioui requested review from harshit-gangal and deepthi March 9, 2022 09:56

systay commented Mar 9, 2022

View reviewed changes

go/vt/discovery/fake_healthcheck.go Outdated Show resolved Hide resolved

refactor: use int instead of atomic int for FakeHealthCheck's current…

4b16187

…TabletUID Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>

systay commented Mar 9, 2022

View reviewed changes

Merge remote-tracking branch 'upstream/main' into delete-discovery-ga…

1e59d83

…teway Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>

frouioui mentioned this pull request Mar 9, 2022

Remove the Gateway interface #9852

Merged

3 tasks

refactor: unexported variables in healthcheck.go

87fe76d

Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>

systay requested a review from ajm188 as a code owner March 10, 2022 07:08

frouioui approved these changes Mar 10, 2022

View reviewed changes

frouioui merged commit 4162b2f into vitessio:main Mar 10, 2022

frouioui deleted the delete-discovery-gateway branch March 10, 2022 14:44

frouioui mentioned this pull request Jun 22, 2022

Documented the legacy healthcheck and tabletgateway and added summary to 14's summary #10567

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Delete discovery gateway #9500

Delete discovery gateway #9500

systay commented Jan 11, 2022 •

edited by frouioui

Loading

harshit-gangal Jan 11, 2022

deepthi Jan 11, 2022 •

edited

Loading

deepthi Jan 13, 2022

harshit-gangal Jan 11, 2022

deepthi Jan 19, 2022

systay commented Jan 18, 2022

frouioui commented Mar 9, 2022

systay left a comment

frouioui commented Mar 9, 2022

deepthi commented Mar 9, 2022

		@@ -409,7 +409,7 @@ func TestDeleteEqual(t *testing.T) {
		}

		func TestUpdateScatter(t *testing.T) {

Delete discovery gateway #9500

Delete discovery gateway #9500

Conversation

systay commented Jan 11, 2022 • edited by frouioui Loading

Description

Related Issue(s)

Checklist

harshit-gangal Jan 11, 2022

Choose a reason for hiding this comment

deepthi Jan 11, 2022 • edited Loading

Choose a reason for hiding this comment

deepthi Jan 13, 2022

Choose a reason for hiding this comment

harshit-gangal Jan 11, 2022

Choose a reason for hiding this comment

deepthi Jan 19, 2022

Choose a reason for hiding this comment

systay commented Jan 18, 2022

frouioui commented Mar 9, 2022

systay left a comment

Choose a reason for hiding this comment

frouioui commented Mar 9, 2022

deepthi commented Mar 9, 2022

systay commented Jan 11, 2022 •

edited by frouioui

Loading

deepthi Jan 11, 2022 •

edited

Loading