sql/logictest: fix flakes in select_for_update_read_committed #112701
Conversation
mgartner
requested review from
a team and
michae2
and removed request for
a team
|
This change is |
mgartner
force-pushed
the
112677-fix-read-committed-flakes
branch
from
9c63f1d to
781841d
Compare
There was a problem hiding this comment.
Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @mgartner)
There was a problem hiding this comment.
Reviewable status:
pkg/sql/logictest/testdata/logic_test/select_for_update_read_committed line 81 at r1 (raw file):
COMMIT; ---- 20 200 2000
I think what's happening here is that SKIP LOCKED doesn't quite work with shared locks yet: #110743
Maybe we could bring the (incorrect) 3-row result back, with a note about that issue?
|
Previously, michae2 (Michael Erickson) wrote…
I can't get this failure to reproduce locally, and if I add the three rows back the tests fails locally. |
mgartner
force-pushed
the
112677-fix-read-committed-flakes
branch
from
781841d to
46c4355
Compare
|
Previously, mgartner (Marcus Gartner) wrote…
Nevermind, I think I found the problem—I forgot to include the change to set the txn isolation of the first BEGIN. |
The `select_for_update_read_committed` tests were flaking because not all statements were being run under READ COMMITTED isolation. The logic test infrastructure does not allow fine-grained control of sessions, and setting the isolation level in one statement would only apply to a single session. Subsequent statements are not guaranteed to run in the same session because they could run in any session in the connection pool. This commit wraps each statement in an explicitly transaction with an explicit isolation level to ensure READ COMMITTED is used. In the future, we should investigate allowing fine-grained and explicit control of sessions in logic tests. Fixes cockroachdb#112677 Release note: None
mgartner
force-pushed
the
112677-fix-read-committed-flakes
branch
from
46c4355 to
430cffc
Compare
|
TFTR! bors r+ |
|
Build succeeded: |
The
select_for_update_read_committedtests were flaking because notall statements were being run under READ COMMITTED isolation. The logic
test infrastructure does not allow fine-grained control of sessions, and
setting the isolation level in one statement would only apply to a
single session. Subsequent statements are not guaranteed to run in the
same session because they could run in any session in the connection
pool. This commit wraps each statement in an explicitly transaction with
an explicit isolation level to ensure READ COMMITTED is used.
In the future, we should investigate allowing fine-grained and explicit
control of sessions in logic tests.
Fixes #112677
Release note: None