Skip to content

ci: increase integration-test-build timeout from 45 to 90 minutes#6244

Merged
desmondcheongzx merged 1 commit intomainfrom
ci/bump-integration-build-timeout
Feb 19, 2026
Merged

ci: increase integration-test-build timeout from 45 to 90 minutes#6244
desmondcheongzx merged 1 commit intomainfrom
ci/bump-integration-build-timeout

Conversation

@desmondcheongzx
Copy link
Copy Markdown
Collaborator

The 45-minute timeout from #6241 is still not enough — the build timed out again in this run because the Rust cache (Swatinem/rust-cache) missed entirely, forcing a cold build of all 676 crates. Bumping to 90 minutes so the cold build can complete and repopulate the cache.

Longer term we should investigate sccache or larger runners to make cold builds more robust.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@github-actions github-actions Bot added the ci label Feb 19, 2026
@desmondcheongzx desmondcheongzx enabled auto-merge (squash) February 19, 2026 08:16
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Feb 19, 2026

Greptile Summary

Doubled the integration-test-build job timeout from 45 to 90 minutes to accommodate cold Rust builds when the cache misses.

  • Pragmatic fix for immediate CI reliability — cold builds of 676 crates exceeded the previous 45-minute limit
  • Second timeout increase in recent commits (30→45→90 minutes), indicating cache miss is a recurring issue
  • PR author acknowledges this is a band-aid and mentions investigating sccache or larger runners as long-term solutions

Confidence Score: 5/5

  • Safe to merge — simple timeout adjustment with no code changes
  • This is a minimal risk configuration change that only increases a CI timeout value. The change is well-justified by the linked CI run showing timeout failures, and the author has identified the root cause (cold cache forcing full rebuild of 676 crates). No logic changes or new functionality introduced.
  • No files require special attention

Important Files Changed

Filename Overview
.github/workflows/pr-test-suite.yml Increased integration-test-build timeout from 45 to 90 minutes to handle cold Rust cache builds

Last reviewed commit: 7e2b655

Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot left a comment

Choose a reason for hiding this comment

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

1 file reviewed, no comments

Edit Code Review Agent Settings | Greptile

@codecov
Copy link
Copy Markdown

codecov Bot commented Feb 19, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.33%. Comparing base (f1ca321) to head (7e2b655).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #6244      +/-   ##
==========================================
- Coverage   73.36%   73.33%   -0.03%     
==========================================
  Files         999      999              
  Lines      132543   132543              
==========================================
- Hits        97240    97207      -33     
- Misses      35303    35336      +33     

see 4 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@desmondcheongzx desmondcheongzx merged commit 4bc428e into main Feb 19, 2026
34 of 35 checks passed
@desmondcheongzx desmondcheongzx deleted the ci/bump-integration-build-timeout branch February 19, 2026 09:26
desmondcheongzx added a commit that referenced this pull request Feb 20, 2026
Add `cache-workspace-crates: "true"` to the Swatinem/rust-cache config
for the integration-test-build job.

`cache-all-crates` only controls registry cleanup (downloaded `.crate`
files and extracted sources in `~/.cargo/registry/`). Workspace crate
fingerprints, build script outputs, and compiled `.rlib`/`.rmeta`
artifacts in `target/` are cleaned by `cleanTargetDir()` during cache
save — which builds its keep-list from
`getPackagesOutsideWorkspaceRoot()`, explicitly excluding workspace
members. This caused all ~70 local crates to recompile from scratch
every run (~25 min at `opt-level=3`).

`cache-workspace-crates: "true"` adds workspace members to the keep-list
so their artifacts survive the save cleanup.

## Results

Tested on this PR with temporary `save-if: true` to bootstrap the cache:

| Metric | Cold cache | Warm cache |
|---|---|---|
| Cache restore | 2s | 21s |
| Build wheels | **36 min** | **10 sec** |
| Total job | ~37 min | ~1 min 41 sec |

Combined with the restore-mtime fix from #6246 (source file mtimes older
than cached dep-info mtimes), cargo now sees all workspace crates as
fresh.

See #6244, #6246.

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
desmondcheongzx added a commit that referenced this pull request Feb 20, 2026
…#6264)

Extract the inline restore-mtime script into a reusable composite action
(`.github/actions/restore-mtime/`) and apply the full cargo cache
treatment to all 10 jobs that use Swatinem/rust-cache:

