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


##########
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:
   This is only for local use as the key in `Map<String, 
OpenKeyBucket.Builder>`,
   and this is the only occurence in this method.
   
   If we define the map as `Map<Pair<String, String>, OpenKeyBucket.Builder>`,
   It can be replaced by `Pair.of(volume, bucket)`.
   Do you think there is a need to change it?



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

Reply via email to