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

roachtest: support ycsb under read committed #107112

Closed
nvanbenschoten opened this issue Jul 18, 2023 · 0 comments · Fixed by #107517
Closed

roachtest: support ycsb under read committed #107112

nvanbenschoten opened this issue Jul 18, 2023 · 0 comments · Fixed by #107517
Assignees
Labels
A-read-committed Related to the introduction of Read Committed A-testing Testing tools and infrastructure C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)

Comments

@nvanbenschoten
Copy link
Member

nvanbenschoten commented Jul 18, 2023

As part of the Read Committed implementation, nightly testing of YCSB will be adapted to run at the Read Committed isolation level. When doing so, transaction retry loops will be removed from the workload to validate that retry errors are rare or non-existent under weak isolation.

YCSB provides a useful sandbox to test weaker isolation levels because it contains a diverse collection of simple operations run with high contention.

Jira issue: CRDB-29897

@nvanbenschoten nvanbenschoten added C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) A-testing Testing tools and infrastructure A-read-committed Related to the introduction of Read Committed labels Jul 18, 2023
@nvanbenschoten nvanbenschoten self-assigned this Jul 25, 2023
nvanbenschoten added a commit to nvanbenschoten/cockroach that referenced this issue Jul 25, 2023
Informs cockroachdb#107112.

This commit adds an `--isolation-level` flag to ycsb, 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
nvanbenschoten added a commit to nvanbenschoten/cockroach that referenced this issue Jul 25, 2023
Closes cockroachdb#107112.

This commit adds the following six roachtest variants:
```
ycsb/A/nodes=3/cpu=32/isolation-level=read-committed
ycsb/B/nodes=3/cpu=32/isolation-level=read-committed
ycsb/C/nodes=3/cpu=32/isolation-level=read-committed
ycsb/D/nodes=3/cpu=32/isolation-level=read-committed
ycsb/E/nodes=3/cpu=32/isolation-level=read-committed
ycsb/F/nodes=3/cpu=32/isolation-level=read-committed
```

Release note: None
craig bot pushed a commit that referenced this issue Jul 26, 2023
105857: sql: add implicit SELECT FOR SHARE locking to FK checks r=DrewKimball,nvanbenschoten,rytaft,mgartner a=michae2

**explain: add transaction information to EXPLAIN ANALYZE**

Add transaction isolation, priority, and quality-of-service to the
output of `EXPLAIN ANALYZE`.

Release note (sql change): `EXPLAIN ANALYZE` output now includes:
- the isolation level of the statement's transaction
- the priority of the statement's transaction
- the quality of service level of the statement's transaction

---

**opt: do not use LockDurabilityGuaranteed under serializable isolation**

This is a follow-up from #103734.

We do not want to use guaranteed-durable (a.k.a. replicated) locking
under serializable isolation, because it prevents pipelining and other
optimizations, and is unnecessary for correctness. This commit amends
8cbc6d1 to only set durability for
`SELECT FOR UPDATE` locking under weaker isolation levels.

This means that query plans will be slightly different under different
isolation levels, and so we must add isolation level to the optimizer
memo staleness calculation.

Furthermore, this commit changes the error message added by
e633d5e to be about guaranteed-durable
locking rather than `SELECT FOR UPDATE`, because in a later commit this
specific error will also be triggered by foreign key checks under weaker
isolation levels.

Informs: #100144, #100156, #100193, #100194

Release note: None

---

**opt: show locking durability in EXPLAIN (OPT) output**

Because the "guaranteed-durable locking not yet implemented" error
condition is checked in execbuilder, it prevents not only execution but
also `EXPLAIN` of queries using guaranteed-durable locking. Thankfully
`EXPLAIN (OPT)` bypasses execbuilder, and hence still works, so use this
for now to verify that we are enabling durable locking for `SELECT FOR
UPDATE` under read committed isolation.

(Note that we have not yet fixed the `SELECT FOR UPDATE` plans to use
more precise locking, that will come in a later PR.)

Informs: #100194

Release note: None

---

**sql: add implicit SELECT FOR SHARE locking to FK parent checks**

Add SELECT FOR SHARE locking to FK parent checks. Under serializable
isolation, this locking is only used when
`enable_implicit_fk_locking_for_serializable` is set. Under weaker
isolation levels (snapshot and read committed) this locking is always
used.

We only need to lock during the insertion-side FK checks, which verify
the existence of a parent row. Deletion-side FK checks verify the
non-existence of a child row, and these do not need to lock. Instead, to
prevent concurrent inserts or updates to the child that would violate
the FK constraint, we rely on the intent(s) created by the deletion
conflicting with the FK locking of those concurrent inserts or updates.

Fixes: #80683
Informs: #100156

Epic: CRDB-25322

Release note (sql change): Add a new session variable,
`enable_implicit_fk_locking_for_serializable`, which controls locking
during foreign key checks under serializable isolation. With this set to
true, foreign key checks of the referenced (parent) table, such as those
performed during an INSERT or UPDATE of the referencing (child) table,
will lock the referenced row using SELECT FOR SHARE locking. (This is
somewhat analogous to the existing `enable_implicit_select_for_update`
variable but applies to the foreign key checks of a mutation statement
instead of the initial row fetch.)

Under weaker isolation levels such as read committed, SELECT FOR SHARE
locking will always be used to ensure the database maintains the foreign
key constraint, regardless of the current setting of
`enable_implicit_fk_locking_for_serializable`.

107212: ui-e2e-tests: steps to enable cypress tests r=maryliag a=rickystewart

This doesn't get the job fully working yet, but it's an improvement.

Epic: none
Part of #106584
Release note: None

107517: roachtest: add read committed variants of ycsb r=michae2 a=nvanbenschoten

Closes #107112.

This PR adds the following six roachtest variants:
```
ycsb/A/nodes=3/cpu=32/isolation-level=read-committed
ycsb/B/nodes=3/cpu=32/isolation-level=read-committed
ycsb/C/nodes=3/cpu=32/isolation-level=read-committed
ycsb/D/nodes=3/cpu=32/isolation-level=read-committed
ycsb/E/nodes=3/cpu=32/isolation-level=read-committed
ycsb/F/nodes=3/cpu=32/isolation-level=read-committed
```

It does so after adding an `--isolation-level` flag to ycsb, 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

107636: schemachanger: deflake TestConcurrentDeclarativeSchemaChanges r=postamar a=postamar

This commit deflakes this test by checking that the second schema change actually does block because of the first one, rather than checking that it has blocked. The bug was that the latter wasn't always guaranteed to happen because we didn't force the schema changes to run in parallel.

Fixes #106732.

Release note: None

Co-authored-by: Michael Erickson <[email protected]>
Co-authored-by: Ricky Stewart <[email protected]>
Co-authored-by: Nathan VanBenschoten <[email protected]>
Co-authored-by: Marius Posta <[email protected]>
@craig craig bot closed this as completed in 2a335a2 Jul 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-read-committed Related to the introduction of Read Committed A-testing Testing tools and infrastructure C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant