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

VReplication: trafficswitcher's changeShardsAccess always operates on the source keyspace #8019

Closed
ajm188 opened this issue May 3, 2021 · 0 comments · Fixed by #8020
Closed

Comments

@ajm188
Copy link
Contributor

ajm188 commented May 3, 2021

In changeShardsAccess, we always operate on the sourceKeyspace of the workflow here, ignoring the keyspace parameter:

func (ts *trafficSwitcher) changeShardsAccess(ctx context.Context, keyspace string, shards []*topo.ShardInfo, access accessType) error {
	if err := ts.wr.ts.UpdateDisableQueryService(ctx, ts.sourceKeyspace, shards, topodatapb.TabletType_MASTER, nil, access == disallowWrites /* disable */); err != nil {
		return err
	}
	return ts.wr.refreshMasters(ctx, shards)
}

However in allowTargetWrites, we pass targetKeyspace as the keyspace parameter here:

func (ts *trafficSwitcher) allowTargetWrites(ctx context.Context) error {
	if ts.migrationType == binlogdatapb.MigrationType_TABLES {
		return ts.allowTableTargetWrites(ctx)
	}
	return ts.changeShardsAccess(ctx, ts.targetKeyspace, ts.targetShards(), allowWrites)
}

Now, technically, this is currently fine, because we don't do any reshard workflows across keyspaces (it doesn't make sense), but that can't really be expressed in the code, and if we were to ever change that design constraint, we could get surprised by this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant