streamingccl: create new tenant automatically for replication stream#83646
Conversation
|
This change is |
|
I purposefully did not include the necessary The code to validate that the tenant is inactive is commented out until #83650 is resolved. Right now the tenant is marked inactive but its still possible to open a SQL connection to it and query its tables. |
1632472 to
7bdf88d
Compare
| if _, err := sql.GetTenantRecord(ctx, p.ExecCfg(), nil, newTenantID.ToUint64()); err == nil { | ||
| return errors.Newf("tenant with id %s already exists", newTenantID) | ||
| } | ||
| if err := p.ExtendedEvalContext().Tenant.CreateInactiveTenant(ctx, newTenantID.ToUint64()); err != nil { |
There was a problem hiding this comment.
I elected to use the full CreateTenant code rather than just creating a record like Restore does because it seemed like just creating a record did eventually initialize the rest of the tenant such that I run queries against it, but prior to that point a sql connection would error and ActivateTenant didn't look like it would necessarily force the initialization so I was concerned about a stream that was created and then completed very quickly, activating the tenant prior to it being actually ready. I didn't end up looking into the mechanism for this eventual initialization.
There was a problem hiding this comment.
Coming back to this. I think in the long run we want to do something a little different.
The problem with initialising the tenant is that it means that the keyspace we are writing into isn't completely empty. Namely it will have entries schema entries for the default database, the cluster version setting, the id sequence populated. If this destination tenant is created after the source tenant, then the timestamp on those KVs will likely be above whatever gets replicated to us.
because it seemed like just creating a record did eventually initialize the rest of the tenant such that I run queries against it
Did you see this outside of the streaming replication tests? If you only saw it in the streaming replication tests, my guess for what you were seeing is that eventually enough of the source cluster's tenant data is replicated to it for it to be viable.
There was a problem hiding this comment.
I think your theory on "the tenant works when enough has been streamed in" is true, since attempting to connect right after stream creation doesn't work, connecting after waiting 2 seconds but without cutover still works, and connecting after cutover always works.
7bdf88d to
bf84b8a
Compare
bf84b8a to
ac4af26
Compare
stevendanna
left a comment
There was a problem hiding this comment.
Thanks for jumping into this. This looks reasonable to me. I've left some comments but nothing blocking.
5e692c0 to
267427e
Compare
0617def to
53c05df
Compare
Originally tenants had to be created manually prior to replication stream creation. This change instead creates a new tenant upon calling the RESTORE INTO command using the tenant ID that was provided. The tenant is considered inactive until the cutover point, at which the tenant is activated. Release note: None
53c05df to
e9fe630
Compare
|
bors r+ |
|
Build succeeded: |
Resolves #76952
Originally tenants had to be created manually prior to replication
stream creation. This change instead creates a new tenant upon calling
the RESTORE INTO command using the tenant ID that was provided. The
tenant is considered inactive until the cutover point, at which the
tenant is activated.
Release note: None