No more EndPoints in topology.

[alainjobart]

WIP for now, not ready for review. Starting with the tests, then will do more.

---

This change is

[alainjobart]

@guoliang100 ready for review now.

@enisoc @michael-berlin FYI

[michael-berlin]

LGTM on the vtworker changes.

I left some minor comments.

Previously, alainjobart (Alain Jobart) wrote…

@guoliang100 ready for review now.

@enisoc @michael-berlin FYI

---

Review status: 0 of 96 files reviewed at latest revision, 3 unresolved discussions.

---

go/vt/discovery/healthcheck.go, line 80 [r1] (raw file):

// Alias returns the alias of the tablet.
// The return value can be used e.g. to generate the input for the topo API.
func (e *TabletStats) Alias() *topodatapb.TabletAlias {

If you like, you can remove this function now. (I only added it recently for my usage in vtworker.)

Getting the Alias is much more straight forward now.

---

go/vt/discovery/healthcheck.go, line 659 [r1] (raw file):

// StatusAsHTML returns an HTML version of the status.
func (epcs *TabletsCacheStatus) StatusAsHTML() template.HTML {

There are a few receiver names here and there which still have the old name.

I also saw "ep" and "epList" as variable names.

It would be best to rename them all eventually?

In this particular case, the receiver should be shortened as well to a 1 or 2 letter word.

---

go/vt/worker/topo_utils.go, line 28 [r1] (raw file):

  // Therefore, the default for this variable must be higher
  // than vttablet's -health_check_interval.
  waitForHealthyTabletsTimeout = flag.Duration("wait_for_healthy_rdonly_endpoints_timeout", 60*time.Second, "maximum time to wait if less than --min_healthy_rdonly_endpoints are available")

I'm fine with changing the flag as well to be consistent here.

I can do that in a separate internal CL later.

---

Comments from Reviewable

[alainjobart]

New commit coming to address these,thanks.

Previously, michael-berlin (Michael Berlin) wrote…

LGTM on the vtworker changes.

I left some minor comments.

---

Review status: 0 of 96 files reviewed at latest revision, 3 unresolved discussions.

---

go/vt/discovery/healthcheck.go, line 80 [r1] (raw file):

Previously, michael-berlin (Michael Berlin) wrote…

If you like, you can remove this function now. (I only added it recently for my usage in vtworker.)

Getting the Alias is much more straight forward now.

Done.

---

go/vt/discovery/healthcheck.go, line 659 [r1] (raw file):

Previously, michael-berlin (Michael Berlin) wrote…

There are a few receiver names here and there which still have the old name.

I also saw "ep" and "epList" as variable names.

It would be best to rename them all eventually?

In this particular case, the receiver should be shortened as well to a 1 or 2 letter word.

I renamed most of the ones I found in the next commit.

---

go/vt/worker/topo_utils.go, line 28 [r1] (raw file):

Previously, michael-berlin (Michael Berlin) wrote…

I'm fine with changing the flag as well to be consistent here.

I can do that in a separate internal CL later.

Yes I didn't want to change any externally visible flags as part of this. Thanks for doing this later. :)

---

Comments from Reviewable

[guoliang100]

You also need to change go/cmd/vtgate/status.go because of EndPointsCacheStatus change in healthcheck.go

Previously, alainjobart (Alain Jobart) wrote…

New commit coming to address these,thanks.

---

Reviewed 96 of 96 files at r1.
Review status: 84 of 97 files reviewed at latest revision, 3 unresolved discussions.

---

Comments from Reviewable

[alainjobart]

I think status.go uses TabletsCacheStatus, which contents hasn't changed (even though the name changed, but the name is unused in the html renderer).

In any case, we have integration tests that test the rendering, right?

Previously, guoliang100 (Liang) wrote…

You also need to change go/cmd/vtgate/status.go because of EndPointsCacheStatus change in healthcheck.go

---

Review status: 84 of 97 files reviewed at latest revision, 3 unresolved discussions.

---

Comments from Reviewable

[guoliang100]

Reviewed 13 of 13 files at r2.
Review status: all files reviewed at latest revision, 4 unresolved discussions.

---

go/cmd/vtgate/status.go, line 236 [r2] (raw file):

    <th>Shard</th>
    <th>TabletType</th>
    <th>EndPointsStats</th>

Please also rename this.

---

Comments from Reviewable

[guoliang100]

Please rename the line in status.go, and then LGTM

Previously, alainjobart (Alain Jobart) wrote…

I think status.go uses TabletsCacheStatus, which contents hasn't changed (even though the name changed, but the name is unused in the html renderer).

In any case, we have integration tests that test the rendering, right?

---

Review status: all files reviewed at latest revision, 4 unresolved discussions.

---

Comments from Reviewable