logictest: support async queries in logictests#79871
logictest: support async queries in logictests#79871craig[bot] merged 1 commit intocockroachdb:masterfrom
Conversation
|
This change is |
AlexTalks
left a comment
There was a problem hiding this comment.
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @nvanbenschoten and @yuzefovich)
pkg/sql/logictest/logic.go, line 3430 at r1 (raw file):
go func() { startedChan <- struct{}{} res, err := db.Exec(execSQL)
@nvanbenschoten this was the cause of the race condition in the tests we were looking at recently, unfortunately. The goroutine was racing with other operations that would modify t.db.
yuzefovich
left a comment
There was a problem hiding this comment.
Nice! Only nits from me,
Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status:
-- commits, line 26 at r1:
nit: it sounds like the fix should be in a separate commit.
pkg/sql/logictest/logic.go, line 568 at r1 (raw file):
const ( queryRewritePlaceholderPrefix = "__async_query_rewrite_placeholder"
nit: when defining a single const value we tend to not use the parenthesis and do it in a single line instead.
pkg/sql/logictest/logic.go, line 1071 at r1 (raw file):
logicQuery // the channel on which to receive the query results, when completed.
nit: s/the/The/.
AlexTalks
left a comment
There was a problem hiding this comment.
Reviewable status:
Previously, yuzefovich (Yahor Yuzefovich) wrote…
nit: it sounds like the fix should be in a separate commit.
I'll fix up the text here, but I put the fix in a separate PR already actually: #79925
pkg/sql/logictest/logic.go, line 568 at r1 (raw file):
Previously, yuzefovich (Yahor Yuzefovich) wrote…
nit: when defining a single const value we tend to not use the parenthesis and do it in a single line instead.
OK 👍
pkg/sql/logictest/logic.go, line 1071 at r1 (raw file):
Previously, yuzefovich (Yahor Yuzefovich) wrote…
nit:
s/the/The/.
OK 👍
6a60d91 to
0a784a8
Compare
0a784a8 to
08221ef
Compare
|
bors r+ |
79871: logictest: support async queries in logictests r=AlexTalks a=AlexTalks This change adds support for asynchronous queries in logictests, which will execute the query in a separate goroutine, prior to awaiting and validating the results when unblocked by any contending operations. While previously logictests had supported asynchronous statements as introduced in #79010, the `query` execution, which returns rows to be validated, is now supported in asynchronous mode with this change. This feature is supported with the following syntax: ``` query <typestring> async,<options...> namedQuery <some query that may block during execution> ---- <expected query results> ... <operations that would free the locks namedQuery is waiting on> ... awaitquery namedQuery <waits on namedQuery to complete and validate the results> ``` While this feature is not supported in conjunction with some query options such as `retry`, or when placed in a `repeat` block, it does support being run with the `--rewrite` flag. This change also fixes a bug with asynchronous statements that could cause a race condition in testing. Release note: None Co-authored-by: Alex Sarkesian <sarkesian@cockroachlabs.com>
|
Build failed: |
This change adds support for asynchronous queries in logictests, which will execute the query in a separate goroutine, prior to awaiting and validating the results when unblocked by any contending operations. While previously logictests had supported asynchronous statements as introduced in cockroachdb#79010, the `query` execution, which returns rows to be validated, is now supported in asynchronous mode with this change. This feature is supported with the following syntax: ``` query <typestring> async,<options...> namedQuery <some query that may block during execution> ---- <expected query results> ... <operations that would free the locks namedQuery is waiting on> ... awaitquery namedQuery <waits on namedQuery to complete and validate the results> ``` While this feature is not supported in conjunction with some query options such as `retry`, or when placed in a `repeat` block, it does support being run with the `--rewrite` flag. Release note: None
08221ef to
1613edb
Compare
|
bors r+ |
|
Build succeeded: |
|
Encountered an error creating backports. Some common things that can go wrong:
You might need to create your backport manually using the backport tool. error creating merge commit from 1613edb to blathers/backport-release-22.1-79871: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict [] you may need to manually resolve merge conflicts with the backport tool. Backport to branch 22.1.x failed. See errors above. 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan. |
This change adds support for asynchronous queries in logictests, which
will execute the query in a separate goroutine, prior to awaiting and
validating the results when unblocked by any contending operations.
While previously logictests had supported asynchronous statements as
introduced in #79010, the
queryexecution, which returns rows to bevalidated, is now supported in asynchronous mode with this change.
This feature is supported with the following syntax:
While this feature is not supported in conjunction with some query
options such as
retry, or when placed in arepeatblock, it doessupport being run with the
--rewriteflag. This change also fixes abug with asynchronous statements that could cause a race condition in
testing.
Release note: None