You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2022/04/01 17:19:25 UTC
[GitHub] [ozone] fapifta opened a new pull request #3262: HDDS-5909. EC: Onboard EC into upgrade framework
fapifta opened a new pull request #3262:
URL: https://github.com/apache/ozone/pull/3262
## What changes were proposed in this pull request?
The change introduces the OM and HDDS LayoutFeature for EC. In order for EC to work, there are no upgrade actions necessary so none are defined, EC does work based on the normal facilities, and solves the erasure coded storage of data solely on the client side. The server side just gets knowledge about the fact that data inside an object is erasure coded with the ReplicationConfig.
The change also introduces a new ClientVersion to mark that the client side supports erasure coded storage, also fixes a few APIDoc typos where ClientVersion remained ClientVersions during the rename of the class.
The change also adds request and response validation to ensure the following backward compatibility and upgrade related questions:
- During an upgrade until the cluster is finalized, there aren't any way to create a key/bucket/container/pipeline with ECReplicationConfig.
- After an upgrade a client is not served with any type of object that has an ECReplicationconfig that the old client would not understand.
The change affects SCM and OM, so that SCM Admin and SCM Container Location requests are also disabled alongside with any requests that the OM ClientProtocol provides.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-5909
## How was this patch tested?
As the logic added is so simple, I did not added new tests for the request/response validation so far, current tests should work without any modification, while in order to add compatibility tests for the client and server side I believe we need a bigger effort to extend what we already have for compatibility alongside with the development related to HDDS-6390
--
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
[GitHub] [ozone] errose28 commented on a change in pull request #3262: HDDS-5909. EC: Onboard EC into upgrade framework
Posted by GitBox <gi...@apache.org>.
errose28 commented on a change in pull request #3262:
URL: https://github.com/apache/ozone/pull/3262#discussion_r840977428
##########
File path: hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/ClientVersion.java
##########
@@ -35,6 +35,9 @@
VERSION_HANDLES_UNKNOWN_DN_PORTS(1,
"Client version that handles the REPLICATION port in DatanodeDetails."),
+ ERASURE_CODING_SUPPORT(2, "This client version has support for Erasure"
+ + "Coding."),
Review comment:
nit. String concat is missing a space.
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/protocol/StorageContainerLocationProtocolServerSideTranslatorPB.java
##########
@@ -172,9 +192,184 @@ public ScmContainerLocationResponse submitRequest(RpcController controller,
scm.getScmHAManager().getRatisServer().triggerNotLeaderException(),
scm.getClientRpcPort(), scm.getScmId());
}
- return dispatcher
+ // After the request interceptor (now validator) framework is extended to
+ // this server interface, this should be removed and solved via new
+ // annotated interceptors.
+ boolean checkResponseForECRepConfig = false;
+ if (request.getCmdType() == GetContainer
+ || request.getCmdType() == ListContainer
+ || request.getCmdType() == GetContainerWithPipeline
+ || request.getCmdType() == GetContainerWithPipelineBatch
+ || request.getCmdType() == GetExistContainerWithPipelinesInBatch
+ || request.getCmdType() == ListPipelines
+ || request.getCmdType() == GetPipeline) {
+ if (request.getVersion() <
+ ClientVersion.ERASURE_CODING_SUPPORT.toProtoValue()) {
Review comment:
Since this inner case will usually be false, it might be slightly better to check it first.
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/protocol/ScmBlockLocationProtocolServerSideTranslatorPB.java
##########
@@ -127,6 +128,16 @@ private SCMBlockLocationResponse processMessage(
try {
switch (request.getCmdType()) {
case AllocateScmBlock:
+ if (scm.getLayoutVersionManager().needsFinalization() &&
+ scm.getLayoutVersionManager().getMetadataLayoutVersion() <
+ HDDSLayoutFeature.ERASURE_CODED_STORAGE_SUPPORT.layoutVersion()
+ ) {
+ if (request.getAllocateScmBlockRequest().hasEcReplicationConfig()) {
+ throw new ServiceException("Cluster is not finalized yet, it is"
+ + " not enabled to create blocks with Erasure Coded"
+ + " replication type.");
Review comment:
I know we don't have them yet, but is the plan to eventually handle this with request hooks when we get those ported to the SCM as well?
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerRequestHandler.java
##########
@@ -581,6 +711,30 @@ private GetFileStatusResponse getOzoneFileStatus(
return rb.build();
}
+ @RequestFeatureValidator(
+ conditions = ValidationCondition.OLDER_CLIENT_REQUESTS,
+ processingPhase = RequestProcessingPhase.POST_PROCESS,
+ requestType = Type.GetFileStatus
+ )
+ public static OMResponse disallowGetFileStatusWithECReplicationConfig(
+ OMRequest req, OMResponse resp, ValidationContext ctx)
+ throws ServiceException {
+ if (!resp.hasGetFileStatusResponse()) {
+ return resp;
+ }
+ if (resp.getGetFileStatusResponse().getStatus().getKeyInfo()
+ .hasEcReplicationConfig()) {
+ throw new ServiceException("Key is a key with"
+ + " Erasure Coded replication, which the client can not understand."
+ + " Please upgrade the client before trying to read the key info"
+ + " for" + req.getGetFileStatusRequest().getKeyArgs().getVolumeName()
Review comment:
nit. Missing space in concatenation.
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/protocol/ScmBlockLocationProtocolServerSideTranslatorPB.java
##########
@@ -127,6 +128,16 @@ private SCMBlockLocationResponse processMessage(
try {
switch (request.getCmdType()) {
case AllocateScmBlock:
+ if (scm.getLayoutVersionManager().needsFinalization() &&
+ scm.getLayoutVersionManager().getMetadataLayoutVersion() <
+ HDDSLayoutFeature.ERASURE_CODED_STORAGE_SUPPORT.layoutVersion()
Review comment:
Could this be simplified to `scm.getLayoutVersionManager().isAllowed(HDDSLayoutFeature.ERASURE_CODED_STORAGE_SUPPORT)`?
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerRequestHandler.java
##########
@@ -597,6 +751,28 @@ private LookupFileResponse lookupFile(LookupFileRequest request,
.build();
}
+ @RequestFeatureValidator(
+ conditions = ValidationCondition.OLDER_CLIENT_REQUESTS,
+ processingPhase = RequestProcessingPhase.POST_PROCESS,
+ requestType = Type.LookupFile
+ )
+ public static OMResponse disallowLookupFileWithECReplicationConfig(
+ OMRequest req, OMResponse resp, ValidationContext ctx)
+ throws ServiceException {
+ if (!resp.hasLookupFileResponse()) {
+ return resp;
+ }
+ if (resp.getLookupFileResponse().getKeyInfo().hasEcReplicationConfig()) {
+ throw new ServiceException("Key is a key with"
+ + " Erasure Coded replication, which the client can not understand."
+ + " Please upgrade the client before trying to read the key info"
+ + " for" + req.getLookupFileRequest().getKeyArgs().getVolumeName()
Review comment:
nit. Missing space in concatenation.
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMDirectoryCreateRequest.java
##########
@@ -407,4 +414,24 @@ public static OMDirectoryCreateRequest getInstance(
}
return new OMDirectoryCreateRequest(omRequest, bucketLayout);
}
+
+ @RequestFeatureValidator(
+ conditions = ValidationCondition.CLUSTER_NEEDS_FINALIZATION,
+ processingPhase = RequestProcessingPhase.PRE_PROCESS,
+ requestType = Type.CreateDirectory
+ )
+ public static OMRequest disallowCreateDirectoryWithECReplicationConfig(
+ OMRequest req, ValidationContext ctx) throws ServiceException {
+ if (ctx.versionManager().getMetadataLayoutVersion()
+ < OMLayoutFeature.ERASURE_CODED_STORAGE_SUPPORT.layoutVersion()) {
+ if (req.getCreateDirectoryRequest().getKeyArgs()
+ .hasEcReplicationConfig()) {
Review comment:
Not directly related to this patch, and I am missing some background on the EC feature here, but what does it mean to create an erasure coded directory?
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerRequestHandler.java
##########
@@ -378,6 +387,30 @@ private InfoBucketResponse infoBucket(InfoBucketRequest request)
return resp.build();
}
+ @RequestFeatureValidator(
+ conditions = ValidationCondition.OLDER_CLIENT_REQUESTS,
Review comment:
This will block the request for all old clients, but a few versions in the future, clients can be old but still have EC support.
I think we could get the client version to this method by having `RequestValidations#validateResponse` (and `RequestValidations#validateRequest` as well) take the context as a parameter which they can forward to the annotated methods, instead of all the annotated methods receiving the same pre-constructed context. `OzoneManagerProtocolServerSideTranslatorPB#submitRequest` should have enough information to construct this context when invoking these methods.
Let me know what you think of this approach, or if you have a better idea.
--
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