HDDS-10634. Recon - listKeys API for listing keys with optional filters #6658

devmadhuu · 2024-05-08T17:43:06Z

This PR adds a new API in Recon for listing keys for OBS buckets, Legacy buckets and FSO buckets with filters and recursively in a flat structure for OBS, LEGACY and FSO buckets.

New API:

api/v1/keys/listKeys?startPrefix=/volume1/obs-bucket/&limit=105

Default values of API parameters if not provided:

replicationType - empty string and filter will not be applied, so list out all keys irrespective of replication type.
creationTime - empty string and filter will not be applied, so list out keys irrespective of age, else list out keys which got created on or after provided creationTime
keySize - 0 bytes, which means all keys greater than zero bytes will be listed, effectively all.
startPrefix - /
prevKey - ""
limit - 1000
Behavior of API:
For OBS bucket - list out limit number of keys on the provided path.
This API will implement pagination support using prevKey and limit params.

Get List of All Keys:
GET /api/v1/keys/listKeys

API params:

replicationType - Filter for RATIS or EC replication keys
creationDate in "MM-dd-yyyy HH:mm:ss" string format.
startPrefix
prevKey
limit
keySize

Now lets consider, we have following OBS, LEGACY and FSO bucket key/files namespace tree structure

For OBS Bucket

/volume1/obs-bucket/key1
/volume1/obs-bucket/key1/key2
/volume1/obs-bucket/key1/key2/key3
/volume1/obs-bucket/key4
/volume1/obs-bucket/key5
/volume1/obs-bucket/key6

For LEGACY Bucket

/volume1/legacy-bucket/key
/volume1/legacy-bucket/key1/key2
/volume1/legacy-bucket/key1/key2/key3
/volume1/legacy-bucket/key4
/volume1/legacy-bucket/key5
/volume1/legacy-bucket/key6

For FSO Bucket

/volume1/fso-bucket/dir1/dir2/dir3
/volume1/fso-bucket/dir1/testfile
/volume1/fso-bucket/dir1/file1
/volume1/fso-bucket/dir1/dir2/testfile
/volume1/fso-bucket/dir1/dir2/file1
/volume1/fso-bucket/dir1/dir2/dir3/testfile
/volume1/fso-bucket/dir1/dir2/dir3/file1

Input Request for OBS bucket:

   `api/v1/keys/listKeys?startPrefix=/volume1/obs-bucket&limit=2&replicationType=RATIS`

Output Response:

{
   "status": "OK",
   "path": "/volume1/obs-bucket",
   "replicatedDataSize": 20971520,
   "unReplicatedDataSize": 20971520,
   "keyCount": 2,
   "lastKey": "/volume1/obs-bucket/key1/key2",
   "keys": [
       {
           "key": "/volume1/obs-bucket/key1",
           "path": "key1",
           "inStateSince": 1715174266126,
           "size": 10485760,
           "replicatedSize": 10485760,
           "replicationInfo": {
               "replicationFactor": "ONE",
               "requiredNodes": 1,
               "replicationType": "RATIS"
           },
           "creationTime": 1715174266126,
           "modificationTime": 1715174267480,
           "isKey": true
       },
       {
           "key": "/volume1/obs-bucket/key1/key2",
           "path": "key1/key2",
           "inStateSince": 1715174269510,
           "size": 10485760,
           "replicatedSize": 10485760,
           "replicationInfo": {
               "replicationFactor": "ONE",
               "requiredNodes": 1,
               "replicationType": "RATIS"
           },
           "creationTime": 1715174269510,
           "modificationTime": 1715174270410,
           "isKey": true
       }
   ]
}

Input Request for FSO bucket:

       `api/v1/keys/listKeys?startPrefix=/volume1/fso-bucket&limit=2&replicationType=RATIS`

Output Response:

{
    "status": "OK",
    "path": "/volume1/fso-bucket",
    "replicatedDataSize": 62914560,
    "unReplicatedDataSize": 20971520,
    "keyCount": 2,
    "lastKey": "/-9223372036854775552/-9223372036854775040/-9223372036854774525/testfile",
    "keys": [
        {
            "key": "/-9223372036854775552/-9223372036854775040/-9223372036854774525/file1",
            "path": "file1",
            "inStateSince": 1715174237440,
            "size": 10485760,
            "replicatedSize": 31457280,
            "replicationInfo": {
                "replicationFactor": "THREE",
                "requiredNodes": 3,
                "replicationType": "RATIS"
            },
            "creationTime": 1715174237440,
            "modificationTime": 1715174238161,
            "isKey": true
        },
        {
            "key": "/-9223372036854775552/-9223372036854775040/-9223372036854774525/testfile",
            "path": "testfile",
            "inStateSince": 1715174234840,
            "size": 10485760,
            "replicatedSize": 31457280,
            "replicationInfo": {
                "replicationFactor": "THREE",
                "requiredNodes": 3,
                "replicationType": "RATIS"
            },
            "creationTime": 1715174234840,
            "modificationTime": 1715174235562,
            "isKey": true
        }
    ]
}

