You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "jojochuang (via GitHub)" <gi...@apache.org> on 2024/07/31 22:23:15 UTC
[PR] HDDS-11258. [hsync] Add new OM layout version. [ozone]
jojochuang opened a new pull request, #7013:
URL: https://github.com/apache/ozone/pull/7013
## What changes were proposed in this pull request?
HDDS-11258. [hsync] Add new OM layout version.
Please describe your PR in detail:
Add a new OM layout version HBASE_SUPPORT in Ozone 1.5.0 to prevent any misuse of the APIs (hsync, recoverLease, listOpenFiles) until upgrade finalizes.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-11258
## How was this patch tested?
Unit tests covering OM upgrade code path.
--
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
Re: [PR] HDDS-11258. [hsync] Add new OM layout version. [ozone]
Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang commented on PR #7013:
URL: https://github.com/apache/ozone/pull/7013#issuecomment-2264613816
Merged. Thanks @ChenSammi @errose28
--
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
Re: [PR] HDDS-11258. [hsync] Add new OM layout version. [ozone]
Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang commented on code in PR #7013:
URL: https://github.com/apache/ozone/pull/7013#discussion_r1700472401
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestHSync.java:
##########
@@ -226,6 +241,72 @@ public static void teardown() {
}
}
+ private static void preFinalizationChecks() throws IOException {
+ final String rootPath = String.format("%s://%s/",
+ OZONE_OFS_URI_SCHEME, CONF.get(OZONE_OM_ADDRESS_KEY));
+ CONF.set(CommonConfigurationKeysPublic.FS_DEFAULT_NAME_KEY, rootPath);
+
+ final String dir = OZONE_ROOT + bucket.getVolumeName()
+ + OZONE_URI_DELIMITER + bucket.getName();
+
+ final Path file = new Path(dir, "pre-finalization");
+ try (RootedOzoneFileSystem fs = (RootedOzoneFileSystem)FileSystem.get(CONF)) {
+ try (FSDataOutputStream outputStream = fs.create(file, true)) {
+ OMException omException = assertThrows(OMException.class, outputStream::hsync);
+ assertFinalizationExceptionForHsyncLeaseRecovery(omException);
+ }
+ final OzoneManagerProtocol omClient = client.getObjectStore()
+ .getClientProxy().getOzoneManagerClient();
+ OMException omException = assertThrows(OMException.class,
+ () -> omClient.listOpenFiles("", 100, ""));
+ assertFinalizationException(omException);
+
+ omException = assertThrows(OMException.class,
+ () -> fs.recoverLease(file));
+ assertFinalizationException(omException);
+
+ fs.delete(file, false);
+ }
+ }
+
+ private static void assertFinalizationExceptionForHsyncLeaseRecovery(OMException omException) {
+ assertEquals(NOT_SUPPORTED_OPERATION_PRIOR_FINALIZATION,
+ omException.getResult());
+ assertThat(omException.getMessage())
+ .contains("Cluster does not have the HBase support feature finalized yet");
+ }
+
+ private static void assertFinalizationException(OMException omException) {
+ assertEquals(NOT_SUPPORTED_OPERATION_PRIOR_FINALIZATION,
+ omException.getResult());
+ assertThat(omException.getMessage())
+ .contains("cannot be invoked before finalization.");
Review Comment:
I'm inclined to merge this one first. And will rebase HDDS-11260 afterwards.
--
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
Re: [PR] HDDS-11258. [hsync] Add new OM layout version. [ozone]
Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi commented on code in PR #7013:
URL: https://github.com/apache/ozone/pull/7013#discussion_r1699948481
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCommitRequest.java:
##########
@@ -545,22 +545,34 @@ public static OMRequest blockCommitKeyWithBucketLayoutFromOldClient(
return req;
}
+ /**
+ * Validates key commit requests.
+ * We do not want to allow clients to perform hsync or lease recovery requests
+ * until the cluster has finalized the HBase support feature.
+ *
+ * @param req - the request to validate
+ * @param ctx - the validation context
+ * @return the validated request
+ * @throws OMException if the request is invalid
+ */
@RequestFeatureValidator(
conditions = ValidationCondition.CLUSTER_NEEDS_FINALIZATION,
processingPhase = RequestProcessingPhase.PRE_PROCESS,
requestType = Type.CommitKey
)
- public static OMRequest disallowHsync(
+ public static OMRequest disallowHBase(
Review Comment:
Am I missing something? I didn't find where this disallowHsync or disallowHBase is called.
--
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
Re: [PR] HDDS-11258. [hsync] Add new OM layout version. [ozone]
Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi commented on code in PR #7013:
URL: https://github.com/apache/ozone/pull/7013#discussion_r1699939385
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/upgrade/OMLayoutFeature.java:
##########
@@ -44,7 +44,8 @@ public enum OMLayoutFeature implements LayoutFeature {
FILESYSTEM_SNAPSHOT(5, "Ozone version supporting snapshot"),
- QUOTA(6, "Ozone quota re-calculate");
+ QUOTA(6, "Ozone quota re-calculate"),
+ HBASE_SUPPORT(7, "Full support of hsync for HBase");
Review Comment:
Can you mention the lease recovery too in the description?
--
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
Re: [PR] HDDS-11258. [hsync] Add new OM layout version. [ozone]
Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi commented on code in PR #7013:
URL: https://github.com/apache/ozone/pull/7013#discussion_r1699948481
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCommitRequest.java:
##########
@@ -545,22 +545,34 @@ public static OMRequest blockCommitKeyWithBucketLayoutFromOldClient(
return req;
}
+ /**
+ * Validates key commit requests.
+ * We do not want to allow clients to perform hsync or lease recovery requests
+ * until the cluster has finalized the HBase support feature.
+ *
+ * @param req - the request to validate
+ * @param ctx - the validation context
+ * @return the validated request
+ * @throws OMException if the request is invalid
+ */
@RequestFeatureValidator(
conditions = ValidationCondition.CLUSTER_NEEDS_FINALIZATION,
processingPhase = RequestProcessingPhase.PRE_PROCESS,
requestType = Type.CommitKey
)
- public static OMRequest disallowHsync(
+ public static OMRequest disallowHBase(
Review Comment:
Am I missing something? I didn't find where this disallowHsync or disallowHBase is called.
--
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
Re: [PR] HDDS-11258. [hsync] Add new OM layout version. [ozone]
Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang merged PR #7013:
URL: https://github.com/apache/ozone/pull/7013
--
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
Re: [PR] HDDS-11258. [hsync] Add new OM layout version. [ozone]
Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang commented on code in PR #7013:
URL: https://github.com/apache/ozone/pull/7013#discussion_r1700470772
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMRecoverLeaseRequest.java:
##########
@@ -101,6 +103,7 @@ public OMRecoverLeaseRequest(OMRequest omRequest) {
}
@Override
+ @DisallowedUntilLayoutVersion(HBASE_SUPPORT)
Review Comment:
We never advertise hsync and lease recovery APIs and hsync is disabled by default in Ozone 1.4.0, so it shouldn't cause a regression.
These annotations are used to prevent undefined behavior if a downgrade is necessary.
--
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
Re: [PR] HDDS-11258. [hsync] Add new OM layout version. [ozone]
Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang commented on PR #7013:
URL: https://github.com/apache/ozone/pull/7013#issuecomment-2261657168
The PR is ready for review. The test code is adopted from TestOmSnapshot.
--
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
Re: [PR] HDDS-11258. [hsync] Add new OM layout version. [ozone]
Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi commented on code in PR #7013:
URL: https://github.com/apache/ozone/pull/7013#discussion_r1699959172
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestHSync.java:
##########
@@ -226,6 +241,72 @@ public static void teardown() {
}
}
+ private static void preFinalizationChecks() throws IOException {
+ final String rootPath = String.format("%s://%s/",
+ OZONE_OFS_URI_SCHEME, CONF.get(OZONE_OM_ADDRESS_KEY));
+ CONF.set(CommonConfigurationKeysPublic.FS_DEFAULT_NAME_KEY, rootPath);
+
+ final String dir = OZONE_ROOT + bucket.getVolumeName()
+ + OZONE_URI_DELIMITER + bucket.getName();
+
+ final Path file = new Path(dir, "pre-finalization");
+ try (RootedOzoneFileSystem fs = (RootedOzoneFileSystem)FileSystem.get(CONF)) {
+ try (FSDataOutputStream outputStream = fs.create(file, true)) {
+ OMException omException = assertThrows(OMException.class, outputStream::hsync);
+ assertFinalizationExceptionForHsyncLeaseRecovery(omException);
+ }
+ final OzoneManagerProtocol omClient = client.getObjectStore()
+ .getClientProxy().getOzoneManagerClient();
+ OMException omException = assertThrows(OMException.class,
+ () -> omClient.listOpenFiles("", 100, ""));
+ assertFinalizationException(omException);
+
+ omException = assertThrows(OMException.class,
+ () -> fs.recoverLease(file));
+ assertFinalizationException(omException);
+
+ fs.delete(file, false);
+ }
+ }
+
+ private static void assertFinalizationExceptionForHsyncLeaseRecovery(OMException omException) {
+ assertEquals(NOT_SUPPORTED_OPERATION_PRIOR_FINALIZATION,
+ omException.getResult());
+ assertThat(omException.getMessage())
+ .contains("Cluster does not have the HBase support feature finalized yet");
+ }
+
+ private static void assertFinalizationException(OMException omException) {
+ assertEquals(NOT_SUPPORTED_OPERATION_PRIOR_FINALIZATION,
+ omException.getResult());
+ assertThat(omException.getMessage())
+ .contains("cannot be invoked before finalization.");
Review Comment:
Do these checks need be changed if " HDDS-11260. [hsync] Add Ozone Manager protocol version " got merged first?
--
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
Re: [PR] HDDS-11258. [hsync] Add new OM layout version. [ozone]
Posted by "errose28 (via GitHub)" <gi...@apache.org>.
errose28 commented on code in PR #7013:
URL: https://github.com/apache/ozone/pull/7013#discussion_r1700421630
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCommitRequest.java:
##########
@@ -545,22 +545,34 @@ public static OMRequest blockCommitKeyWithBucketLayoutFromOldClient(
return req;
}
+ /**
+ * Validates key commit requests.
+ * We do not want to allow clients to perform hsync or lease recovery requests
+ * until the cluster has finalized the HBase support feature.
+ *
+ * @param req - the request to validate
+ * @param ctx - the validation context
+ * @return the validated request
+ * @throws OMException if the request is invalid
+ */
@RequestFeatureValidator(
conditions = ValidationCondition.CLUSTER_NEEDS_FINALIZATION,
processingPhase = RequestProcessingPhase.PRE_PROCESS,
requestType = Type.CommitKey
)
- public static OMRequest disallowHsync(
+ public static OMRequest disallowHBase(
OMRequest req, ValidationContext ctx) throws OMException {
if (!ctx.versionManager()
- .isAllowed(OMLayoutFeature.HSYNC)) {
+ .isAllowed(OMLayoutFeature.HBASE_SUPPORT)) {
Review Comment:
Hsync has also already been finalized on master. Can we leave the original validator and have this one just for the recovery flag?
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMRecoverLeaseRequest.java:
##########
@@ -101,6 +103,7 @@ public OMRecoverLeaseRequest(OMRequest omRequest) {
}
@Override
+ @DisallowedUntilLayoutVersion(HBASE_SUPPORT)
Review Comment:
`OMRecoverLeaseRequest` is already on master. Disabling it here would actually be a step backwards. What are we trying to prevent in this case?
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/upgrade/OMLayoutFeature.java:
##########
@@ -44,7 +44,8 @@ public enum OMLayoutFeature implements LayoutFeature {
FILESYSTEM_SNAPSHOT(5, "Ozone version supporting snapshot"),
- QUOTA(6, "Ozone quota re-calculate");
+ QUOTA(6, "Ozone quota re-calculate"),
+ HBASE_SUPPORT(7, "Full support of hsync for HBase");
Review Comment:
+1 a description of what actually changed in the metadata would be helpful
--
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