search

cockroachdb / cockroach

CockroachDB — the cloud native, distributed SQL database designed for high availability, effortless scale, and control over data placement.
https://www.cockroachlabs.com
Other
30.07k stars 3.8k forks source link

distsqlplan: fakeSpanResolver forces DistSQL planning to need a txn #29590

Open andreimatei opened 6 years ago

andreimatei commented 6 years ago

The fakeSpanResolver currently uses the planning txn (a concept that I don't think we should pretend is well defined) to do scans for figuring out possible (fake) split points; it uses said txn so that its scans don't interfere with the actual execution of the query (I think). While magic and clever, I don't like this very much :)

Besides the fact that conceptually we should be able to do planning without being in the context of a txn (we've made various strides in that direction in the classic planner), the mechanism is very invasive because the txn needs to be passed to the resolver very indirectly - through DistSQLPlanner.NewPlanningCtx(). https://github.com/cockroachdb/cockroach/blob/3e62ce0bd5435e92fcf8e451a846a530136af2c1/pkg/sql/distsql_physical_planner.go#L3270 So the PlanningCtx and the spanResolver interface needs to take this txn just because this FakeResolver guy. I starting pulling on this because I'm trying to write an integration test between DistSQL and KV. There I want to create a plan outside of a txn and then call DistSQLPlanner.Run() in a txn under my control. Such tests are generally quite hard to write because creating a DistSQL plan requires use of heavy machinery, and NewPlanningCtx() seemingly asking for a txn added to my struggle.

Could the FakePlanner do non-transactional INCONSISTENT or READ_UNCOMMITTED reads instead? https://github.com/cockroachdb/cockroach/blob/44a5f0685eed55ef76a437e1cc196ebe9e33c757/pkg/roachpb/api.proto#L41 Neither of these populate the timestamp cache, so they shouldn't interfere with the query execution? I can try the change if I get a nod.

cc @asubiotto

Jira issue: CRDB-4859

RaduBerinde commented 6 years ago

I don't see a problem with it doing INCONSISTENT or READ_UNCOMITTED reads.

github-actions[bot] commented 3 years ago

We have marked this issue as stale because it has been inactive for 18 months. If this issue is still relevant, removing the stale label or adding a comment will keep it active. Otherwise, we'll close it in 5 days to keep the issue queue tidy. Thank you for your contribution to CockroachDB!