- `fetch-depth: 0` on checkout (full history for mtime restoration)
- `restore-mtime` composite action (deterministic source file timestamps
for cargo fingerprinting)
- `cache-workspace-crates: "true"` (preserve workspace crate artifacts
in cache — the key fix from #6261)
- `shared-key` by cargo profile (consolidate caches instead of one per
job)
- `save-if: main-only` (single source of truth, avoids PR cache
pollution and LRU eviction)

Cache keys are shared by cargo profile:
- `Linux-dev-build`: unit-test, rust-tests, doctests, test-imports,
style, docgen (build-docs.yml)
- `Linux-integration-build`: integration-test-build, profile-daft,
property-based-tests
- `Linux-dev-bench-build`: benchmark-codspeed (buildjet, separate cache
provider)

Previously each job maintained its own cache via `prefix-key`, resulting
in ~8 separate cache entries for the same profile. Consolidating to 3
shared keys frees significant space against the 10 GB repo cache limit.

On the integration-test-build job (#6261), this approach reduced Build
wheels from 36 min to 10 sec on warm cache. Expect similar improvements
for unit-test (~8 min build) and benchmark-codspeed (~25 min compile +
link).

See #6244, #6246, #6261.

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
gavin9402 pushed a commit to gavin9402/Daft that referenced this pull request Apr 7, 2026
…entual-Inc#6244)

The 45-minute timeout from Eventual-Inc#6241 is still not enough — the build timed
out again in [this
run](https://github.com/Eventual-Inc/Daft/actions/runs/22172157962/job/64112220739)
because the Rust cache (`Swatinem/rust-cache`) missed entirely, forcing
a cold build of all 676 crates. Bumping to 90 minutes so the cold build
can complete and repopulate the cache.

Longer term we should investigate `sccache` or larger runners to make
cold builds more robust.

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
gavin9402 pushed a commit to gavin9402/Daft that referenced this pull request Apr 7, 2026
…c#6261)

Add `cache-workspace-crates: "true"` to the Swatinem/rust-cache config
for the integration-test-build job.

`cache-all-crates` only controls registry cleanup (downloaded `.crate`
files and extracted sources in `~/.cargo/registry/`). Workspace crate
fingerprints, build script outputs, and compiled `.rlib`/`.rmeta`
artifacts in `target/` are cleaned by `cleanTargetDir()` during cache
save — which builds its keep-list from
`getPackagesOutsideWorkspaceRoot()`, explicitly excluding workspace
members. This caused all ~70 local crates to recompile from scratch
every run (~25 min at `opt-level=3`).

`cache-workspace-crates: "true"` adds workspace members to the keep-list
so their artifacts survive the save cleanup.

## Results

Tested on this PR with temporary `save-if: true` to bootstrap the cache:

| Metric | Cold cache | Warm cache |
|---|---|---|
| Cache restore | 2s | 21s |
| Build wheels | **36 min** | **10 sec** |
| Total job | ~37 min | ~1 min 41 sec |

Combined with the restore-mtime fix from Eventual-Inc#6246 (source file mtimes older
than cached dep-info mtimes), cargo now sees all workspace crates as
fresh.

See Eventual-Inc#6244, Eventual-Inc#6246.

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
gavin9402 pushed a commit to gavin9402/Daft that referenced this pull request Apr 7, 2026
…Eventual-Inc#6264)

Extract the inline restore-mtime script into a reusable composite action
(`.github/actions/restore-mtime/`) and apply the full cargo cache
treatment to all 10 jobs that use Swatinem/rust-cache:

- `fetch-depth: 0` on checkout (full history for mtime restoration)
- `restore-mtime` composite action (deterministic source file timestamps
for cargo fingerprinting)
- `cache-workspace-crates: "true"` (preserve workspace crate artifacts
in cache — the key fix from Eventual-Inc#6261)
- `shared-key` by cargo profile (consolidate caches instead of one per
job)
- `save-if: main-only` (single source of truth, avoids PR cache
pollution and LRU eviction)

Cache keys are shared by cargo profile:
- `Linux-dev-build`: unit-test, rust-tests, doctests, test-imports,
style, docgen (build-docs.yml)
- `Linux-integration-build`: integration-test-build, profile-daft,
property-based-tests
- `Linux-dev-bench-build`: benchmark-codspeed (buildjet, separate cache
provider)

Previously each job maintained its own cache via `prefix-key`, resulting
in ~8 separate cache entries for the same profile. Consolidating to 3
shared keys frees significant space against the 10 GB repo cache limit.

On the integration-test-build job (Eventual-Inc#6261), this approach reduced Build
wheels from 36 min to 10 sec on warm cache. Expect similar improvements
for unit-test (~8 min build) and benchmark-codspeed (~25 min compile +
link).

See Eventual-Inc#6244, Eventual-Inc#6246, Eventual-Inc#6261.

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant