v3: write_only support for unique lookup vindexes#3658
v3: write_only support for unique lookup vindexes#3658alainjobart merged 3 commits intovitessio:masterfrom sougou:v3
Conversation
There are use cases where a unique vindex may not know the target keyspace id. If so, we should allow it to return a keyrange, which will allow the vtgate engine to scatter the query. The unique vindex property is still useful for the optimizer because it can still use the unique property to deduce within-shard joins. This feature may not be 100% relational, but it satisfies certain use cases that would otherwise become very inefficient.
Now that unique lookup vindexes can return a scatter, we can add write_only support for them.
|
@alainjobart I had to resolve a merge conflict with executor.go. Can you verify that I interpreted your change correctly? |
|
This does not contain "autocommit" yet? Or has that been merged separately? |
|
autocommit is already merged #3600. |
rafael
left a comment
rafael
left a comment
There was a problem hiding this comment.
Given @alainjobart confirmation about the executor, change LGTM. The part that makes a bit nervous is the same concern I had in #3600. I feel that when we come back to this code and try to follow up what's going on between range or ID, it will be hard to reason about it. I think more documentation in the code will be helpful. However, I can't think of way of changing this without a major refactor. 😅
| if ksid == nil { | ||
| continue | ||
| switch { | ||
| case ksid.Range != nil: |
There was a problem hiding this comment.
I think we should add a comment here explaining in which cases it will be a range vs an ID (i.e when using this feature). Otherwise, in this context it's going to be hard to understand why it could be one way or the other.
| executor, _, _, _ := createExecutorEnv() | ||
|
|
||
| // If a unique vindex returns a keyrange, we fail the insert | ||
| _, err := executorExec(executor, "insert into keyrange_table(krcol_unique, krcol) values(1, 1)", nil) |
There was a problem hiding this comment.
I had a question around this that @sougou responded in Slack. For future reference, adding here. It was not clear to me what would happen if you have an unique LookupIndex that is in write mode and then it enters in this code path.
In that case, it would mean that lookup was the primary vindex and it should fail.
|
@alainjobart this can be merged. |
|
LGTM |
5 participants
The first commit changes the vindex API to allow for unique vindexes to return a keyrange instead of a specific keyspace id. This is a way for a unique vindex to tell the engine that it doesn't know the keyspace id, and ask it to scatter the query.
The second commit uses the above feature to support write_only mode for unique vindexes: during this mode the vindex will be written to, but not read from, allowing you to backfill missing entries while new ones continue to be populated. Once the back-fill is complete, write_only can be turned off.