errose28 commented on a change in pull request #3175:
URL: https://github.com/apache/ozone/pull/3175#discussion_r828398319
##########
File path:
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java
##########
@@ -1069,6 +1069,20 @@ public boolean isFSOptimizedBucket() {
return false;
}
+ /**
+ * only for ofs.
+ * @param volumeName
+ * @param bucketName
+ * @return
+ * @throws IOException
+ */
+ public boolean isFSOptimizedBucket(String volumeName, String bucketName)
Review comment:
This looks unused right now. Is there a reason to add this?
##########
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java
##########
@@ -533,6 +533,8 @@ private boolean isKeyEmpty(OmKeyInfo keyInfo) {
int maxKeys) throws IOException {
Preconditions.checkNotNull(volumeName);
Preconditions.checkNotNull(bucketName);
+ Preconditions.checkArgument(!isBucketFSOptimized(volumeName, bucketName),
+ "ListKeys called for fso bucket instead of listStatus");
Review comment:
This should be an error returned to the client, not a precondition check
that crashes the OM. This is because the RPC endpoint is open for anyone to
call with whatever values they want, and it is our responsibility to handle
that on the server side without crashing.
##########
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketCreateRequest.java
##########
@@ -152,6 +152,9 @@ public OMClientResponse validateAndUpdateCache(OzoneManager
ozoneManager,
String omDefaultBucketLayout = ozoneManager.getOMDefaultBucketLayout();
BucketLayout defaultType =
BucketLayout.fromString(omDefaultBucketLayout);
omBucketInfo = OmBucketInfo.getFromProtobuf(bucketInfo, defaultType);
+ LOG.info("Bucket Layout not present for volume/bucket = {}/{}, "
+ + "initialising with default bucket layout" + ": {}", volumeName,
Review comment:
Are this and the below log providing any new information that the audit
log does not already have? It looks like existing practice is to defer these
type of messages to the audit log to avoid INFO clutter in the log for common
requests.
##########
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysDeleteRequest.java
##########
@@ -248,4 +252,16 @@ private static void addDeletedKeys(
auditMap.put(UNDELETED_KEYS_LIST, String.join(",", unDeletedKeys));
}
+ public static OMKeysDeleteRequest getInstance(
+ OzoneManagerProtocolProtos.DeleteKeyArgs keyArgs, OMRequest omRequest,
+ OzoneManager ozoneManager) throws IOException {
+
+ BucketLayout bucketLayout =
+ OzoneManagerUtils.getBucketLayout(keyArgs.getVolumeName(),
+ keyArgs.getBucketName(), ozoneManager, new HashSet<>());
+ Preconditions.checkArgument(!bucketLayout.isFileSystemOptimized(),
Review comment:
Same as above, these should be OMExceptions not precondition checks. You
usually don't want to precondition check something you got off the wire, as
that gives the client the ability to take down your service. Also since we are
reading DB state in a write request this would probably be better suited for
validateAndUpdateCache, as a create/delete/create could possibly change the
bucket layout between here and then.
##########
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java
##########
@@ -276,4 +281,16 @@ public OMClientResponse
validateAndUpdateCache(OzoneManager ozoneManager,
auditMap.put(UNRENAMED_KEYS_MAP, unRenameKeysMap.toString());
return auditMap;
}
+
+ public static OMKeysRenameRequest getInstance(
+ OzoneManagerProtocolProtos.RenameKeysArgs keyArgs, OMRequest omRequest,
+ OzoneManager ozoneManager) throws IOException {
+
+ BucketLayout bucketLayout =
+ OzoneManagerUtils.getBucketLayout(keyArgs.getVolumeName(),
+ keyArgs.getBucketName(), ozoneManager, new HashSet<>());
+ Preconditions.checkArgument(!bucketLayout.isFileSystemOptimized(),
Review comment:
Same comment as keysDeleteRequest.
--
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]
