ClickHouse / ClickHouse/clickhouse-go
Failures with Test #1229
- Dominant language
- Go
- Stars
- 3.3k
- Forks
- 684
- Avg merge
- 2d 3h
- Merged PRs (30d)
- 14
Description
## Description
Test 1229 is failing due to timeout issues during concurrent query execution. The test creates 100 concurrent queries with a 4 second timeout. When the timeout is exceeded, `rows.Close()` is called on context that has already been cancelled, which results in `context deadline exceeded` errors and `i/o timeout` errors on packet read.
The core problem is that after `conn.Query()` completes (successfully or not) within a cancelled context, calling `rows.Close()` on that same cancelled context can fail. The test doesn't properly handle the scenario where the query times out.
## Code Solution
```go
func Test1229(t *testing.T) {
const (
queryTimeout = 4 * time.Second
concurrency = 100
)
var (
conn, err = clickhouse_tests.GetConnectionTCPWithOptions("issues", clickhouse.Settings{
"max_execution_time": 60,
}, nil, &clickhouse.Compression{
Method: clickhouse.CompressionLZ4,
}, func(o *clickhouse.Options) {
o.MaxOpenConns = concurrency
o.MaxIdleConns = concurrency
})
)
require.NoError(t, err)
ctx := context.Background()
const ddl = "CREATE TABLE IF NOT EXISTS test_1229 (`test1` String, `test2` String) Engine = Memory"
require.NoError(t, conn.Exec(ctx, ddl))
defer func() {
require.NoError(t, conn.Exec(ctx, "DROP TABLE IF EXISTS test_1229"))
}()
const insertQuery = "INSERT INTO test_1229 VALUES ('test1value%d', 'test2value%d')"
for i := 0; i < concurrency; i++ {
withTimeoutCtx, cancel := context.WithTimeout(ctx, queryTimeout)
require.NoError(t, conn.Exec(withTimeoutCtx, fmt.Sprintf(insertQuery, i, i)))
cancel()
}
wg := new(sync.WaitGroup)
const selectQuery = "SELECT test1, test2 FROM test_1229"
errTestQueryTimeout := fmt.Errorf("Test1229: query budget %s exceeded", queryTimeout)
for i := 0; i < concurrency; i++ {
wg.Add(1)
go func() {
defer wg.Done()
withTimeoutCtx, cancel := context.WithTimeoutCause(ctx, queryTimeout, errTestQueryTimeout)
defer cancel()
rows, err := conn.Query(withTimeoutCtx, selectQuery)
// Only close rows if query was successful
if err == nil && rows != nil {
// Use background context to close rows to avoid cancellation issues
closeErr := rows.Close()
require.NoErrorf(t, closeErr, "rows.Close failed; ctx cause=%v", context.Cause(withTimeoutCtx))
} else if err != nil {
// Context deadline exceeded is acceptable in this test
require.Truef(t, context.Cause(withTimeoutCtx) != nil ||
err == context.DeadlineExceeded,
"query failed with unexpected error: %v; ctx cause=%v", err, context.Cause(withTimeoutCtx))
}
}()
}
wg.Wait()
assert.EventuallyWithT(t, func(ct *assert.CollectT) {
openConnections := conn.Stats().Open
assert.Zerof(ct, openConnections, "open connections: %d", openConnections)
}, time.Second*5, time.Millisecond*10)
}
```
- We should check if rows are valid before calling `Close()`
- We should accept context deadline exceeded errors
- We should properly validate error conditions
Contributor guide
Research direction
Start with Test1229 and its concurrent conn.Query calls, then inspect how rows.Close handles queries whose contexts have been cancelled. Run the test with its ClickHouse connection setup and verify timeout errors are accepted, valid rows are closed safely, and conn.Stats().Open reaches zero.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- clickhouse, go
- Domain
- databases, testing-qa
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 72/100