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/19 09:17:21 UTC
[GitHub] [ozone] kaijchen opened a new pull request, #3319: HDDS-4123. Integrate OM Open Key Cleanup Service Into Existing Code
kaijchen opened a new pull request, #3319:
URL: https://github.com/apache/ozone/pull/3319
## What changes were proposed in this pull request?
This pull request completes the open key cleanup service outlined in the parent Jira HDDS-4120. It implements the `OpenKeyCleanupService` class, and starts and stops the service in `KeyManagerImpl`. The following configurations have been defined to specify the service's behavior:
1. ozone.open.key.cleanup.service.interval (default value 24 hours)
2. ozone.open.key.expire.threshold (default value 1 week)
3. ozone.open.key.cleanup.limit.per.task (default value 1000 keys)
See `ozone-defaults.xml` for their corresponding descriptions.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-4123
## How was this patch tested?
Integration test
--
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] kaijchen commented on pull request #3319: HDDS-4123. Integrate OM Open Key Cleanup Service Into Existing Code
Posted by GitBox <gi...@apache.org>.
kaijchen commented on PR #3319:
URL: https://github.com/apache/ozone/pull/3319#issuecomment-1148235576
100x TestOpenKeyCleanupService: https://github.com/kaijchen/ozone/actions/runs/2452367006
--
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] kaijchen commented on a diff in pull request #3319: HDDS-4123. Integrate OM Open Key Cleanup Service Into Existing Code
Posted by GitBox <gi...@apache.org>.
kaijchen commented on code in PR #3319:
URL: https://github.com/apache/ozone/pull/3319#discussion_r891119059
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OpenKeyCleanupService.java:
##########
@@ -73,18 +165,72 @@ public int getPriority() {
@Override
public BackgroundTaskResult call() throws Exception {
- // This method is currently never used. It will be implemented in
- // HDDS-4122, and integrated into the rest of the code base in HDDS-4123.
+ if (!shouldRun()) {
+ return BackgroundTaskResult.EmptyTaskResult.newResult();
+ }
+
+ runCount.incrementAndGet();
+ long startTime = Time.monotonicNow();
+ List<OpenKeyBucket> openKeyBuckets = null;
try {
- // The new API for deleting expired open keys in OM HA will differ
- // significantly from the old implementation.
- // The old implementation has been removed so the code compiles.
- keyManager.getExpiredOpenKeys(Duration.ZERO, 0, BucketLayout.DEFAULT);
+ openKeyBuckets = keyManager.getExpiredOpenKeys(expireThreshold,
+ cleanupLimitPerTask, bucketLayout);
} catch (IOException e) {
- LOG.error("Unable to get hanging open keys, retry in"
- + " next interval", e);
+ LOG.error("Unable to get hanging open keys, retry in next interval", e);
+ }
+
+ if (openKeyBuckets != null && !openKeyBuckets.isEmpty()) {
+ int numOpenKeys = openKeyBuckets.stream()
+ .mapToInt(OpenKeyBucket::getKeysCount).sum();
+
+ OMRequest omRequest = createRequest(openKeyBuckets);
+ submitRequest(omRequest);
+
+ LOG.debug("Number of expired keys submitted for deletion: {}, elapsed"
+ + " time: {}ms", numOpenKeys, Time.monotonicNow() - startTime);
+ submittedOpenKeyCount.addAndGet(numOpenKeys);
}
return BackgroundTaskResult.EmptyTaskResult.newResult();
}
+
+ private OMRequest createRequest(List<OpenKeyBucket> openKeyBuckets) {
+ DeleteOpenKeysRequest request =
+ DeleteOpenKeysRequest.newBuilder()
+ .addAllOpenKeysPerBucket(openKeyBuckets)
+ .setBucketLayout(bucketLayout.toProto())
+ .build();
+
+ OMRequest omRequest = OMRequest.newBuilder()
+ .setCmdType(Type.DeleteOpenKeys)
+ .setDeleteOpenKeysRequest(request)
+ .setClientId(clientId.toString())
+ .build();
+
+ return omRequest;
+ }
+
+ private void submitRequest(OMRequest omRequest) {
+ try {
+ if (isRatisEnabled()) {
+ OzoneManagerRatisServer server = ozoneManager.getOmRatisServer();
+
+ RaftClientRequest raftClientRequest = RaftClientRequest.newBuilder()
+ .setClientId(clientId)
+ .setServerId(server.getRaftPeerId())
+ .setGroupId(server.getRaftGroupId())
+ .setCallId(runCount.get())
+ .setMessage(Message.valueOf(
+ OMRatisHelper.convertRequestToByteString(omRequest)))
+ .setType(RaftClientRequest.writeRequestType())
+ .build();
+
+ server.submitRequest(omRequest, raftClientRequest);
Review Comment:
I think metrics is already included in OMOpenKeysDeleteRequest.
https://github.com/apache/ozone/blob/377be9644fb2e7a1b6b6413a82ac15e3dc4c48d3/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMOpenKeysDeleteRequest.java#L67
https://github.com/apache/ozone/blob/377be9644fb2e7a1b6b6413a82ac15e3dc4c48d3/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMOpenKeysDeleteRequest.java#L81
https://github.com/apache/ozone/blob/377be9644fb2e7a1b6b6413a82ac15e3dc4c48d3/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMOpenKeysDeleteRequest.java#L130
https://github.com/apache/ozone/blob/377be9644fb2e7a1b6b6413a82ac15e3dc4c48d3/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMOpenKeysDeleteRequest.java#L171
--
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] captainzmc commented on pull request #3319: HDDS-4123. Integrate OM Open Key Cleanup Service Into Existing Code
Posted by GitBox <gi...@apache.org>.
captainzmc commented on PR #3319:
URL: https://github.com/apache/ozone/pull/3319#issuecomment-1158706764
Hi @kaijchen , did you tested this patch in local cluster? For example, keep writing failed keys. The clean up service keep deleting in the background. Run a few days for stability test to see if there's anything problem.
--
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] kaijchen commented on a diff in pull request #3319: HDDS-4123. Integrate OM Open Key Cleanup Service Into Existing Code
Posted by GitBox <gi...@apache.org>.
kaijchen commented on code in PR #3319:
URL: https://github.com/apache/ozone/pull/3319#discussion_r890099921
##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOpenKeyCleanupService.java:
##########
@@ -0,0 +1,213 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+package org.apache.hadoop.ozone.om;
+
+import org.apache.commons.lang3.RandomUtils;
+import org.apache.hadoop.hdds.client.StandaloneReplicationConfig;
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
+import org.apache.hadoop.hdds.scm.container.common.helpers.ExcludeList;
+import org.apache.hadoop.hdds.server.ServerUtils;
+import org.apache.hadoop.hdds.utils.db.DBConfigFromFile;
+import org.apache.hadoop.ozone.om.helpers.BucketLayout;
+import org.apache.hadoop.ozone.om.helpers.OmBucketInfo;
+import org.apache.hadoop.ozone.om.helpers.OmKeyArgs;
+import org.apache.hadoop.ozone.om.helpers.OmVolumeArgs;
+import org.apache.hadoop.ozone.om.helpers.OpenKeySession;
+import org.apache.hadoop.ozone.om.protocol.OzoneManagerProtocol;
+import org.apache.hadoop.ozone.om.request.OMRequestTestUtils;
+import org.apache.ozone.test.GenericTestUtils;
+import org.apache.ratis.util.ExitUtils;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Timeout;
+import org.junit.jupiter.api.io.TempDir;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.CsvSource;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.nio.file.Path;
+import java.time.Duration;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.UUID;
+import java.util.concurrent.TimeUnit;
+
+import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_OPEN_KEY_CLEANUP_SERVICE_INTERVAL;
+import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_OPEN_KEY_EXPIRE_THRESHOLD;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/**
+ * Test Key Deleting Service.
+ * <p>
+ * This test does the following things.
+ * <p>
+ * 1. Creates a bunch of keys. 2. Then executes delete key directly using
+ * Metadata Manager. 3. Waits for a while for the KeyDeleting Service to pick up
+ * and call into SCM. 4. Confirms that calls have been successful.
+ */
+public class TestOpenKeyCleanupService {
+ private OzoneManagerProtocol writeClient;
+ private OzoneManager om;
+ private static final Logger LOG =
+ LoggerFactory.getLogger(TestOpenKeyCleanupService.class);
+
+ private static final Duration SERVICE_INTERVAL = Duration.ofMillis(100);
+ private static final Duration EXPIRE_THRESHOLD = Duration.ofMillis(200);
+ private KeyManager keyManager;
+ private OMMetadataManager omMetadataManager;
+
+ @BeforeAll
+ public static void setup() {
+ ExitUtils.disableSystemExit();
+ }
+
+ @BeforeEach
+ public void createConfAndInitValues(@TempDir Path tempDir) throws Exception {
+ OzoneConfiguration conf = new OzoneConfiguration();
+ System.setProperty(DBConfigFromFile.CONFIG_DIR, "/");
+ ServerUtils.setOzoneMetaDirPath(conf, tempDir.toString());
+ conf.setTimeDuration(OZONE_OM_OPEN_KEY_CLEANUP_SERVICE_INTERVAL,
+ SERVICE_INTERVAL.toMillis(), TimeUnit.MILLISECONDS);
+ conf.setTimeDuration(OZONE_OM_OPEN_KEY_EXPIRE_THRESHOLD,
+ EXPIRE_THRESHOLD.toMillis(), TimeUnit.MILLISECONDS);
+ conf.setQuietMode(false);
+ OmTestManagers omTestManagers = new OmTestManagers(conf);
+ keyManager = omTestManagers.getKeyManager();
+ omMetadataManager = omTestManagers.getMetadataManager();
+ writeClient = omTestManagers.getWriteClient();
+ om = omTestManagers.getOzoneManager();
+ }
+
+ @AfterEach
+ public void cleanup() throws Exception {
+ om.stop();
+ }
+
+ /**
+ * In this test, we create a bunch of keys and delete them. Then we start the
+ * KeyDeletingService and pass a SCMClient which does not fail. We make sure
+ * that all the keys that we deleted is picked up and deleted by
+ * OzoneManager.
+ *
+ * @throws IOException - on Failure.
+ */
+
Review Comment:
```suggestion
```
--
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] kaijchen commented on a diff in pull request #3319: HDDS-4123. Integrate OM Open Key Cleanup Service Into Existing Code
Posted by GitBox <gi...@apache.org>.
kaijchen commented on code in PR #3319:
URL: https://github.com/apache/ozone/pull/3319#discussion_r891183705
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestSecureOzoneManager.java:
##########
@@ -85,8 +83,6 @@ public void init() throws Exception {
omId = UUID.randomUUID().toString();
conf.setBoolean(OZONE_ACL_ENABLED, true);
conf.setBoolean(OZONE_SECURITY_ENABLED_KEY, true);
- conf.setTimeDuration(OZONE_OM_OPEN_KEY_EXPIRE_THRESHOLD,
- 2, TimeUnit.SECONDS);
Review Comment:
There is no problem in TestSecureOzoneManager with or without HDDS-4123
https://github.com/kaijchen/ozone/actions/runs/2454403397
https://github.com/kaijchen/ozone/actions/runs/2454495538
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestSecureOzoneManager.java:
##########
@@ -85,8 +83,6 @@ public void init() throws Exception {
omId = UUID.randomUUID().toString();
conf.setBoolean(OZONE_ACL_ENABLED, true);
conf.setBoolean(OZONE_SECURITY_ENABLED_KEY, true);
- conf.setTimeDuration(OZONE_OM_OPEN_KEY_EXPIRE_THRESHOLD,
- 2, TimeUnit.SECONDS);
Review Comment:
I don't think so. IIRC, this test is flaky before.
--
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] kaijchen commented on a diff in pull request #3319: HDDS-4123. Integrate OM Open Key Cleanup Service Into Existing Code
Posted by GitBox <gi...@apache.org>.
kaijchen commented on code in PR #3319:
URL: https://github.com/apache/ozone/pull/3319#discussion_r891106865
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestSecureOzoneManager.java:
##########
@@ -85,8 +83,6 @@ public void init() throws Exception {
omId = UUID.randomUUID().toString();
conf.setBoolean(OZONE_ACL_ENABLED, true);
conf.setBoolean(OZONE_SECURITY_ENABLED_KEY, true);
- conf.setTimeDuration(OZONE_OM_OPEN_KEY_EXPIRE_THRESHOLD,
- 2, TimeUnit.SECONDS);
Review Comment:
I don't think so. IIRC, this test is flaky before.
--
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] kaijchen commented on pull request #3319: HDDS-4123. Integrate OM Open Key Cleanup Service Into Existing Code
Posted by GitBox <gi...@apache.org>.
kaijchen commented on PR #3319:
URL: https://github.com/apache/ozone/pull/3319#issuecomment-1158769499
Yes, I have tested it in a local cluster. From the log we can see open key cleanup service works as expected.
The loglevel was changed to `INFO` for testing purpose.
```
2022-06-17 15:52:14,891 [OpenKeyCleanupService#0] INFO org.apache.hadoop.ozone.om.OpenKeyCleanupService: running open key cleanup task
2022-06-17 15:52:14,892 [OpenKeyCleanupService#0] INFO org.apache.hadoop.ozone.om.OpenKeyCleanupService: running open key cleanup task
2022-06-17 16:02:14,891 [OpenKeyCleanupService#0] INFO org.apache.hadoop.ozone.om.OpenKeyCleanupService: running open key cleanup task
2022-06-17 16:02:14,922 [OpenKeyCleanupService#0] INFO org.apache.hadoop.ozone.om.OpenKeyCleanupService: Number of expired keys submitted for deletion: 20, elapsed time: 31ms
2022-06-17 16:02:14,922 [OpenKeyCleanupService#0] INFO org.apache.hadoop.ozone.om.OpenKeyCleanupService: running open key cleanup task
2022-06-17 16:12:14,891 [OpenKeyCleanupService#0] INFO org.apache.hadoop.ozone.om.OpenKeyCleanupService: running open key cleanup task
2022-06-17 16:12:14,891 [OpenKeyCleanupService#0] INFO org.apache.hadoop.ozone.om.OpenKeyCleanupService: running open key cleanup task
```
--
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] captainzmc merged pull request #3319: HDDS-4123. Integrate OM Open Key Cleanup Service Into Existing Code
Posted by GitBox <gi...@apache.org>.
captainzmc merged PR #3319:
URL: https://github.com/apache/ozone/pull/3319
--
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] kaijchen commented on pull request #3319: HDDS-4123. Integrate OM Open Key Cleanup Service Into Existing Code
Posted by GitBox <gi...@apache.org>.
kaijchen commented on PR #3319:
URL: https://github.com/apache/ozone/pull/3319#issuecomment-1159610554
Limiting number of expired keys is working as expected.
```
2022-06-18 22:42:14,910 [OpenKeyCleanupService#0] INFO org.apache.hadoop.ozone.om.OpenKeyCleanupService: running open key cleanup task
2022-06-18 22:42:14,976 [OpenKeyCleanupService#0] INFO org.apache.hadoop.ozone.om.OpenKeyCleanupService: running open key cleanup task
2022-06-18 22:52:14,910 [OpenKeyCleanupService#0] INFO org.apache.hadoop.ozone.om.OpenKeyCleanupService: running open key cleanup task
2022-06-18 22:52:14,990 [OpenKeyCleanupService#0] INFO org.apache.hadoop.ozone.om.OpenKeyCleanupService: Number of expired keys submitted for deletion: 1000, elapsed time: 80ms
2022-06-18 22:52:14,990 [OpenKeyCleanupService#0] INFO org.apache.hadoop.ozone.om.OpenKeyCleanupService: running open key cleanup task
2022-06-18 23:02:14,910 [OpenKeyCleanupService#0] INFO org.apache.hadoop.ozone.om.OpenKeyCleanupService: running open key cleanup task
2022-06-18 23:02:14,995 [OpenKeyCleanupService#0] INFO org.apache.hadoop.ozone.om.OpenKeyCleanupService: Number of expired keys submitted for deletion: 1000, elapsed time: 86ms
2022-06-18 23:02:14,996 [OpenKeyCleanupService#0] INFO org.apache.hadoop.ozone.om.OpenKeyCleanupService: running open key cleanup task
2022-06-18 23:12:14,910 [OpenKeyCleanupService#0] INFO org.apache.hadoop.ozone.om.OpenKeyCleanupService: running open key cleanup task
2022-06-18 23:12:14,979 [OpenKeyCleanupService#0] INFO org.apache.hadoop.ozone.om.OpenKeyCleanupService: Number of expired keys submitted for deletion: 108, elapsed time: 69ms
2022-06-18 23:12:14,979 [OpenKeyCleanupService#0] INFO org.apache.hadoop.ozone.om.OpenKeyCleanupService: running open key cleanup task
2022-06-18 23:22:14,910 [OpenKeyCleanupService#0] INFO org.apache.hadoop.ozone.om.OpenKeyCleanupService: running open key cleanup task
2022-06-18 23:22:14,965 [OpenKeyCleanupService#0] INFO org.apache.hadoop.ozone.om.OpenKeyCleanupService: running open key cleanup task
2022-06-18 23:32:14,910 [OpenKeyCleanupService#0] INFO org.apache.hadoop.ozone.om.OpenKeyCleanupService: running open key cleanup task
```
--
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] captainzmc commented on pull request #3319: HDDS-4123. Integrate OM Open Key Cleanup Service Into Existing Code
Posted by GitBox <gi...@apache.org>.
captainzmc commented on PR #3319:
URL: https://github.com/apache/ozone/pull/3319#issuecomment-1148544612
Hi @errose28, this task is the last part of HDDS-4120, through which open key can be cleaned normally. Could you also help review this?
--
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] kaijchen commented on pull request #3319: HDDS-4123. Integrate OM Open Key Cleanup Service Into Existing Code
Posted by GitBox <gi...@apache.org>.
kaijchen commented on PR #3319:
URL: https://github.com/apache/ozone/pull/3319#issuecomment-1159896974
Thanks @captainzmc for reviewing and merging this.
--
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] kaijchen commented on a diff in pull request #3319: HDDS-4123. Integrate OM Open Key Cleanup Service Into Existing Code
Posted by GitBox <gi...@apache.org>.
kaijchen commented on code in PR #3319:
URL: https://github.com/apache/ozone/pull/3319#discussion_r890099921
##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOpenKeyCleanupService.java:
##########
@@ -0,0 +1,213 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+package org.apache.hadoop.ozone.om;
+
+import org.apache.commons.lang3.RandomUtils;
+import org.apache.hadoop.hdds.client.StandaloneReplicationConfig;
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
+import org.apache.hadoop.hdds.scm.container.common.helpers.ExcludeList;
+import org.apache.hadoop.hdds.server.ServerUtils;
+import org.apache.hadoop.hdds.utils.db.DBConfigFromFile;
+import org.apache.hadoop.ozone.om.helpers.BucketLayout;
+import org.apache.hadoop.ozone.om.helpers.OmBucketInfo;
+import org.apache.hadoop.ozone.om.helpers.OmKeyArgs;
+import org.apache.hadoop.ozone.om.helpers.OmVolumeArgs;
+import org.apache.hadoop.ozone.om.helpers.OpenKeySession;
+import org.apache.hadoop.ozone.om.protocol.OzoneManagerProtocol;
+import org.apache.hadoop.ozone.om.request.OMRequestTestUtils;
+import org.apache.ozone.test.GenericTestUtils;
+import org.apache.ratis.util.ExitUtils;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Timeout;
+import org.junit.jupiter.api.io.TempDir;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.CsvSource;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.nio.file.Path;
+import java.time.Duration;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.UUID;
+import java.util.concurrent.TimeUnit;
+
+import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_OPEN_KEY_CLEANUP_SERVICE_INTERVAL;
+import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_OPEN_KEY_EXPIRE_THRESHOLD;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/**
+ * Test Key Deleting Service.
+ * <p>
+ * This test does the following things.
+ * <p>
+ * 1. Creates a bunch of keys. 2. Then executes delete key directly using
+ * Metadata Manager. 3. Waits for a while for the KeyDeleting Service to pick up
+ * and call into SCM. 4. Confirms that calls have been successful.
+ */
+public class TestOpenKeyCleanupService {
+ private OzoneManagerProtocol writeClient;
+ private OzoneManager om;
+ private static final Logger LOG =
+ LoggerFactory.getLogger(TestOpenKeyCleanupService.class);
+
+ private static final Duration SERVICE_INTERVAL = Duration.ofMillis(100);
+ private static final Duration EXPIRE_THRESHOLD = Duration.ofMillis(200);
+ private KeyManager keyManager;
+ private OMMetadataManager omMetadataManager;
+
+ @BeforeAll
+ public static void setup() {
+ ExitUtils.disableSystemExit();
+ }
+
+ @BeforeEach
+ public void createConfAndInitValues(@TempDir Path tempDir) throws Exception {
+ OzoneConfiguration conf = new OzoneConfiguration();
+ System.setProperty(DBConfigFromFile.CONFIG_DIR, "/");
+ ServerUtils.setOzoneMetaDirPath(conf, tempDir.toString());
+ conf.setTimeDuration(OZONE_OM_OPEN_KEY_CLEANUP_SERVICE_INTERVAL,
+ SERVICE_INTERVAL.toMillis(), TimeUnit.MILLISECONDS);
+ conf.setTimeDuration(OZONE_OM_OPEN_KEY_EXPIRE_THRESHOLD,
+ EXPIRE_THRESHOLD.toMillis(), TimeUnit.MILLISECONDS);
+ conf.setQuietMode(false);
+ OmTestManagers omTestManagers = new OmTestManagers(conf);
+ keyManager = omTestManagers.getKeyManager();
+ omMetadataManager = omTestManagers.getMetadataManager();
+ writeClient = omTestManagers.getWriteClient();
+ om = omTestManagers.getOzoneManager();
+ }
+
+ @AfterEach
+ public void cleanup() throws Exception {
+ om.stop();
+ }
+
+ /**
+ * In this test, we create a bunch of keys and delete them. Then we start the
+ * KeyDeletingService and pass a SCMClient which does not fail. We make sure
+ * that all the keys that we deleted is picked up and deleted by
+ * OzoneManager.
+ *
+ * @throws IOException - on Failure.
+ */
+
Review Comment:
```suggestion
```
--
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] kaijchen commented on a diff in pull request #3319: HDDS-4123. Integrate OM Open Key Cleanup Service Into Existing Code
Posted by GitBox <gi...@apache.org>.
kaijchen commented on code in PR #3319:
URL: https://github.com/apache/ozone/pull/3319#discussion_r891119059
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OpenKeyCleanupService.java:
##########
@@ -73,18 +165,72 @@ public int getPriority() {
@Override
public BackgroundTaskResult call() throws Exception {
- // This method is currently never used. It will be implemented in
- // HDDS-4122, and integrated into the rest of the code base in HDDS-4123.
+ if (!shouldRun()) {
+ return BackgroundTaskResult.EmptyTaskResult.newResult();
+ }
+
+ runCount.incrementAndGet();
+ long startTime = Time.monotonicNow();
+ List<OpenKeyBucket> openKeyBuckets = null;
try {
- // The new API for deleting expired open keys in OM HA will differ
- // significantly from the old implementation.
- // The old implementation has been removed so the code compiles.
- keyManager.getExpiredOpenKeys(Duration.ZERO, 0, BucketLayout.DEFAULT);
+ openKeyBuckets = keyManager.getExpiredOpenKeys(expireThreshold,
+ cleanupLimitPerTask, bucketLayout);
} catch (IOException e) {
- LOG.error("Unable to get hanging open keys, retry in"
- + " next interval", e);
+ LOG.error("Unable to get hanging open keys, retry in next interval", e);
+ }
+
+ if (openKeyBuckets != null && !openKeyBuckets.isEmpty()) {
+ int numOpenKeys = openKeyBuckets.stream()
+ .mapToInt(OpenKeyBucket::getKeysCount).sum();
+
+ OMRequest omRequest = createRequest(openKeyBuckets);
+ submitRequest(omRequest);
+
+ LOG.debug("Number of expired keys submitted for deletion: {}, elapsed"
+ + " time: {}ms", numOpenKeys, Time.monotonicNow() - startTime);
+ submittedOpenKeyCount.addAndGet(numOpenKeys);
}
return BackgroundTaskResult.EmptyTaskResult.newResult();
}
+
+ private OMRequest createRequest(List<OpenKeyBucket> openKeyBuckets) {
+ DeleteOpenKeysRequest request =
+ DeleteOpenKeysRequest.newBuilder()
+ .addAllOpenKeysPerBucket(openKeyBuckets)
+ .setBucketLayout(bucketLayout.toProto())
+ .build();
+
+ OMRequest omRequest = OMRequest.newBuilder()
+ .setCmdType(Type.DeleteOpenKeys)
+ .setDeleteOpenKeysRequest(request)
+ .setClientId(clientId.toString())
+ .build();
+
+ return omRequest;
+ }
+
+ private void submitRequest(OMRequest omRequest) {
+ try {
+ if (isRatisEnabled()) {
+ OzoneManagerRatisServer server = ozoneManager.getOmRatisServer();
+
+ RaftClientRequest raftClientRequest = RaftClientRequest.newBuilder()
+ .setClientId(clientId)
+ .setServerId(server.getRaftPeerId())
+ .setGroupId(server.getRaftGroupId())
+ .setCallId(runCount.get())
+ .setMessage(Message.valueOf(
+ OMRatisHelper.convertRequestToByteString(omRequest)))
+ .setType(RaftClientRequest.writeRequestType())
+ .build();
+
+ server.submitRequest(omRequest, raftClientRequest);
Review Comment:
I think metrics is already included in OMOpenKeysDeleteRequest.
https://github.com/apache/ozone/blob/377be9644fb2e7a1b6b6413a82ac15e3dc4c48d3/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMOpenKeysDeleteRequest.java#L67
https://github.com/apache/ozone/blob/377be9644fb2e7a1b6b6413a82ac15e3dc4c48d3/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMOpenKeysDeleteRequest.java#L81
https://github.com/apache/ozone/blob/377be9644fb2e7a1b6b6413a82ac15e3dc4c48d3/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMOpenKeysDeleteRequest.java#L130
--
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] kaijchen commented on pull request #3319: HDDS-4123. Integrate OM Open Key Cleanup Service Into Existing Code
Posted by GitBox <gi...@apache.org>.
kaijchen commented on PR #3319:
URL: https://github.com/apache/ozone/pull/3319#issuecomment-1148566031
Thanks @captainzmc for the review. I think there is one more task left after this, HDDS-6769.
--
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] kaijchen commented on pull request #3319: HDDS-4123. Integrate OM Open Key Cleanup Service Into Existing Code
Posted by GitBox <gi...@apache.org>.
kaijchen commented on PR #3319:
URL: https://github.com/apache/ozone/pull/3319#issuecomment-1147391574
@errose28 @captainzmc PTAL.
--
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] captainzmc commented on a diff in pull request #3319: HDDS-4123. Integrate OM Open Key Cleanup Service Into Existing Code
Posted by GitBox <gi...@apache.org>.
captainzmc commented on code in PR #3319:
URL: https://github.com/apache/ozone/pull/3319#discussion_r891089996
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestSecureOzoneManager.java:
##########
@@ -85,8 +83,6 @@ public void init() throws Exception {
omId = UUID.randomUUID().toString();
conf.setBoolean(OZONE_ACL_ENABLED, true);
conf.setBoolean(OZONE_SECURITY_ENABLED_KEY, true);
- conf.setTimeDuration(OZONE_OM_OPEN_KEY_EXPIRE_THRESHOLD,
- 2, TimeUnit.SECONDS);
Review Comment:
Does the OpenKey Clean Update Service cause this CI to fail?
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OpenKeyCleanupService.java:
##########
@@ -73,18 +165,72 @@ public int getPriority() {
@Override
public BackgroundTaskResult call() throws Exception {
- // This method is currently never used. It will be implemented in
- // HDDS-4122, and integrated into the rest of the code base in HDDS-4123.
+ if (!shouldRun()) {
+ return BackgroundTaskResult.EmptyTaskResult.newResult();
+ }
+
+ runCount.incrementAndGet();
+ long startTime = Time.monotonicNow();
+ List<OpenKeyBucket> openKeyBuckets = null;
try {
- // The new API for deleting expired open keys in OM HA will differ
- // significantly from the old implementation.
- // The old implementation has been removed so the code compiles.
- keyManager.getExpiredOpenKeys(Duration.ZERO, 0, BucketLayout.DEFAULT);
+ openKeyBuckets = keyManager.getExpiredOpenKeys(expireThreshold,
+ cleanupLimitPerTask, bucketLayout);
} catch (IOException e) {
- LOG.error("Unable to get hanging open keys, retry in"
- + " next interval", e);
+ LOG.error("Unable to get hanging open keys, retry in next interval", e);
+ }
+
+ if (openKeyBuckets != null && !openKeyBuckets.isEmpty()) {
+ int numOpenKeys = openKeyBuckets.stream()
+ .mapToInt(OpenKeyBucket::getKeysCount).sum();
+
+ OMRequest omRequest = createRequest(openKeyBuckets);
+ submitRequest(omRequest);
+
+ LOG.debug("Number of expired keys submitted for deletion: {}, elapsed"
+ + " time: {}ms", numOpenKeys, Time.monotonicNow() - startTime);
+ submittedOpenKeyCount.addAndGet(numOpenKeys);
}
return BackgroundTaskResult.EmptyTaskResult.newResult();
}
+
+ private OMRequest createRequest(List<OpenKeyBucket> openKeyBuckets) {
+ DeleteOpenKeysRequest request =
+ DeleteOpenKeysRequest.newBuilder()
+ .addAllOpenKeysPerBucket(openKeyBuckets)
+ .setBucketLayout(bucketLayout.toProto())
+ .build();
+
+ OMRequest omRequest = OMRequest.newBuilder()
+ .setCmdType(Type.DeleteOpenKeys)
+ .setDeleteOpenKeysRequest(request)
+ .setClientId(clientId.toString())
+ .build();
+
+ return omRequest;
+ }
+
+ private void submitRequest(OMRequest omRequest) {
+ try {
+ if (isRatisEnabled()) {
+ OzoneManagerRatisServer server = ozoneManager.getOmRatisServer();
+
+ RaftClientRequest raftClientRequest = RaftClientRequest.newBuilder()
+ .setClientId(clientId)
+ .setServerId(server.getRaftPeerId())
+ .setGroupId(server.getRaftGroupId())
+ .setCallId(runCount.get())
+ .setMessage(Message.valueOf(
+ OMRatisHelper.convertRequestToByteString(omRequest)))
+ .setType(RaftClientRequest.writeRequestType())
+ .build();
+
+ server.submitRequest(omRequest, raftClientRequest);
Review Comment:
After successful deletion. Can we add some metrics to show how many open keys has been deleted? This should be helpful in production.
--
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] captainzmc commented on pull request #3319: HDDS-4123. Integrate OM Open Key Cleanup Service Into Existing Code
Posted by GitBox <gi...@apache.org>.
captainzmc commented on PR #3319:
URL: https://github.com/apache/ozone/pull/3319#issuecomment-1159891983
> Limiting number of expired keys is working as expected.
>
The tests looked good, thanks @kaijchen working on this. Let's merge this PR first, if other modifications are needed, we can submit a new patch.
--
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