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/29 00:00:03 UTC

[GitHub] [ozone] kerneltime commented on a diff in pull request #3226: HDDS-6491. Support FSO keys in getExpiredOpenKeys

kerneltime commented on code in PR #3226:
URL: https://github.com/apache/ozone/pull/3226#discussion_r861382786


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java:
##########
@@ -1193,33 +1195,55 @@ public List<BlockGroup> getPendingDeletionKeys(final int keyCount)
   }
 
   @Override
-  public List<String> getExpiredOpenKeys(Duration expireThreshold,
-      int count) throws IOException {
+  public List<OpenKeyBucket> getExpiredOpenKeys(Duration expireThreshold,
+      int limit, BucketLayout bucketLayout) throws IOException {
+    Map<String, OpenKeyBucket.Builder> expiredKeys = new HashMap<>();
+
     // Only check for expired keys in the open key table, not its cache.
     // If a key expires while it is in the cache, it will be cleaned
     // up after the cache is flushed.
-    List<String> expiredKeys = Lists.newArrayList();
-
     try (TableIterator<String, ? extends KeyValue<String, OmKeyInfo>>
-        keyValueTableIterator = getOpenKeyTable(getBucketLayout()).iterator()) {
+        keyValueTableIterator = getOpenKeyTable(bucketLayout).iterator()) {
 
-      final long queryTime = Instant.now().toEpochMilli();
+      final long expiredCreationTimestamp =
+          Instant.now().minus(expireThreshold).toEpochMilli();
 
-      while (keyValueTableIterator.hasNext() && expiredKeys.size() < count) {
+      OpenKey.Builder builder = OpenKey.newBuilder();
+
+      int count = 0;
+      while (count < limit && keyValueTableIterator.hasNext()) {
         KeyValue<String, OmKeyInfo> openKeyValue = keyValueTableIterator.next();
-        String openKey = openKeyValue.getKey();
+        String dbOpenKeyName = openKeyValue.getKey();
+        long clientID = Long.parseLong(dbOpenKeyName.substring(
+            dbOpenKeyName.lastIndexOf(OM_KEY_PREFIX) + 1));
         OmKeyInfo openKeyInfo = openKeyValue.getValue();
 
-        final long openKeyAgeMillis = queryTime - openKeyInfo.getCreationTime();
-        final Duration openKeyAge = Duration.ofMillis(openKeyAgeMillis);
-
-        if (openKeyAge.compareTo(expireThreshold) >= 0) {
-          expiredKeys.add(openKey);
+        if (openKeyInfo.getCreationTime() <= expiredCreationTimestamp) {
+          final String volume = openKeyInfo.getVolumeName();
+          final String bucket = openKeyInfo.getBucketName();
+          final String mapKey = volume + OM_KEY_PREFIX + bucket;

Review Comment:
   Please add helper static methods to evaluate such strings.



##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java:
##########
@@ -1193,33 +1195,55 @@ public List<BlockGroup> getPendingDeletionKeys(final int keyCount)
   }
 
   @Override
-  public List<String> getExpiredOpenKeys(Duration expireThreshold,
-      int count) throws IOException {
+  public List<OpenKeyBucket> getExpiredOpenKeys(Duration expireThreshold,
+      int limit, BucketLayout bucketLayout) throws IOException {
+    Map<String, OpenKeyBucket.Builder> expiredKeys = new HashMap<>();
+
     // Only check for expired keys in the open key table, not its cache.
     // If a key expires while it is in the cache, it will be cleaned
     // up after the cache is flushed.
-    List<String> expiredKeys = Lists.newArrayList();
-
     try (TableIterator<String, ? extends KeyValue<String, OmKeyInfo>>
-        keyValueTableIterator = getOpenKeyTable(getBucketLayout()).iterator()) {
+        keyValueTableIterator = getOpenKeyTable(bucketLayout).iterator()) {
 
-      final long queryTime = Instant.now().toEpochMilli();
+      final long expiredCreationTimestamp =
+          Instant.now().minus(expireThreshold).toEpochMilli();
 
-      while (keyValueTableIterator.hasNext() && expiredKeys.size() < count) {
+      OpenKey.Builder builder = OpenKey.newBuilder();
+
+      int count = 0;
+      while (count < limit && keyValueTableIterator.hasNext()) {
         KeyValue<String, OmKeyInfo> openKeyValue = keyValueTableIterator.next();
-        String openKey = openKeyValue.getKey();
+        String dbOpenKeyName = openKeyValue.getKey();
+        long clientID = Long.parseLong(dbOpenKeyName.substring(
+            dbOpenKeyName.lastIndexOf(OM_KEY_PREFIX) + 1));

Review Comment:
   Please add a separate static method to evaluate the `index` for `substring` in `OzoneConsts.java`



##########
hadoop-ozone/interface-storage/src/main/java/org/apache/hadoop/ozone/om/OMMetadataManager.java:
##########
@@ -260,13 +261,14 @@ List<OmVolumeArgs> listVolumes(String userName, String prefix,
    * Returns the names of up to {@code count} open keys whose age is
    * greater than or equal to {@code expireThreshold}.
    *
-   * @param count The maximum number of open keys to return.
+   * @param limit The maximum number of open keys to return.
    * @param expireThreshold The threshold of open key expire age.
-   * @return a list of {@link String} representing names of open expired keys.
+   * @param bucketLayout The type of open keys to get (e.g. DEFAULT or FSO).
+   * @return a {@link List} of {@link OpenKeyBucket}, the expired open keys.

Review Comment:
   ```suggestion
      * @return a {@link List} of {@link OpenKeyBucket}: List of open keys per bucket.
   ```



-- 
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