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

Feature Request: include source tablet alias in FullStatus RPC response #17297

Open
timvaillancourt opened this issue Nov 28, 2024 · 5 comments
Assignees
Labels
Component: VTorc Vitess Orchestrator integration Component: VTTablet Type: Enhancement Logical improvement (somewhere between a bug and feature)

Comments

@timvaillancourt
Copy link
Contributor

timvaillancourt commented Nov 28, 2024

Feature Description

This issue requests that replicationdatapb.Status includes the tablet alias of a tablet's current primary replication source

Today it appears only the source_host and source_port of the PRIMARY are included in this response

The reason this is useful is VTOrc is having to use the source_host+source_port in sqlite3 queries to find the record of a replica's source, and these fields are unindexed. I considered just-indexing those 2 x fields (I think these fields used to be indexed), but using the tablet alias of the primary would use the Primary-Key and avoid another index, which I prefer. Also this information logically makes sense to be in this output (IMHO)

I propose this new field is topodata.TabletAlias source_alias = 26; or string source_alias = 26;. primary_alias would work too

cc @GuptaManan100 for feedback

Use Case(s)

VTOrc + any future use cases that benefit from an easy lookup of "what source alias is X tablet is replicating from?"

@timvaillancourt timvaillancourt added Type: Enhancement Logical improvement (somewhere between a bug and feature) Component: VTorc Vitess Orchestrator integration Component: VTTablet labels Nov 28, 2024
@timvaillancourt timvaillancourt self-assigned this Nov 28, 2024
@timvaillancourt timvaillancourt changed the title Feature Request: include primary tablet alias in FullStatus RPC response Feature Request: include source tablet alias in FullStatus RPC response Nov 28, 2024
@timvaillancourt
Copy link
Contributor Author

Updated to use term "source" only, not "primary", because those aren't always the same thing

@timvaillancourt
Copy link
Contributor Author

Updated to use term "source" only, not "primary", because those aren't always the same thing

After more thought, perhaps both source_alias and primary_alias should be added 🤔. This would tell us both the primary and if there is any replication chaining, incorrect primary configured, etc

@GuptaManan100
Copy link
Member

primary_alias is something that can be read from the topo server, so I don't think we need that. Source alias is something that I agree can be useful 👍

@timvaillancourt
Copy link
Contributor Author

timvaillancourt commented Dec 3, 2024

primary_alias is something that can be read from the topo server, so I don't think we need that. Source alias is something that I agree can be useful 👍

@GuptaManan100 just source_alias sounds good 👍

Any preference on storing this as topodata.TabletAlias vs string? I think I prefer the later

@GuptaManan100
Copy link
Member

GuptaManan100 commented Dec 4, 2024

Latter is good!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: VTorc Vitess Orchestrator integration Component: VTTablet Type: Enhancement Logical improvement (somewhere between a bug and feature)
Projects
None yet
Development

No branches or pull requests

2 participants