workload/tpcc: support Read Committed isolation#113834
workload/tpcc: support Read Committed isolation#113834craig[bot] merged 4 commits intocockroachdb:masterfrom
Conversation
|
This change is |
|
I tested out both of the new roachtest variants and both passed, running without error and passing the post-run consistency checks. This was not the case before the commit here that added Here are some basic performance results, comparing Serializable and Read Committed on TPC-C:
|
cdbce1a to
43eaef5
Compare
|
Another reason why SELECT FOR UPDATE seems to be slower with Read Committed is that its performs unnecessary lookup joins in certain cases. For example, this is what the root@localhost:26257/system/tpcc> EXPLAIN SELECT no_o_id
-> FROM new_order
-> WHERE no_w_id = 2 AND no_d_id = 3
-> ORDER BY no_o_id ASC
-> LIMIT 1
-> FOR UPDATE;
info
-------------------------------------------------------------------------------------
distribution: local
vectorized: true
• lookup join (semi)
│ estimated row count: 1
│ table: new_order@new_order_pkey
│ equality: (no_w_id, no_d_id, no_o_id) = (no_w_id,no_d_id,no_o_id)
│ equality cols are key
│ locking strength: for update
│ locking durability: guaranteed
│
└── • scan
estimated row count: 1 (<0.01% of the table; stats collected 7 minutes ago)
table: new_order@new_order_pkey
spans: [/2/3 - /2/3]
limit: 1@michae2 is working on eliminating this redundant lookup join. (do we have an issue for this?) |
michae2
left a comment
There was a problem hiding this comment.
It would be interesting to have a variant of pkg/sql/opt/xform/testdata/external/tpcc which shows plans under read committed isolation (they would be different for these two SFU statements).
@michae2 is working on eliminating this redundant lookup join. (do we have an issue for this?)
Issue coming, here is the draft PR: #114401
Reviewed 1 of 1 files at r1, 6 of 6 files at r2, 8 of 8 files at r3, 1 of 1 files at r4, all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @herkolategan and @srosenberg)
Addresses a TODO. This query has been supported by the optimizer since at least 526b4e5, which is when it was rewritten in the optimizer's test suite. Epic: None Release note: None
Informs cockroachdb#100176. This commit adds an `--isolation-level` flag to tpcc, which controls the isolation level to run the workload transactions under. If unset, the workload will run with the default isolation level of the database. Release note: None
Informs cockroachdb#100176. This commit adds SELECT FOR UPDATE locking in two places to ensure that the workload avoids anomalies when run under Read Committed isolation. The first of these is in the NewOrder transaction, when querying the "stock" table in preparation for updating quantities and order counts for the items in an order. There are no consistency checks which fail without this, but the locking is present in benchbase (https://github.com/cmu-db/benchbase/blob/546afa60dae4f8a6b00b84b77c77ff7684e494ad/src/main/java/com/oltpbenchmark/benchmarks/tpcc/procedures/NewOrder.java#L88) and makes sense to do. The second of these is in the Delivery transaction, when querying the "new_order" table to select an order to deliver. The order selected is processed by the transaction, including updating counters in the corresponding "customer" row, so it's important to have full isolation. Without this, consistency checks `3.3.2.10` and `3.3.2.12` (`workload check tpcc --expensive-checks`) do fail, presumably because a customer's row is updated twice for a single order. This use of SELECT FOR UPDATE in the Delivery transaction is an alternative to a patch like 36709df, which would probably be more efficient than the approach we have here, but would not exercise the database in an interesting way. We opt to use SELECT FOR UPDATE. Release note: None
Closes cockroachdb#100176. This commit adds the following two roachtest variants: ``` tpcc-nowait/isolation-level=read-committed/nodes=3/w=1 tpcc/headroom/isolation-level=read-committed/n4cpu16 ``` It also ensures that the `tpcc-nowait` tests runs the full set of expensive consistency checks at the end. The "nowait" variant run a more heavily contended version of tpcc, but with few warehouses, so the checks should still be fast. Release note: None
43eaef5 to
b9bdc43
Compare
|
TFTR! bors r+
Agreed. Wherever there are differences in how components handle isolation levels, it's interesting to split them out in tests.
Thanks! Does the query in #113834 (comment) qualify for the rules added in that PR? |
|
Build succeeded: |
Added this in another commit to the PR. It doesn't show the optimization (which occurs in execbuilder) but does show differences from Serializable.
Yes, both SFU queries qualify: |
|
blathers backport 23.2 |
Closes #100176.
This PR consists of a series of commits which together add support for Read Committed isolation to the TPC-C workload and then use it to add new roachtest variants.
See individual commits, including an interesting change to explicit row-level locking in TPC-C transactions to avoid concurrency anomalies.
Release note: None