Copilot commented on code in PR #11808:
URL: https://github.com/apache/ozone/pull/11808#discussion_r4395240361


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotDiffManager.java:
##########
@@ -799,13 +802,19 @@ Pair<List<DiffReportEntry>, String> createPageResponse(
    */
   @VisibleForTesting
   void checkReportsIntegrity(final SnapshotDiffJob diffJob,
+                             final String jobKey,
                              final String largestPageIndex,
                              boolean lastPage) throws IOException {
-    // For last page check last entry returned if the largest entry key equals 
the largest key stored in the job entry.
-    if (lastPage && (diffJob.getLargestEntryKey() == null || 
!diffJob.getLargestEntryKey().equals(largestPageIndex))) {
+    // A DONE job persisted before largestEntryKey was introduced deserializes 
with a
+    // null largestEntryKey. Such a legacy job carries no stored last-entry to 
compare,
+    // so skip the integrity assertion for it rather than treating it as a 
failure.
+    if (lastPage && diffJob.getLargestEntryKey() != null &&
+        !diffJob.getLargestEntryKey().equals(largestPageIndex)) {
       LOG.error("Expected last entry: {} but found " +
               "Largest Page entry: {}", diffJob.getLargestEntryKey(), 
largestPageIndex);
-      updateJobStatus(diffJob.getJobId(), DONE, FAILED);
+      // The job is stored under jobKey (fromSnapshotId + DELIMITER + 
toSnapshotId), not
+      // under its jobId; passing the jobId here would look up a missing row.
+      updateJobStatus(jobKey, DONE, FAILED);

Review Comment:
   A delayed reader of a corrupt report can mark a healthy replacement 
generation FAILED if another request resubmits and completes the job under the 
same `jobKey` before this transition. Atomically check that the current row 
still has `diffJob.getJobId()` and status DONE before updating it, following 
the generation guard in `updateJobStatusToDone`. Add a regression test that 
checks an old report after a replacement generation has completed.



-- 
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]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to