Re: [PR] HDDS-11975. wrap TermIndex in ExecutionContext [ozone]

["adoroszlai (via GitHub)" <gi...@apache.org>]

adoroszlai commented on code in PR #7602:
URL: https://github.com/apache/ozone/pull/7602#discussion_r1902774504


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/OMClientRequest.java:
##########
@@ -140,12 +140,15 @@ public void handleRequestFailure(OzoneManager ozoneManager) {
    *
    * @return the response that will be returned to the client.
    */
-  public abstract OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, TermIndex termIndex);
+  public abstract OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, ExecutionContext context);
 
   /** For testing only. */
   @VisibleForTesting
   public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, long transactionLogIndex) {
-    return validateAndUpdateCache(ozoneManager, TransactionInfo.getTermIndex(transactionLogIndex));
+    ExecutionContext context = new ExecutionContext();
+    context.setTermIndex(TransactionInfo.getTermIndex(transactionLogIndex));
+    context.setIndex(transactionLogIndex);

Review Comment:
   Please improve safety of `ExecutionContext`:
   
   1. Remove mutators (`set...`)
   2. Create private constructors
   3. Make member variables `final`
   4. Introduce factory methods like:
   
   ```java
     public static ExecutionContext of(long index)
     public static ExecutionContext of(TermIndex termIndex)
   ```
   
   This reduces steps required in code that creates instances, and ensures only valid `ExecutionContext` can be created:
   
   ```suggestion
       return validateAndUpdateCache(ozoneManager, ExecutionContext.forIndex(transactionLogIndex));
   ```
   
   If you plan to add many more members, then add builder instead of factory.



##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/ExecutionContext.java:
##########
@@ -0,0 +1,47 @@
+/**

Review Comment:
   nit: license header is not javadoc
   
   ```suggestion
   /*
   ```



-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org