errose28 commented on code in PR #6932:
URL: https://github.com/apache/ozone/pull/6932#discussion_r1676430255
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketCreateRequest.java:
##########
@@ -460,9 +460,9 @@ public static OMRequest
handleCreateBucketWithBucketLayoutDuringPreFinalize(
* they do not understand.
*/
@RequestFeatureValidator(
- conditions = ValidationCondition.OLDER_CLIENT_REQUESTS,
processingPhase = RequestProcessingPhase.PRE_PROCESS,
- requestType = Type.CreateBucket
+ requestType = Type.CreateBucket,
+ maxClientVersion = ClientVersion.ERASURE_CODING_SUPPORT
Review Comment:
We should make this so that `ClientVersion.BUCKET_LAYOUT_SUPPORT` is
provided in the annotation for validators that deal with bucket layout. That
would make this an exclusive upper bound on the client version that needs to be
processed instead of an inclusive one.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidatorRegistry.java:
##########
@@ -46,6 +52,9 @@ public class ValidatorRegistry {
private final EnumMap<ValidationCondition,
EnumMap<Type, EnumMap<RequestProcessingPhase, List<Method>>>>
validators = new EnumMap<>(ValidationCondition.class);
+ private final Map<Pair<Type, RequestProcessingPhase>, Pair<List<Method>,
TreeMap<Integer, Integer>>>
+ maxAllowedVersionValidatorMap = new HashMap<>(Type.values().length *
RequestProcessingPhase.values().length,
+ 1.0f);
Review Comment:
If we get rid `ValidationCondition` then we just have 3 keys to identify the
validator(s) that need to run:
- Request type
- Request phase
- Client version
A triple nested map of these keys will be much easier to follow than the
wild data structure created here.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/RequestFeatureValidator.java:
##########
@@ -96,4 +97,15 @@
*/
Type requestType();
+
+ /**
+ * The max client version for which the validator would run. This condition
happens in parallel with the specified
+ * validation conditions. The validator would run either one of the
specified conditions apply or the request
+ * client version is equal to or older than the specified version. The value
defaults to FUTURE_VERSION so the
+ * validator would never run for any of the clients, the validator could
still run based on the specified
+ * conditions though.
+ * @returns the max required client version for which the validator runs for
older clients.
+ */
+ ClientVersion maxClientVersion() default ClientVersion.FUTURE_VERSION;
Review Comment:
If we remove the `conditions` field then this probably shouldn't have a
default value. Technically the request validators can be used for things other
than client versioning but I don't think we have a use case for that right now.
Narrowing the scope of conditions indicates what their intended use is.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/RequestFeatureValidator.java:
##########
@@ -81,7 +82,7 @@
* Runtime conditions in which a validator should run.
* @return a list of conditions when the validator should be applied
*/
- ValidationCondition[] conditions();
+ ValidationCondition[] conditions() default {};
Review Comment:
Do we still need this field?
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketCreateRequest.java:
##########
@@ -460,9 +460,9 @@ public static OMRequest
handleCreateBucketWithBucketLayoutDuringPreFinalize(
* they do not understand.
*/
@RequestFeatureValidator(
- conditions = ValidationCondition.OLDER_CLIENT_REQUESTS,
processingPhase = RequestProcessingPhase.PRE_PROCESS,
- requestType = Type.CreateBucket
+ requestType = Type.CreateBucket,
+ maxClientVersion = ClientVersion.ERASURE_CODING_SUPPORT
Review Comment:
Also we can remove the `if` statements in the body now right?
--
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-unsubscr...@ozone.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@ozone.apache.org
For additional commands, e-mail: issues-h...@ozone.apache.org
