Skip to content

Scope shared TestDataflowRunner staging directory by UTC date - #41258

Draft
damccorm wants to merge 3 commits into
masterfrom
fix/dataflow-package-util-restage-expired
Draft

damccorm wants to merge 3 commits into
masterfrom
fix/dataflow-package-util-restage-expired

Conversation

@damccorm

@damccorm damccorm commented Nov 13, 2026 •

Copy link
Copy Markdown
Contributor

Note: this PR was auto-generated by tooling. Before moving it out of draft, the author has done a full review of the PR, specifically evaluating it on the following criteria:

  • The PR is an appropriate fix for the issue (e.g. we're not changing a test to avoid a bug)
  • The PR is a simple fix to the issue (e.g. we're not adding infra when we could just modify a test)
  • The PR is not covering for other problems (e.g. if we're bumping timeouts, it is not because we're covering for a performance problem)

Full details of the methodology of generation and review can be found here - https://docs.google.com/document/d/14lj7ECVov4T4EdwmIzNlWrRGE3lLDMOwX5xUMGzaLoc/edit?usp=sharing

If you have concerns with the PR or with the underlying methodology used to generate this, please comment below - they will be addressed by the author not the tooling :)

Summary

Fixes recurring Workflow failed. Causes: One or more access checks for temp location or staged files failed" failures in PostCommit Java Examples Dataflow V2 (run 43313768013) and PostCommit Java ValidatesRunner Dataflow V2 Streaming (run 43025792107).

Root Cause

  • In PR #40321 (fa34c16167c), TestDataflowRunner.fromOptions and runners/google-cloud-dataflow-java/build.gradle were updated from per-job staging (<tempLocation>/staging) to a static shared staging path (<tempRoot>/staging, i.e., gs://temp-storage-for-validates-runner-tests/staging) across all runs.
  • gs://temp-storage-for-validates-runner-tests is configured with a 7-day GCS object lifecycle Delete rule (age: 7).
  • Because third-party dependency JARs have content-addressed filenames that never change across commits, PackageUtil.alreadyStaged sees an object staged ~6.99 days ago during job submission and skips re-uploading it, and minutes later GCS's 7-day lifecycle sweeper deletes the object while the Dataflow worker is starting up.

Fix

  • Scope the shared TestDataflowRunner staging directory under tempRoot by UTC date (staging-<yyyy-MM-dd>) in TestDataflowRunner.fromOptions whenever the default per-job staging path (<tempLocation>/staging) or unversioned shared staging path (<tempRoot>/staging) is used, while preserving custom explicit --stagingLocation values.
  • All TestDataflowRunner pipelines executed on the same UTC day continue to share and reuse staged artifacts (via both GcsStager.STAGED_FILES_CACHE and PackageUtil.alreadyStaged), while staged test objects are guaranteed to be < 24h old and never near the 7-day GCS lifecycle deletion threshold.
  • Leaves PackageUtil untouched so production pipelines reuse previously staged artifacts indefinitely as intended.

Verification

  • ./gradlew :runners:google-cloud-dataflow-java:spotlessCheck :runners:google-cloud-dataflow-java:test --tests org.apache.beam.runners.dataflow.TestDataflowRunnerTest :runners:google-cloud-dataflow-java:checkstyleMain :runners:google-cloud-dataflow-java:checkstyleTest
  • Triggered PostCommit Java Examples Dataflow V2 (beam_PostCommit_Java_Examples_Dataflow_V2.yml) on branch fix/dataflow-package-util-restage-expired.

@damccorm damccorm added the test-failure-cleanup Automated test health and flake cleanup PRs label Nov 13, 2026
@damccorm

Copy link
Copy Markdown
Contributor Author

/gemini review

@github-actions github-actions Bot added runners dataflow size/s S: 10-49 lines changed labels Nov 13, 2026

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

📋 Review Summary

This PR updates PackageUtil.alreadyStaged to check remoteMetadata.lastModifiedMillis() and re-stage artifacts older than 1 day (MAX_STAGED_FILE_AGE), preventing shared staging objects from being deleted mid-job by GCS lifecycle policies (such as the 7-day TTL on gs://temp-storage-for-validates-runner-tests). The implementation and unit test coverage in PackageUtilTest are clean, well-scoped, and backwards-compatible with file systems or mocks that report 0 for lastModifiedMillis.

🔍 General Feedback

  • Guarding the age check with lastModifiedMillis > 0 nicely preserves behavior for file system implementations that do not populate modification timestamps as well as existing unit test fixtures.
  • Re-uploading objects older than 1 day refreshes the GCS object generation and creation timestamp well ahead of the 7-day lifecycle deletion window while maintaining cross-job caching within a 24-hour period.

💡 Tip: Comment /gemini review on this pull request to trigger a new Gemini code review.

@damccorm damccorm added the ready-for-human Draft PR with all checks passing and all review comments resolved, ready for human review label Nov 13, 2026
@codecov

codecov Bot commented Nov 13, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 58.54%. Comparing base (fa1d467) to head (524a9ee).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master   #41258      +/-   ##
============================================
+ Coverage     57.74%   58.54%   +0.80%     
+ Complexity     2634     2291     -343     
============================================
  Files          1203     1132      -71     
  Lines        171888   169864    -2024     
  Branches       2182     1487     -695     
============================================
+ Hits          99256    99454     +198     
+ Misses        69973    67866    -2107     
+ Partials       2659     2544     -115     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 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.

}
long lastModifiedMillis = remoteMetadata.lastModifiedMillis();
if (lastModifiedMillis > 0
&& Instant.now().getMillis() - lastModifiedMillis > MAX_STAGED_FILE_AGE.getMillis()) {

@damccorm damccorm Nov 13, 2026 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Instead of checking the if the file age is older than 1 day, can we figure out what the file's TTL is somehow and compare to that?

If not, we should find a different way of handling this (probably staging artifacts with unique names in our tests). The purpose of the staging directory is to reuse previously staged artifacts (even if they were staged a long time ago)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good call! GCS object metadata (Objects.get / FileSystems.matchSingleFileSpec) does not expose a TTL or lifecycle expiration timestamp, and bucket lifecycle rules (age: 7 on gs://temp-storage-for-validates-runner-tests) are only available via Buckets.get (storage.buckets.get), whereas production --stagingLocation directories are intended to reuse staged artifacts indefinitely.

Tracing git history showed this race started in PR #40321 (fa34c16167c), which changed TestDataflowRunner from per-job staging (<tempLocation>/staging) to a single static shared directory (<tempRoot>/staging). I reverted the PackageUtil changes and updated TestDataflowRunner.fromOptions in 524a9ee to scope the shared test staging directory under tempRoot by UTC date (staging-<yyyy-MM-dd>), so all tests running on the same day still share and reuse staged JARs while never referencing objects near the 7-day GCS lifecycle deletion threshold.

@damccorm damccorm removed the ready-for-human Draft PR with all checks passing and all review comments resolved, ready for human review label Nov 13, 2026
@damccorm damccorm changed the title Re-stage Dataflow packages older than 1 day in PackageUtil Scope shared TestDataflowRunner staging directory by UTC date Nov 13, 2026
@damccorm

Copy link
Copy Markdown
Contributor Author

/gemini review

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

📋 Review Summary

This PR resolves recurring Dataflow post-commit failures caused by the 7-day GCS lifecycle deletion rule on gs://temp-storage-for-validates-runner-tests deleting shared staged dependency JARs while jobs are starting up. By scoping the shared TestDataflowRunner staging directory under tempRoot by the UTC date (staging-<yyyy-MM-dd>), tests running on the same day continue to share staged artifacts and in-memory staging cache entries while ensuring staged objects are always fresh (< 24h old).

🔍 General Feedback

  • Normalizing rawStagingLocation via FileSystems.matchNewDirectory cleanly handles both default per-job paths and explicit --stagingLocation=<tempRoot>/staging flags (with or without a trailing slash) from build.gradle, while preserving custom staging locations.
  • Evaluating DAILY_STAGING_DIR once per JVM load is a good choice, as it keeps stagingLocation consistent across tests in the same JVM run (preserving GcsStager cache hits and DataflowTestBatchCoordinator compatibility keys even if a run crosses UTC midnight).
  • Unit tests in TestDataflowRunnerTest thoroughly cover default, unversioned shared, and custom staging location cases.

💡 Tip: Comment /gemini review on this pull request to trigger a new Gemini code review.

@damccorm damccorm added the ready-for-human Draft PR with all checks passing and all review comments resolved, ready for human review label Nov 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dataflow ready-for-human Draft PR with all checks passing and all review comments resolved, ready for human review runners size/s S: 10-49 lines changed test-failure-cleanup Automated test health and flake cleanup PRs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant