[GitHub] [ozone] rakeshadr commented on a change in pull request #2265: HDDS-5244. Allow multiple OM request versions to be supported at same layout version.

[GitBox <gi...@apache.org>]

rakeshadr commented on a change in pull request #2265:
URL: https://github.com/apache/ozone/pull/2265#discussion_r638083677



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/upgrade/OMLayoutFeatureAspect.java
##########
@@ -63,16 +68,47 @@ public void checkLayoutFeature(JoinPoint joinPoint) throws Throwable {
         lvm = new OMLayoutVersionManager();
       }
     }
+    checkIsAllowed(joinPoint.getSignature().toShortString(), lvm, featureName);
+  }
+
+  private void checkIsAllowed(String operationName,
+                              LayoutVersionManager lvm,
+                              String featureName) throws OMException {
     if (!lvm.isAllowed(featureName)) {
       LayoutFeature layoutFeature = lvm.getFeature(featureName);
       throw new OMException(String.format("Operation %s cannot be invoked " +
               "before finalization. It belongs to the layout feature %s, " +
               "whose layout version is %d. Current Layout version is %d",
-          joinPoint.getSignature().toShortString(),
+          operationName,
           layoutFeature.name(),
           layoutFeature.layoutVersion(),
           lvm.getMetadataLayoutVersion()),
           NOT_SUPPORTED_OPERATION);
     }
   }
+
+  @Pointcut("execution(* " +
+      "org.apache.hadoop.ozone.om.request.OMClientRequest+.preExecute(..)) " +
+      "&& @this(org.apache.hadoop.ozone.om.upgrade.BelongsToLayoutVersion)")
+  public void omRequestPointCut() {
+  }
+
+  @Before("omRequestPointCut()")
+  public void beforeRequestApplyTxn(final JoinPoint joinPoint)

Review comment:
       Could you please add some example case to show the usage of this preExecute hook. Thanks!

##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/utils/OzoneManagerRatisUtils.java
##########
@@ -58,110 +82,133 @@
  */
 public final class OzoneManagerRatisUtils {
 
-  private static final Logger LOG = LoggerFactory
-      .getLogger(OzoneManagerRatisUtils.class);
-
   private OzoneManagerRatisUtils() {
   }
-
-  public static OMClientRequest getRequest(OzoneManager om,
-                                           OMRequest omRequest) {
+  /**
+   * Create OMClientRequest which encapsulates the OMRequest.
+   * @param omRequest
+   * @return OMClientRequest
+   * @throws IOException
+   */
+  public static OMClientRequest createClientRequest(OMRequest omRequest) {
     Type cmdType = omRequest.getCmdType();
     switch (cmdType) {
+    case CreateVolume:
+      return new OMVolumeCreateRequest(omRequest);
+    case SetVolumeProperty:
+      boolean hasQuota = omRequest.getSetVolumePropertyRequest()
+          .hasQuotaInBytes();
+      boolean hasOwner = omRequest.getSetVolumePropertyRequest().hasOwnerName();
+      Preconditions.checkState(hasOwner || hasQuota, "Either Quota or owner " +
+          "should be set in the SetVolumeProperty request");
+      Preconditions.checkState(!(hasOwner && hasQuota), "Either Quota or " +
+          "owner should be set in the SetVolumeProperty request. Should not " +
+          "set both");
+      if (hasQuota) {
+        return new OMVolumeSetQuotaRequest(omRequest);
+      } else {
+        return new OMVolumeSetOwnerRequest(omRequest);
+      }
+    case DeleteVolume:
+      return new OMVolumeDeleteRequest(omRequest);
+    case CreateBucket:
+      return new OMBucketCreateRequest(omRequest);
+    case DeleteBucket:
+      return new OMBucketDeleteRequest(omRequest);
+    case SetBucketProperty:
+      return new OMBucketSetPropertyRequest(omRequest);
+    case AllocateBlock:
+      return new OMAllocateBlockRequest(omRequest);
+    case CreateKey:
+      return new OMKeyCreateRequest(omRequest);
+    case CommitKey:
+      return new OMKeyCommitRequest(omRequest);
+    case DeleteKey:
+      return new OMKeyDeleteRequest(omRequest);
+    case DeleteKeys:
+      return new OMKeysDeleteRequest(omRequest);
+    case RenameKey:
+      return new OMKeyRenameRequest(omRequest);
+    case RenameKeys:
+      return new OMKeysRenameRequest(omRequest);
+    case CreateDirectory:
+      return new OMDirectoryCreateRequest(omRequest);
+    case CreateFile:
+      return new OMFileCreateRequest(omRequest);

Review comment:
       Thanks @avijayanhwx for the patch. Appreciate, if you could explain me the changes to be done to incorporate HDDS-2939 prefix set of classes ?




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

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