errose28 commented on a change in pull request #3211:
URL: https://github.com/apache/ozone/pull/3211#discussion_r831544358
##########
File path:
hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java
##########
@@ -290,20 +293,26 @@ public XceiverClientFactory getXceiverClientManager() {
return xceiverClientManager;
}
- static boolean validateOmVersion(String expectedVersion,
+ static boolean validateOmVersion(OzoneManagerVersion expectedVersion,
Review comment:
I think `expectedVersion` -> `minimumVersion` makes more sense with the
current changes.
##########
File path:
hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java
##########
@@ -210,16 +210,19 @@ public RpcClient(ConfigurationSource conf, String
omServiceId)
ozoneManagerProtocolClientSideTranslatorPB.setS3AuthCheck(isS3);
if (isS3) {
// S3 Auth works differently and needs OM version to be at 2.0.0
- String omVersion = conf.get(OZONE_OM_CLIENT_PROTOCOL_VERSION_KEY);
- if (!validateOmVersion(omVersion, serviceInfoEx.getServiceInfoList()))
{
+ OzoneManagerVersion minOmVersion = conf.getEnum(
Review comment:
I think this should be run for all clients, not just S3 gateway ones,
since the OM version is given to all clients requesting service info. S3g would
specify the non-default version, so the effect would be the same.
##########
File path:
hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java
##########
@@ -210,16 +210,19 @@ public RpcClient(ConfigurationSource conf, String
omServiceId)
ozoneManagerProtocolClientSideTranslatorPB.setS3AuthCheck(isS3);
if (isS3) {
// S3 Auth works differently and needs OM version to be at 2.0.0
- String omVersion = conf.get(OZONE_OM_CLIENT_PROTOCOL_VERSION_KEY);
- if (!validateOmVersion(omVersion, serviceInfoEx.getServiceInfoList()))
{
+ OzoneManagerVersion minOmVersion = conf.getEnum(
+ OZONE_CLIENT_REQUIRED_OM_VERSION_MIN_KEY,
+ OzoneManagerVersion.DEFAULT_VERSION);
+ if (!validateOmVersion(
+ minOmVersion, serviceInfoEx.getServiceInfoList())) {
if (LOG.isDebugEnabled()) {
for (ServiceInfo s : serviceInfoEx.getServiceInfoList()) {
LOG.debug("Node {} version {}", s.getHostname(),
- s.getProtobuf().getOMProtocolVersion());
+ s.getProtobuf().getOMVersion());
}
}
throw new RuntimeException("OmVersion expected "
- + omVersion
+ + minOmVersion
Review comment:
Let's update the message to indicate the minimum version requirement was
not met. Right now it reads like an exact match is required.
##########
File path:
hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneManagerVersion.java
##########
@@ -30,6 +30,7 @@
*/
public enum OzoneManagerVersion implements ComponentVersion {
DEFAULT_VERSION(0, "Initial version"),
+ NEW_S3G_AUTH_SCHEME(1, "New S3G auth scheme support is present in OM."),
Review comment:
Maybe `S3G_PERSISTENT_CONNECTIONS` would be a more specific name? While
this may be new right now, it won't be that way forever.
##########
File path:
hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConfigKeys.java
##########
@@ -466,11 +466,11 @@
public static final String OZONE_CLIENT_TEST_OFS_BUCKET_LAYOUT_DEFAULT =
"FILE_SYSTEM_OPTIMIZED";
- public static final String OZONE_OM_CLIENT_PROTOCOL_VERSION_KEY =
- "ozone.om.client.protocol.version";
- // The version of the protocol for Client (S3G/OFS) to OM Communication.
- // The protocol starts at 2.0.0 and a null or empty value for older versions.
- public static final String OZONE_OM_CLIENT_PROTOCOL_VERSION = "2.0.0";
+ public static final String OZONE_CLIENT_REQUIRED_OM_VERSION_MIN_KEY =
+ "ozone.client.required.om.version.min";
+
+ public static final String OZONE_CLIENT_REQUIRED_OM_VERSION_MIN_DEFAULT =
+ OzoneManagerVersion.NEW_S3G_AUTH_SCHEME.name();
Review comment:
I think the initial version should be the default, indicating no lower
bound, because all old clients can communicate with all OMs by default. S3
gateway is a special case (most ozone clients do not come from s3g) so it could
set the config as it needs.
--
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]
