damccorm commented on code in PR #41258:
URL: https://github.com/apache/beam/pull/41258#discussion_r4559983844


##########
runners/google-cloud-dataflow-java/src/main/java/org/apache/beam/runners/dataflow/util/PackageUtil.java:
##########
@@ -122,11 +123,33 @@ private CompletionStage<PackageAttributes> 
computePackageAttributes(
         () -> PackageAttributes.forFileToStage(source, hash, dest, 
stagingPath), executorService);
   }
 
+  /**
+   * Maximum age of a previously staged file before it is re-uploaded. Shared 
test and staging
+   * buckets often configure GCS lifecycle deletion rules (for example, 7 days 
on {@code
+   * gs://temp-storage-for-validates-runner-tests}), so re-uploading files 
older than 1 day
+   * refreshes their object creation timestamp well before lifecycle deletion 
can race with job
+   * execution.
+   */
+  static final Duration MAX_STAGED_FILE_AGE = Duration.standardDays(1);
+
   private boolean alreadyStaged(PackageAttributes attributes) throws 
IOException {
     try {
-      long remoteLength =
-          
FileSystems.matchSingleFileSpec(attributes.getDestination().getLocation()).sizeBytes();
-      return remoteLength == attributes.getSize();
+      Metadata remoteMetadata =
+          
FileSystems.matchSingleFileSpec(attributes.getDestination().getLocation());
+      if (remoteMetadata.sizeBytes() != attributes.getSize()) {
+        return false;
+      }
+      long lastModifiedMillis = remoteMetadata.lastModifiedMillis();
+      if (lastModifiedMillis > 0
+          && Instant.now().getMillis() - lastModifiedMillis > 
MAX_STAGED_FILE_AGE.getMillis()) {

Review Comment:
   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](https://github.com/apache/beam/pull/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 
524a9ee2757 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.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to