fix: respect clickhouse max_query_size#4593
Conversation
dtunikov
changed the title
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
🔄 Flaky Test DetectedAnalysis: The flow/e2e package hit its exact 1200s timeout (a whole-suite hang, no assertion/panic trace) in only 1 of 3 matrix cells — the identical ClickHouse code from the PR passed in the other two cells — which is a classic flaky-timeout signature rather than a deterministic bug. ✅ Automatically retrying the workflow |
|
|
||
| // fetchURLBatchLimits reads the effective max_query_size and | ||
| // table_function_remote_max_addresses from ClickHouse directly. | ||
| func (c *ClickHouseConnector) fetchURLBatchLimits(ctx context.Context) urlBatchLimits { |
There was a problem hiding this comment.
Add an integration test for this one? Possible to override settings within a connection and not affect other tests
| rows, err := c.query(ctx, | ||
| "SELECT name, value FROM system.settings WHERE name IN ('max_query_size', 'table_function_remote_max_addresses')") | ||
| if err != nil { | ||
| c.logger.Warn("failed to read ClickHouse batch-limit settings, using defaults", slog.Any("error", err)) |
There was a problem hiding this comment.
Would prefer to stay deterministic in face of network hiccups. I guess if the defaults are not good enough, the worst that's going to happen is either failing and successfully retrying, or too small batching, but the error we'd receive would be the same as today and we'd get paged. Let's just return the error
2 participants
No description provided.