Input Request for Legacy bucket:

       `api/v1/keys/listKeys?startPrefix=/volume1/legacy-bucket&limit=2&replicationType=RATIS`

Output Response:

{
    "status": "OK",
    "path": "/volume1/legacy-bucket",
    "replicatedDataSize": 52428800,
    "unReplicatedDataSize": 52428800,
    "keyCount": 2,
    "lastKey": "/volume1/legacy-bucket/key1/key2",
    "keys": [
        {
            "key": "/volume1/legacy-bucket/key1",
            "path": "key1",
            "inStateSince": 1715174303702,
            "size": 10485760,
            "replicatedSize": 10485760,
            "replicationInfo": {
                "replicationFactor": "ONE",
                "requiredNodes": 1,
                "replicationType": "RATIS"
            },
            "creationTime": 1715174303702,
            "modificationTime": 1715174304619,
            "isKey": true
        },
        {
            "key": "/volume1/legacy-bucket/key1/key2",
            "path": "key1/key2",
            "inStateSince": 1715174306641,
            "size": 41943040,
            "replicatedSize": 41943040,
            "replicationInfo": {
                "replicationFactor": "ONE",
                "requiredNodes": 1,
                "replicationType": "RATIS"
            },
            "creationTime": 1715174306641,
            "modificationTime": 1715174307994,
            "isKey": true
        }
    ]
}

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-10634

How was this patch tested?

Added Junit test cases and tested various assertions.

…Legacy bucket keys with filters.

devmadhuu · 2024-05-10T10:39:36Z

@sodonnel @sumitagrawl @ArafatKhan2198 @dombizita kindly review.

sumitagrawl

@devmadhuu Thanks for working, have few comments

hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/types/ListKeysResponse.java

hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/OMDBInsightEndpoint.java

ArafatKhan2198

Thanks for working on this @devmadhuu Few comments for now Ill send in the next comments after some time.

ArafatKhan2198 · 2024-05-17T07:19:12Z

hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/ReconUtils.java

+ if (StringUtils.isEmpty(dateString)) {
+ return Instant.now().toEpochMilli();
+ }


Should we also Validate dateFormat and timeZone parameters to ensure they are not null.?

In this code flow, caller is passing dateFormat and timezone, so they cannot be null, however I have added null check as this method can be called by other callers as well.

ArafatKhan2198 · 2024-05-17T07:19:59Z

hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/ReconUtils.java

+ * @param dateString
+ * @param dateFormat
+ * @param timeZone
+ * @return


@return the epoch milliseconds representation of the date.

ArafatKhan2198 · 2024-05-17T07:21:05Z

hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/ReconUtils.java

