sql: fix flake in TestTxnContentionEventsTable#118372
sql: fix flake in TestTxnContentionEventsTable#118372craig[bot] merged 1 commit intocockroachdb:masterfrom
Conversation
|
Shouldn't we fix the test proper? I think the problem is that We should also bump the value back to 500ms - this was changed recently in bd9d545. I'm thinking about something like this (haven't tested it though) Thoughts? |
michae2
left a comment
There was a problem hiding this comment.
Yes, you are right, I just got lazy. Good call.
I tried your idea, but it was still possible for the select pg_sleep to start executing before the UPDATE. I think we need to wait until the UPDATE has actually started executing. We can't figure that out in go, because conn.ExecContext is blocking, so I've added a retry loop that repeatedly queries crdb_internal.cluster_queries until it has seen the query. WDYT?
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @mgartner and @yuzefovich)
yuzefovich
left a comment
There was a problem hiding this comment.
Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @mgartner and @michae2)
pkg/sql/crdb_internal_test.go line 1009 at r2 (raw file):
// Wait for the update to show up in cluster_queries. var seen bool for i := 1; i <= 5 && !seen; i++ {
nit: consider using SucceedsSoon or SucceedsWithin helpers.
pkg/sql/crdb_internal_test.go line 1012 at r2 (raw file):
time.Sleep(time.Duration(i) * 10 * time.Millisecond) row := tx.QueryRowContext( ctx, "SELECT EXISTS(SELECT * FROM crdb_internal.cluster_queries WHERE query LIKE '%/* shuba */')",
ISWYDT 😃
In causeContention we deliberately hold a transaction open using pg_sleep to block an update statement. The timing we're trying to achieve is: 1. transaction insert 2. update starts and blocks 3. transaction held open using pg_sleep We were using a WaitGroup to order (2) after (1), but there was no synchronization to ensure (3) came after (2). This commit adds a retry loop that checks `crdb_internal.cluster_queries` to ensure (3) comes after (2). Fixes: cockroachdb#118236 Release note: None
|
TFTR! bors r=yuzefovich |
|
Build succeeded: |
In causeContention we deliberately hold a transaction open using pg_sleep to block an update statement. The timing we're trying to achieve is:
We were using a WaitGroup to order (2) after (1), but there was no synchronization to ensure (3) came after (2).
This commit adds a retry loop that checks
crdb_internal.cluster_queriesto ensure (3) comes after (2).Fixes: #118236
Release note: None