logical: use isql.Session in the crud writer #156491
Conversation
|
This change is |
jeffswenson
force-pushed
the
jeffswenson-ldr-use-isession
branch
from
3228cb3 to
d962bc3
Compare
| show-select table=products | ||
| ---- | ||
| SELECT key_list.index, replication_target.crdb_internal_origin_timestamp, replication_target.crdb_internal_mvcc_timestamp, replication_target.id, replication_target.name, replication_target.unit_price, replication_target.quantity, replication_target.total_value, replication_target.last_updated FROM ROWS FROM (unnest($1::INT8[], $2::DECIMAL(10,2)[])) WITH ORDINALITY AS key_list (key1, key2, index) INNER LOOKUP JOIN [108 AS replication_target] ON (replication_target.id = key_list.key1) AND (replication_target.total_value = key_list.key2) | ||
| SELECT key_list.index, replication_target.crdb_internal_origin_timestamp, replication_target.crdb_internal_mvcc_timestamp, replication_target.id, replication_target.name, replication_target.unit_price, replication_target.quantity, replication_target.total_value, replication_target.last_updated FROM ROWS FROM (unnest($1::INT8[], $2::DECIMAL[])) WITH ORDINALITY AS key_list (key1, key2, index) INNER LOOKUP JOIN [108 AS replication_target] ON (replication_target.id = key_list.key1) AND (replication_target.total_value = key_list.key2) |
There was a problem hiding this comment.
NOTE: the change in this query is a side effect of switching to consistently use the canonical type within LDR. The canonical type of a decimal with fixed with is a decimal with variable width.
Nice. Out of curiosity, does that make it faster than the KV writer now? |
It's faster than the KV writer in cases where the KV writer performs 2-PC and ~20% less efficient in cases where the KV writer hits the 1-PC fast path. |
Awesome. Huge win here. |
msbutler
left a comment
msbutler
left a comment
There was a problem hiding this comment.
Amazing! left a couple nits
|
|
||
| sd = sd.Clone() | ||
| sd.PlanCacheMode = sessiondatapb.PlanCacheModeForceGeneric | ||
| sd.VectorizeMode = sessiondatapb.VectorizeOff |
There was a problem hiding this comment.
just curious: why no vectorized execution or buffered writes?
There was a problem hiding this comment.
Buffered writes doesn't support batches with any WriteOptions set. That should be relatively straightforward if needed. However, I kinda wonder if buffered writes really helps y'all that much.
| ctx context.Context, originTimestamp hlc.Timestamp, | ||
| ) error { | ||
| return s.session.ModifySession(ctx, func(m sessionmutator.SessionDataMutator) { | ||
| m.Data.OriginIDForLogicalDataReplication = 1 |
There was a problem hiding this comment.
nit: no need to set originID everytime, yeah?
There was a problem hiding this comment.
Good point. I moved this to the static session initialization.
jeffswenson
force-pushed
the
jeffswenson-ldr-use-isession
branch
from
d962bc3 to
dd779c0
Compare
This reworks the LDR crud writer to use the isql.Session, prepared statements, and generic query plans. When replicating TPC-C, this PR is more than 2x more efficient than the version of the crud writer that was using the internal executor. Release note: none Fixes: cockroachdb#148310
jeffswenson
force-pushed
the
jeffswenson-ldr-use-isession
branch
from
dd779c0 to
436c452
Compare
|
Thanks for the reviews! bors r+ |
|
Build succeeded: |
4 participants
This reworks the LDR crud writer to use the isql.Session, prepared statements, and generic query plans. When replicating TPC-C, this PR is more than 2x more efficient than the version of the crud writer that was using the internal executor.
Release note: none
Fixes: #148310