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

Removing SrvShard #1706

Merged
merged 5 commits into from May 19, 2016
Merged

Removing SrvShard #1706

merged 5 commits into from May 19, 2016

Conversation

alainjobart
Copy link
Contributor

@alainjobart alainjobart commented May 18, 2016

@enisoc so much simpler!

(also fixed Topology Server doc).


This change is Reviewable

To account for my last round of changes.
Re-generating the doc too.
Lots of comments, docs, ...
It disappeared in my previous changes. Using it to fix reparent test.
@enisoc
Copy link
Member

enisoc commented May 18, 2016

:lgtm:

Previously, alainjobart (Alain Jobart) wrote…

Removing SrvShard

@enisoc so much simpler!

(also fixed Topology Server doc).


Reviewed 70 of 70 files at r1.
Review status: all files reviewed at latest revision, 2 unresolved discussions.


doc/TopologyService.md, line 43 [r1] (raw file):

* There is one local instance per cell, that contains cell-specific information,
  and also rolled-up data from the global + local cell to make it easier for
  clients to find the data. The Vitess serving path should not use the global

In my update of the Concepts doc, I'm careful to not say the "serving path" requires access to local topology. In my experience, that gives some people the mistaken belief that as part of serving each query, we do a read from topology to fetch routing info.


doc/VitessApi.md, line 819 [r1] (raw file):

### topodata.ShardReference

Serving graph information ShardReference is used as a pointer from a SrvKeyspace to a Shard

The fragment "Serving graph information" doesn't fit grammatically here (and below). Was this an accidental find/replace?


Comments from Reviewable

Approved with PullApprove

@alainjobart
Copy link
Contributor Author

I'll fix these two comments, merge end import tomorrow morning...

@alainjobart
Copy link
Contributor Author

Review status: 67 of 70 files reviewed at latest revision, 2 unresolved discussions.


doc/TopologyService.md, line 43 [r1] (raw file):

Previously, enisoc (Anthony Yeh) wrote…

In my update of the Concepts doc, I'm careful to not say the "serving path" requires access to local topology. In my experience, that gives some people the mistaken belief that as part of serving each query, we do a read from topology to fetch routing info.

Done.

doc/VitessApi.md, line 819 [r1] (raw file):

Previously, enisoc (Anthony Yeh) wrote…

The fragment "Serving graph information" doesn't fit grammatically here (and below). Was this an accidental find/replace?

This is generated from the proto file. It had a section name there. Removed it.

Comments from Reviewable

@alainjobart alainjobart merged commit cc6e894 into vitessio:master May 19, 2016
@alainjobart alainjobart deleted the srvshard branch May 19, 2016 14:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants