Skip to content

Fix flaky Playground local cache tests - #39918

Open
bvolpato wants to merge 2 commits into
apache:masterfrom
bvolpato:bvolpato/deflake-playground-local-cache
Open

Fix flaky Playground local cache tests#39918
bvolpato wants to merge 2 commits into
apache:masterfrom
bvolpato:bvolpato/deflake-playground-local-cache

Conversation

@bvolpato

@bvolpato bvolpato commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

Summary

Two local-cache tests depended on runner timing or unsynchronized state:

  • TestLocalCache_GetValue and TestLocalCache_SetValue expired fixtures one millisecond after setup. Slow runners could cross that deadline before assertions ran. Their non-expired fixtures now use a one-minute deadline.
  • TestLocalCache_startGC slept for one millisecond, then read the cache map while the GC goroutine could delete from it. It now polls for cleanup within a bounded deadline while holding the cache mutex. Each subtest owns its context and expiration state, and the nil-cache case still verifies that GC stops.

Production cache behavior is unchanged.

Reproduction

The scheduled Playground precommit failed in TestLocalCache_SetValue/Set_value because GetValue reported the newly stored value as missing:

https://github.com/apache/beam/actions/runs/33273173203

The GC race reproduced immediately with:

go test -race ./internal/cache/local -run '^TestLocalCache_startGC$'

Race detector reported an unlocked test read at local_cache_test.go:575 concurrent with deletion in Cache.clearItems.

Testing

  • cd playground/backend && go test ./internal/cache/local
  • cd playground/backend && go test ./internal/cache/local -run '^TestLocalCache_(GetValue|SetValue)$' -count=10000
  • cd playground/backend && go test -race ./internal/cache/local -run '^TestLocalCache_(GetValue|SetValue)$' -count=100
  • cd playground/backend && go test -race ./internal/cache/local -run '^TestLocalCache_startGC$' -count=1000
  • cd playground/backend && go test -race ./internal/cache/local -count=1

./gradlew :playground:backend:test also ran the changed cache package successfully, then stopped on unrelated local emulator and Scio setup failures.


  • No tracking issue; failing CI run and race reproducer are included above.
  • CHANGES.md is unchanged because this is a test-only stabilization.
  • This contribution is not large.

@bvolpato
bvolpato force-pushed the bvolpato/deflake-playground-local-cache branch from cb0d5fd to c792663 Compare August 29, 2026 22:51
@bvolpato
bvolpato marked this pull request as ready for review August 29, 2026 23:05
@github-actions

Copy link
Copy Markdown
Contributor

Assigning reviewers:

R: @damccorm added as fallback since no labels match configuration

Note: If you would like to opt out of this review, comment assign to next reviewer.

Available commands:

  • stop reviewer notifications - opt out of the automated review tooling
  • remind me after tests pass - tag the comment author after tests pass
  • waiting on author - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)

The PR bot will only process comments in the main thread (not review comments).

@bvolpato
bvolpato marked this pull request as draft August 30, 2026 05:29
@bvolpato
bvolpato force-pushed the bvolpato/deflake-playground-local-cache branch from c792663 to 10038b9 Compare August 30, 2026 05:38
@bvolpato bvolpato changed the title Fix flaky Playground local cache test Fix flaky Playground local cache tests Aug 30, 2026
@bvolpato
bvolpato marked this pull request as ready for review August 30, 2026 05:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant