Skip to content

Conversation

@alpetric
Copy link
Collaborator

@alpetric alpetric commented Jan 7, 2026

No description provided.

@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Jan 7, 2026

Deploying windmill with  Cloudflare Pages  Cloudflare Pages

Latest commit: 04afa85
Status: ✅  Deploy successful!
Preview URL: https://7bc9d588.windmill.pages.dev
Branch Preview URL: https://alp-fix-gocache-warming.windmill.pages.dev

View logs

@claude
Copy link
Contributor

claude bot commented Jan 7, 2026

Claude finished @alpetric's task in 2m 33s —— View job


PR Review: fix: don't clear GOCACHE env in go runner and improve prewarming

  • Gather context and understand the changes
  • Review Dockerfile changes
  • Review go_executor.rs changes
  • Provide comprehensive feedback
  • Add testing instructions

Summary

This PR addresses two related issues with Go build performance in Windmill:

  1. Dockerfile: Improves the Go build prewarming step to more closely mirror Windmill's actual Go wrapper structure
  2. go_executor.rs: Adds the missing GOCACHE environment variable to the go build command

Review

backend/windmill-worker/src/go_executor.rs (Line 247)

Change: Added .env("GOCACHE", GO_CACHE_DIR) to the build_go_cmd Command.

Analysis: This is a critical fix. The code uses .env_clear() on line 233, which clears all environment variables including GOCACHE. While GOPATH is being set (line 236), GOCACHE was not being set, which would cause Go to use its default cache location instead of the prewarmed cache at /tmp/windmill/cache/go.

This change ensures the Go build process uses the prewarmed cache that was created in the Dockerfile, resulting in faster builds.

Dockerfile (Lines 204-213)

Change: The prewarming step now creates a Go project structure that mirrors Windmill's actual wrapper pattern:

  • Creates a module with an inner package
  • Uses the same imports (encoding/json, os, fmt)
  • Mirrors the main.go/inner pattern used by Windmill's Go runner

Before:

# Simple single-file warm.go with just fmt
go mod init gobuildwarm
printf "package foo\nimport (\"fmt\")\nfunc main() { fmt.Println(42) }" > warm.go

After:

# Multi-file structure matching Windmill's wrapper pattern
go mod init mymod
# Creates main.go with encoding/json, os, fmt, and inner package import
# Creates inner/inner.go with Req struct, Run function

This ensures the Go build cache is prewarmed for all the dependencies and build artifacts that Windmill actually uses when running Go scripts.


Potential Considerations

  1. Cache Compatibility: The prewarming now uses go build -x . (build package) instead of go build -x (build without specifying target). This is a minor change but should work correctly.

  2. Module Name: The module name changed from gobuildwarm to mymod. This is intentional to better match how Windmill dynamically generates Go modules.


Verdict

Looks good to merge. The changes are well-thought-out and address a real performance issue by ensuring the Go build cache is properly utilized during runtime.


Testing Instructions

To verify these changes, create a new Go script in Windmill and run it. The first execution should be faster than before since the Go build cache is now properly prewarmed and utilized. Navigate to the script editor, select Go as the language, write a simple script (e.g., one that imports encoding/json and returns a JSON object), and run it. The build step in the job logs should show cache hits instead of compiling all dependencies from scratch.

@rubenfiszel rubenfiszel merged commit 0427b43 into main Jan 8, 2026
23 checks passed
@rubenfiszel rubenfiszel deleted the alp/fix_gocache_warming branch January 8, 2026 06:31
@github-actions github-actions bot locked and limited conversation to collaborators Jan 8, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants