changefeedccl: misc fixes for tenant compatibility #57805
Conversation
dt
requested review from
ajwerner,
a team and
pbardea
and removed request for
a team
|
This change is |
| dsp := execCtx.DistSQLPlanner() | ||
| evalCtx := execCtx.ExtendedEvalContext() | ||
| planCtx := dsp.NewPlanningCtx(ctx, evalCtx, nil /* planner */, noTxn, true /* distribute */) | ||
| planCtx := dsp.NewPlanningCtx(ctx, evalCtx, nil /* planner */, noTxn, execCfg.Codec.ForSystemTenant() /* distribute */) |
There was a problem hiding this comment.
This is temporary, right? Eventually tenants will be distributed?
There was a problem hiding this comment.
Reviewed 4 of 4 files at r1, 2 of 2 files at r2, 1 of 1 files at r3.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @dt and @HonoreDB)
pkg/ccl/changefeedccl/changefeed_dist.go, line 113 at r3 (raw file):
Previously, HonoreDB (Aaron Zinger) wrote…
This is temporary, right? Eventually tenants will be distributed?
It's not in any immediate roadmap. The KV layer is distributed but for now the SQL execution is all local.
pkg/ccl/changefeedccl/kvfeed/scanner.go, line 154 at r3 (raw file):
ctx context.Context, ds *kvcoord.DistSender, targetSpans []roachpb.Span, ) ([]roachpb.Span, error) { ranges, err := allRangeSpans(ctx, ds, targetSpans)
nit: push the unwrapped down to here so that in the backport it's easier to just stick an if around getting the ranges using the new logic vs old logic.
Release note: none.
Release note: none.
Release note: none.
There was a problem hiding this comment.
Reviewable status:
pkg/ccl/changefeedccl/kvfeed/scanner.go, line 154 at r3 (raw file):
Previously, ajwerner wrote…
ctx context.Context, ds *kvcoord.DistSender, targetSpans []roachpb.Span, ) ([]roachpb.Span, error) { ranges, err := allRangeSpans(ctx, ds, targetSpans)nit: push the unwrapped down to here so that in the backport it's easier to just stick an
ifaround getting the ranges using the new logic vs old logic.
I think I'm fine backporting the whole change (or none of it), rather than having the backport do an extra if/else here.
There was a problem hiding this comment.
bors r+
Reviewable status:
|
This PR was included in a batch that was canceled, it will be automatically retried |
|
Build failed: |
|
bors r+ |
|
Build succeeded: |
see commits -- mostly just avoiding using NodeID, plus don't read meta2 directly.