+ Date date = sdf.parse(dateString);
+ return date.getTime(); // Convert to epoch milliseconds
+ } catch (ParseException parseException) {
+ log.error("Date parse exception for date: {} in format: {} -> {}", dateString, dateFormat, parseException);


Can we also log the exception details in the general Exception catch block.

ArafatKhan2198 · 2024-05-17T07:28:15Z

hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/OMDBInsightEndpoint.java

+ public Response validateNames(String resName)
+ throws IllegalArgumentException {
+ if (resName.length() < OzoneConsts.OZONE_MIN_BUCKET_NAME_LENGTH ||
+ resName.length() > OzoneConsts.OZONE_MAX_BUCKET_NAME_LENGTH) {
+ throw new IllegalArgumentException(
+ "Bucket or Volume name length should be between " +
+ OzoneConsts.OZONE_MIN_BUCKET_NAME_LENGTH + " and " +
+ OzoneConsts.OZONE_MAX_BUCKET_NAME_LENGTH);
+ }
+
+ if (resName.charAt(0) == '.' || resName.charAt(0) == '-' ||
+ resName.charAt(resName.length() - 1) == '.' ||


Can we move some the methods mentioned in this class to ReconUtils?
Like the validateNames here, which could be utilised by other endpoints as well.

ArafatKhan2198 · 2024-05-17T07:31:19Z

hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/OMDBInsightEndpoint.java

+ * @param startPrefix The search prefix that was used.
+ * @return The response indicating that no keys matched the search prefix.
+ */
+ private Response noMatchedKeysResponse(String startPrefix) {


This is an effective way to generate response messages. Can we create a new class named ResponseUtil and move these methods to it? This will help reduce the amount of code in the current class and make these utility methods available for other endpoints as well.

ArafatKhan2198 · 2024-05-17T07:37:32Z

hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/OMDBInsightEndpoint.java

+ .filter(keySizeFilter)
+ .collect(Collectors.toList());


The anyMatch method can beused instead of collecting the filtered entries to a list and checking its size. This makes the code more efficient as the stream will short-circuit on the first match.

As discussed, anyMatch just matches any one element in stream and terminate. We need apply filters on all elements. So this may not suit our usecase.

ArafatKhan2198 · 2024-05-17T07:39:56Z

hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/OMDBInsightEndpoint.java

+ return matchedKeys;
+ }
+
+ private boolean applyFilters(Table.KeyValue<String, OmKeyInfo> entry, ParamInfo paramInfo) throws IOException {


Since we are already propagating the throws IOException upwards do we require the try and catch block to handle the IOException?

We could further simplify the code in the method by removing them.

Yes, we need for defining the predicate definition, else compiler gives error.

ArafatKhan2198

Some more comments @devmadhuu

ArafatKhan2198 · 2024-05-17T15:01:18Z

hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/OMDBInsightEndpoint.java

+ * {
+ * "status": "OK",
+ * "path": "/volume1/obs-bucket",
+ * "replicatedDataSize": 62914560,
+ * "unReplicatedDataSize": 62914560,
+ * "lastKey": "/volume1/obs-bucket/key6",
+ * "keys": [
+ * {
+ * "key": "/volume1/obs-bucket/key1",
+ * "path": "volume1/obs-bucket/key1",
+ * "size": 10485760,
+ * "replicatedSize": 10485760,
+ * "replicationInfo": {


If the structure of the JSON response remains consistent across different bucket types, there is no need to show multiple example responses for each bucket type. Instead, we can show one example response and clarify that this structure applies to all bucket types (OBS, Legacy, and FSO).

I want to keep OBS and FSO, as output values different in path, so that it is evident from javadoc comment. I have removed for LEGACY buckets.

ArafatKhan2198 · 2024-05-19T07:58:52Z

hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/OMDBInsightEndpoint.java

+ * @param ids The IDs to construct the object path with.
+ * @return The constructed object path.
+ */
+ private String constructObjectPathWithPrefix(long... ids) {


I believe this can also be placed inside ReconUtils

ArafatKhan2198 · 2024-05-19T07:59:37Z

...p-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/handlers/FSOBucketHandler.java

@@ -300,5 +300,4 @@ public OmDirectoryInfo getDirInfo(String[] names) throws IOException {
 .getDirectoryTable().getSkipCache(dirKey);
 return dirInfo;
 }
-


Please Remove this unwanted change, from the file.

extra line space was not needed. So removed.

ArafatKhan2198 · 2024-05-19T08:28:02Z

hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/api/TestOmDBInsightEndPoint.java

@@ -785,10 +1103,284 @@ public void testGetDirectorySizeInfo() throws Exception {
 assertEquals(18L, keyInsightInfoResp.getUnreplicatedDataSize());
 }

+ @Test
+ public void testListKeysFSOBucket() {


The existing test cases cover the following main scenarios which is great!

Listing keys at different levels (bucket, directory) for FSO and OBS buckets.

Verifying the number of keys returned, paths, keys, replication types, and last keys.

Testing pagination with different limits and across multiple pages.

Filtering keys based on replication type, creation date, and key size.

Handling edge cases like empty results and last page with an empty last key.

Can we write a few more methods that test out these simple scenarios as well.
Empty Buckets: Verify the behaviour when the bucket is empty.
Non-existent Paths: Test with paths that do not exist to ensure the API handles such cases gracefully.

sumitagrawl

@devmadhuu few more comments, please check

hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/OMDBInsightEndpoint.java

… with FSEnabled flag.

sumitagrawl

@devmadhuu Given few comments

hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/OMDBInsightEndpoint.java

…thLimitAcrossDirsAtBucketLevel.

sumitagrawl

LGTM

Arafat2198

Thanks for the changes @devmadhuu
The patch looks good!
LGTM +1

branch_HDDS-10634. Recon - listKeys API for listing of OBS , FSO and …

350edfb

…Legacy bucket keys with filters.

devmadhuu changed the title ~~branch_HDDS-10634. Recon - listKeys API for listing of OBS , FSO and Legacy bucket keys with filters.~~ HDDS-10634. Recon - listKeys API for listing of OBS , FSO and Legacy bucket keys with filters. May 8, 2024

deveshsingh added 2 commits May 8, 2024 23:49

branch_HDDS-10634. Fixed findbugs.

5e98fa1

branch_HDDS-10634. Updated full key path for FSO keys.

0879aa2

adoroszlai changed the title ~~HDDS-10634. Recon - listKeys API for listing of OBS , FSO and Legacy bucket keys with filters.~~ HDDS-10634. Recon - listKeys API for listing keys with optional filters May 10, 2024

branch_HDDS-10634. fixed findbugs.

4620d85

devmadhuu marked this pull request as ready for review May 10, 2024 10:39

adoroszlai added the recon label May 13, 2024

sumitagrawl reviewed May 14, 2024

View reviewed changes

deveshsingh added 5 commits May 15, 2024 09:07

branch_HDDS-10634. fixed some review comments.

f3aab8c

branch_HDDS-10634. fixed some review comments.

d83ba58

branch_HDDS-10634. fixed some review comments.

8adef6e

branch_HDDS-10634. fixed some review comments.

b693c2d

branch_HDDS-10634. fixed findbugs.

c3c41e3

ArafatKhan2198 suggested changes May 17, 2024

View reviewed changes

branch_HDDS-10634. fixed review comments.

7b6c0be

ArafatKhan2198 reviewed May 19, 2024

View reviewed changes

branch_HDDS-10634. Added additional test cases.

c4b0a3e

sumitagrawl reviewed May 20, 2024

View reviewed changes

deveshsingh added 3 commits May 21, 2024 12:17

branch_HDDS-10634. Added additional test cases for LEGACY bucket keys…

3c418e9

… with FSEnabled flag.

branch_HDDS-10634. Added additional test cases for LEGACY bucket keys…

507d6de

… with FSEnabled flag.

branch_HDDS-10634. Added additional test cases for LEGACY bucket keys…

1c1169e

… with FSEnabled flag.

sumitagrawl reviewed May 23, 2024

View reviewed changes

hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/OMDBInsightEndpoint.java Outdated Show resolved Hide resolved

deveshsingh added 4 commits May 23, 2024 17:31

branch_HDDS-10634. Fixed review comments.

d0bfbde

branch_HDDS-10634. Fixed review comments.

ce026b5

branch_HDDS-10634. Added new test case testListKeysFSOBucketTwoPathWi…

77a2cb7

…thLimitAcrossDirsAtBucketLevel.

branch_HDDS-10634. Added new testcases.

3a8ef74

sumitagrawl approved these changes May 24, 2024

View reviewed changes

Arafat2198 approved these changes May 24, 2024

View reviewed changes

sumitagrawl merged commit 40951a4 into apache:master May 26, 2024
39 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

HDDS-10634. Recon - listKeys API for listing keys with optional filters #6658

HDDS-10634. Recon - listKeys API for listing keys with optional filters #6658

devmadhuu commented May 8, 2024 •

edited

devmadhuu commented May 10, 2024

sumitagrawl left a comment

ArafatKhan2198 left a comment

ArafatKhan2198 May 17, 2024

devmadhuu May 17, 2024

ArafatKhan2198 May 17, 2024

devmadhuu May 17, 2024

ArafatKhan2198 May 17, 2024

devmadhuu May 17, 2024

ArafatKhan2198 May 17, 2024

devmadhuu May 17, 2024

ArafatKhan2198 May 17, 2024

devmadhuu May 17, 2024

ArafatKhan2198 May 17, 2024

devmadhuu May 17, 2024

ArafatKhan2198 May 17, 2024

devmadhuu May 17, 2024

ArafatKhan2198 left a comment

ArafatKhan2198 May 17, 2024

devmadhuu May 20, 2024

ArafatKhan2198 May 19, 2024

devmadhuu May 20, 2024

ArafatKhan2198 May 19, 2024

devmadhuu May 20, 2024

ArafatKhan2198 May 19, 2024

devmadhuu May 20, 2024

sumitagrawl left a comment

sumitagrawl left a comment

sumitagrawl left a comment

Arafat2198 left a comment

HDDS-10634. Recon - listKeys API for listing keys with optional filters #6658

HDDS-10634. Recon - listKeys API for listing keys with optional filters #6658

Conversation

devmadhuu commented May 8, 2024 • edited

What is the link to the Apache JIRA

How was this patch tested?

devmadhuu commented May 10, 2024

sumitagrawl left a comment

Choose a reason for hiding this comment

ArafatKhan2198 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ArafatKhan2198 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sumitagrawl left a comment

Choose a reason for hiding this comment

sumitagrawl left a comment

Choose a reason for hiding this comment

sumitagrawl left a comment

Choose a reason for hiding this comment

Arafat2198 left a comment

Choose a reason for hiding this comment

devmadhuu commented May 8, 2024 •

edited