ClickHouse / ClickHouse/clickhousectl

Avoid repeat latest downloads when the master freshness HEAD request times out

Open
#873 0 comments 0 reactions 1 assignee Claimed by @sdairs View on GitHub
Dominant language
Rust
Stars
74
Forks
5
Avg merge
2d 11h
Merged PRs (30d)
196

Description

## Issue

In clickhousectl v0.4.2, `chctl local install latest` successfully installs a master build, but an immediately subsequent `chctl local use latest` downloads the entire binary again when the master freshness check times out.

The user reported installing `26.9.1.1292`, seeing it in `local list`, and then downloading it again with `local use latest`. Both invocations printed:

```text
Master freshness check skipped: master check request to builds.clickhouse.com failed (timeout)
```

The first download completed successfully (~163 MiB). The screenshots establish the timeout and repeated download, but do not establish whether the timeout originated in the user's network, a proxy, or upstream latency.

## Reproduction

1. Use clickhousectl v0.4.2 on a connection where the master binary's `HEAD` request times out but its `GET` succeeds. For deterministic regression coverage, simulate this with a mock server or proxy.
2. Run `chctl local install latest`.
3. Observe the freshness timeout followed by a successful download and installation, e.g. `Installed ClickHouse 26.9.1.1292`.
4. Run `chctl local list` and confirm that version is installed.
5. Without changing the upstream master artifact, run `chctl local use latest`.
6. Observe another freshness timeout and a full binary download.

An existing default remaining selected after `install` is expected and separate from this issue: `install` preserves an existing default; `use` changes it.

## Cause

- `latest` resolves to the rolling master artifact, without an exact version known before download. It deliberately bypasses matching against installed version numbers: multiple master builds can report the same version.
- `install_resolved` attempts a `HEAD` request and only reuses an installed master build when the remote ETag matches the cached record and the binary exists.
- A failed `HEAD` therefore forces a download, even when an unchanged master build is already installed.
- A successful download does not repair the cache after a failed `HEAD`: the downloader returns no response metadata, and installation records freshness only when the earlier `HEAD` provided it. The successful `GET` response's ETag is discarded.

Relevant code: `crates/clickhousectl/src/version_manager/{install,master,download,resolve,network}.rs`. This version-manager code is unchanged between the v0.4.2 tag and the checkout investigated.

The freshness request has a 3-second connect timeout, 5-second read timeout, and 8-second overall timeout. Downloads have more generous limits, so a metadata timeout does not imply the binary download will fail.

## Proposed improvement

Make successful downloads populate the master cache, and support conditional downloads when freshness cannot be established through `HEAD`:

1. Return the ETag and optional Last-Modified metadata from the successful download response. Persist that metadata alongside the detected version after the binary is successfully committed. Use the metadata from the response that supplied the bytes, so the record describes the installed artifact even if master changes between requests.
2. When a cached master record and its installed binary exist, use `If-None-Match` on the `GET` request if `HEAD` fails. Alternatively, replace the separate `HEAD` check with a conditional `GET`.
3. Handle `304 Not Modified` as successful reuse of the installed binary, without downloading a body or running version detection again.
4. Handle `200 OK` as a new download and atomically commit the binary and its corresponding freshness record using the existing install locking/invalidation rules.
5. Preserve `--force` as an unconditional re-download. Do not send a validator when the recorded binary is missing, and do not silently claim an installed build is current when remote validation fails.

## Resulting behavior

- **First install; HEAD times out, GET succeeds:** installation succeeds and caches the GET response's ETag. It does not leave freshness caching unavailable solely because HEAD failed.
- **Repeat `local use latest`; artifact unchanged:** even if HEAD still times out, a conditional GET returns 304. The CLI reuses the installed build and sets it as default without another ~163 MiB transfer.
- **Repeat `local install latest`; artifact unchanged:** reuse succeeds without another full download; an existing default remains unchanged.
- **Master artifact changes, including changes with the same version string:** the conditional GET returns 200; the CLI downloads and installs the new bytes and updates the cache.
- **Recorded binary is missing or `--force` is used:** perform a full download.
- **No usable cached validator exists:** one full download may be necessary to establish the cache. If the server supplies no usable ETag, future reuse cannot be guaranteed by this mechanism.
- **Remote validation/download fails:** report the failure; do not silently substitute an unverified older build for `latest`.

Shared installation callers, including `local server start --version latest`, should benefit from the same behavior.

## Verification / acceptance criteria

Add deterministic coverage for the sequence `HEAD timeout → GET 200 with ETag → repeated request with If-None-Match → 304`, asserting that only one binary body is transferred and `use` selects the installed version. Also cover changed ETags with identical version strings, missing binaries, `--force`, missing validators, failed downloads, and differing HEAD/GET ETags so cache metadata always corresponds to the committed bytes.

Investigation validation: all 14 existing master-cache tests passed, including the test that explicitly disallows reuse after a failed HEAD. Exact-version `local use` also succeeded with an unreachable proxy in an isolated temporary home.

## Workaround

Select the exact version already listed as installed:

```sh
chctl local use 26.9.1.1292
```

This reuses the installed binary without a network lookup; it pins that version rather than checking the current master artifact.

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.