vtexplain2 #6705
Merged
vtexplain2 #6705
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Andrew Mason <amason@slack-corp.com>
…hards to construct a topo Signed-off-by: Andrew Mason <amason@slack-corp.com>
Signed-off-by: Andrew Mason <amason@slack-corp.com>
Signed-off-by: Andrew Mason <amason@slack-corp.com>
sougou
reviewed
Sep 11, 2020
There was a problem hiding this comment.
Cool. Could you write some tests that demonstrate the new functionality? Also, we'll need to update the documentation https://vitess.io/docs/reference/programs/vtexplain/
Signed-off-by: Andrew Mason <amason@slack-corp.com>
Signed-off-by: Andrew Mason <amason@slack-corp.com>
…s runs Signed-off-by: Andrew Mason <amason@slack-corp.com>
Signed-off-by: Andrew Mason <amason@slack-corp.com>
|
Tests added, and preliminary documentation change is here! |
sougou
approved these changes
Sep 13, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR enables vtexplain to use non-evenly sharded keyspaces, by consuming a new piece of user input, namely a json blob in the form of
{ "ks1": $(vtctl FindAllShardsInKeyspace ks1), "ks2": $(vtctl FindAllShardsInKeyspace ks2) }.A few notable changes were required for this to work:
ks-shard-mapandks-shard-map-fileto get the new json blob, in the same style as the sql/sqlfile and vschema/vschema-file args. For this, I madegetFileParamtake an additional arg to dictate whether a failure to pass exactly one of the two arguments should result in an error. This makes it so that ks-shard-map/ks-shard-map-file are optional. Failure to pass exactly one of them will cause vtexplain to fall back to the oldNumShardsbehavior.topodatapb.ShardReferencepointers, (so, keyspace -> shard_name -> shard_ref). ExplainTopo then uses this to construct atopodatapb.SrvKeyspaceinGetSrvKeyspace()rather than doing it based on justNumShards+ an assumption of evenly-sharded keyranges. This is the piece that gives us the flexibility to allow arbitrarily-sharded keyspaces.buildTopologyconstructs a set of ShardReferences and TabletConns based on the keyspaceShardMap. If the user didn't provide a keyspaceShardMap, go back to the NumShards+even sharding behavior. The significant change here is that this function must update theExplainTopo.KeyspaceShardsmap, or it will return incorrect data whenGetSrvKeyspace()is called on it.I've manually tested that both the new and old behaviors work on sharded keyspaces so far, but I wanted to get input at this point.
I'll note that this code can probably simplified if we decide we don't want to support the
NumShardscase, and instead require users to give a json map of keyspace -> shard name -> shard ref. But, I also understand if we need to keep supporting that, if even for a little while.