Skip to content

Conversation

@SpencerTorres
Copy link
Member

@SpencerTorres SpencerTorres commented Nov 6, 2025

Update versions

@SpencerTorres SpencerTorres changed the title Update run-tests.yml Update cloud version matrix (25.10) Nov 6, 2025
Copy link
Contributor

@kavirajk kavirajk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍 looks like some settings are failing in 25.10.

@kavirajk
Copy link
Contributor

kavirajk commented Nov 7, 2025

I'm taking a look at the failures.

@kavirajk
Copy link
Contributor

kavirajk commented Nov 7, 2025

Hmm. I think something changed in 25.10 when passing unknown settings.

In 25.9

 go run custom_settings.go
panic: sendQuery: [HTTP 404] response body: "Code: 115. DB::Exception: Unknown setting 'custom_non_existing_setting': In scope SELECT getSetting('custom_non_existing_setting'). (UNKNOWN_SETTING) (version 25.9.5.21 (official build))
	"

goroutine 1 [running]:
main.main()
	/home/kavi/src/play-go2/custom_settings.go:26 +0x115
exit status 2

But in 25.10 (looks like still 404, but body is missing the excpetion message?)

$
$ go run custom_settings.go
panic: sendQuery: [HTTP 404] response body: ""

goroutine 1 [running]:
main.main()
	/home/kavi/src/play-go2/custom_settings.go:26 +0x115
exit status 2
$

For the same code. I'm investigating more to narrow down. Looks like something related to compression

@kavirajk
Copy link
Contributor

kavirajk commented Nov 7, 2025

Hmm. Ok. I think found the root cause.

tldr; it is caused by enable_http_compression becoming default in 25.10 (but not server's fault)

Some background and my previous assumption:

In Go client. Unless user explicitly set settings.Compression during Open/OpenDB api, we don't pass neither HTTP headers nor compress query params.

As you can see in this code,

If the compression is zstd or lz4

  • we set compress=1 query parameter (basically asking server to compress the data)

If the compression is gzip, deflate or brotli

  • we set Accept-Encoding HTTP headers (basically asking server to use HTTP compression on the response)

There is one subtle misunderstanding in my above assumption. Even if user doesn't set any compression explicitly, Go standard library try to use gzip HTTP compression on the transport layer 😮‍💨 unless you explicitly disable it via DisableCompression: true, in the transport layer. And we use that transport in our Go driver without explicitly disabling the compression. Meaning sending Accept-Encoding: gzip all the time.

So basically what's changed?

Before 25.10 -> CH server ignores this Accept-Encoding: gzip compression by default (because enable_http_compression=0 by default) and always either compress the data based on compress query param. Which Go driver in-turn can correctly choose the compression connection pool to decompress the data.

After 25.10 -> CH server aware of this Accept-Encoding: gzip compression by default (because enable_http_compression=1 by default) and always use HTTP compression. When combined with compress query param, CH server also compress the data. So now the HTTP response body is compressed at two levels. This is the problem. But Go driver always assumes it's single compression. It's a bug in Go client.

Reason for test case failure on this PR

And test case is failing because, when CH send exception message on HTTP body (with double compressed 1. gzip HTTP compression 2. lz4 data compression), the client cannot decode the message when extracting error via row.Err().Error(). If I run with DisableCompression: true on the transport layer, everything works (because no double compression this time)

Signed-off-by: Kaviraj <kavirajkanagaraj@gmail.com>
@kavirajk
Copy link
Contributor

kavirajk commented Nov 7, 2025

@SpencerTorres @jkaflik can I have some extra eyes on this comment and this commit of this PR?

IdleConnTimeout: opt.ConnMaxLifetime,
ResponseHeaderTimeout: opt.ReadTimeout,
TLSClientConfig: opt.TLS,
DisableCompression: true,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given enable_http_compression is default in CH since 25.10. I would rather Go driver control the HTTP header (Accept-Encoding) fully instead of some magic happening behind Go standard library. That's the rationale of it.

@mshustov mshustov requested a review from chernser November 10, 2025 09:58
@kavirajk kavirajk self-assigned this Nov 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants