ArafatKhan2198 commented on code in PR #7723:
URL: https://github.com/apache/ozone/pull/7723#discussion_r1961162541


##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/FileSizeCountTask.java:
##########
@@ -91,11 +92,17 @@ public Pair<String, Boolean> reprocess(OMMetadataManager 
omMetadataManager) {
         reprocessBucketLayout(BucketLayout.LEGACY, omMetadataManager,
             fileSizeCountMap);
     if (!statusFSO && !statusOBS) {
-      return new ImmutablePair<>(getTaskName(), false);
+      return new TaskResult.Builder()
+          .setTaskName(getTaskName())
+          .setTaskSuccess(false)
+          .build();

Review Comment:
   If we find ourselves repeating the same TaskResult creation code frequently, 
it can be beneficial to extract it into a helper method. 
   
   ```suggestion
         if (!statusFSO && !statusOBS) {
           return buildTaskResult(false);
         }
   ```
   
   New method added would be for this class :- 
   ```
   private TaskResult buildTaskResult(boolean success) {
     return new TaskResult.Builder()
         .setTaskName(getTaskName())
         .setTaskSuccess(success)
         .build();
   }
   ```
   
   Not a major deal but would create simplicity and a reduce the code. We could 
do the same for other classes as well.



##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/FileSizeCountTask.java:
##########
@@ -138,11 +145,17 @@ public Collection<String> getTaskTables() {
    * Read the Keys from update events and update the count of files
    * pertaining to a certain upper bound.
    *
-   * @param events Update events - PUT/DELETE.
-   * @return Pair
+   * @param events            The batch of OM update events to be processed.
+   * @param subTaskSeekPosMap A map containing the seek positions for
+   *                          each sub-task, indicating where to start 
processing.
+   * @return A {@link TaskResult} containing:
+   *         - The task name.
+   *         - A map of sub-task names to their respective seek positions.
+   *         - A boolean indicating whether the task was successful.
    */
   @Override
-  public Pair<String, Boolean> process(OMUpdateEventBatch events) {
+  public TaskResult process(OMUpdateEventBatch events,
+                            Map<String, Integer> subTaskSeekPosMap) {

Review Comment:
   We aren't doing anything with subTaskSeekPosMap here in this Class?



##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/FileSizeCountTask.java:
##########
@@ -189,7 +202,10 @@ public Pair<String, Boolean> process(OMUpdateEventBatch 
events) {
         } catch (Exception e) {
           LOG.error("Unexpected exception while processing key {}.",
               updatedKey, e);
-          return new ImmutablePair<>(getTaskName(), false);
+          return new TaskResult.Builder()
+              .setTaskName(getTaskName())
+              .setTaskSuccess(false)
+              .build();

Review Comment:
   We could call the helper build method for this as well.



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