Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

NSFS | versioning | Implement list-object-versions #7203

Merged
merged 3 commits into from
Jun 7, 2023
Merged

NSFS | versioning | Implement list-object-versions #7203

merged 3 commits into from
Jun 7, 2023

Conversation

vh05
Copy link
Contributor

@vh05 vh05 commented Feb 13, 2023 •
edited

Explain the changes

list-object-versions returns metadata about all versions of the objects in a bucket. You can also use request parameters as selection criteria to return metadata about a subset of all the object versions.

Moved object sorting under .versions to dir_cache

1. Kept 2 seperate cache for list-objects and list_object_versions
2. Sorting of the all entries done in version_dir_cache
3. handled keymarker and versions id marker
4. Added test cases for list object versions

Issues: Fixed #7169

Testing Instructions:

  1. Create few objects and append the same to create versions of the original object
  2. Delete few object
  3. Performat list-object_versions operation. This should returns the metadata of all objects including versions and delete_markers
  • Doc added/updated
  • Tests added

Signed-off-by: Vinayakswami Hariharmath vharihar@redhat.com

@vh05 vh05 force-pushed the nsfs_list_object_versions branch 3 times, most recently from c89c1b1 to 949aa0f Compare February 20, 2023 08:58
@romayalon
Copy link
Contributor

@v-harihar
Hi, wasn't here for a long time, finally, have the chance to review these changes.
I see some missing implementation here - In the design doc, few implementation details were discussed and I can't find them covered by this PR -
Sort - all keys and within key sort all versions.
New parameters -
version_id_marker - will start listing from that marker
New response fields -
DeleteMarker- will tell if a version is a delete marker / regular version.
IsLatest - boolean (will be marked when traversing on the real folder/ null for disabled buckets)
VersionIDMarker - string
NextVersionIDMarker - string

Also, we will need relevant unit tests for this to be covered.

@vh05
Copy link
Contributor Author

vh05 commented Feb 28, 2023

@v-harihar Hi, wasn't here for a long time, finally, have the chance to review these changes. I see some missing implementation here - In the design doc, few implementation details were discussed and I can't find them covered by this PR - Sort - all keys and within key sort all versions. New parameters - version_id_marker - will start listing from that marker New response fields - DeleteMarker- will tell if a version is a delete marker / regular version. IsLatest - boolean (will be marked when traversing on the real folder/ null for disabled buckets) VersionIDMarker - string NextVersionIDMarker - string

Also, we will need relevant unit tests for this to be covered.

sure.

@vh05 vh05 marked this pull request as draft March 23, 2023 09:16
@vh05 vh05 force-pushed the nsfs_list_object_versions branch from 949aa0f to c54d9e9 Compare March 28, 2023 05:34
@pull-request-size pull-request-size bot added size/L and removed size/M labels Mar 28, 2023
@vh05 vh05 force-pushed the nsfs_list_object_versions branch from c54d9e9 to ab270d0 Compare March 28, 2023 08:34
@pull-request-size pull-request-size bot added size/M and removed size/L labels Mar 28, 2023
@vh05 vh05 force-pushed the nsfs_list_object_versions branch from ab270d0 to bdcb986 Compare March 28, 2023 16:02
@pull-request-size pull-request-size bot added size/L and removed size/M labels Mar 28, 2023
@vh05 vh05 force-pushed the nsfs_list_object_versions branch 6 times, most recently from f420337 to 7945f08 Compare March 31, 2023 10:34
@vh05 vh05 marked this pull request as ready for review April 3, 2023 05:41
@vh05 vh05 force-pushed the nsfs_list_object_versions branch 2 times, most recently from 34b352b to bdfd7aa Compare April 3, 2023 07:20
@vh05 vh05 force-pushed the nsfs_list_object_versions branch 2 times, most recently from fd98596 to 2cc78b7 Compare May 29, 2023 11:14
romayalon
romayalon reviewed May 30, 2023
View reviewed changes
src/sdk/namespace_fs.js Outdated Show resolved Hide resolved
src/test/unit_tests/test_bucketspace_versioning.js Outdated Show resolved Hide resolved
src/sdk/namespace_fs.js Show resolved Hide resolved
src/sdk/namespace_fs.js Outdated Show resolved Hide resolved
src/sdk/namespace_fs.js Show resolved Hide resolved
src/sdk/namespace_fs.js Outdated Show resolved Hide resolved
@vh05 vh05 force-pushed the nsfs_list_object_versions branch 3 times, most recently from 6cdad11 to a82c772 Compare June 1, 2023 06:41
romayalon
romayalon reviewed Jun 4, 2023
View reviewed changes
src/sdk/namespace_fs.js Outdated Show resolved Hide resolved
src/sdk/namespace_fs.js Show resolved Hide resolved
src/sdk/namespace_fs.js Outdated Show resolved Hide resolved
liranmauda
liranmauda reviewed Jun 5, 2023
View reviewed changes
src/sdk/namespace_fs.js Outdated Show resolved Hide resolved
liranmauda
liranmauda reviewed Jun 5, 2023
View reviewed changes
src/sdk/namespace_fs.js
Comment on lines +2 to +3
/*eslint max-lines: ["error", 3000]*/
/*eslint max-statements: ["error", 80, { "ignoreTopLevelFunctions": true }]*/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder, isn't there a better way then adding those?

@vh05 vh05 force-pushed the nsfs_list_object_versions branch 3 times, most recently from 23ac50a to 6ff2db8 Compare June 5, 2023 10:24
dannyzaken
dannyzaken previously approved these changes Jun 5, 2023
View reviewed changes
@dannyzaken dannyzaken self-requested a review June 5, 2023 11:55
@dannyzaken dannyzaken dismissed their stale review June 5, 2023 11:55

approved by mistake. will let Romy approve

@vh05 vh05 requested review from romayalon and liranmauda June 5, 2023 12:52
@vh05 vh05 force-pushed the nsfs_list_object_versions branch from 6a60005 to e779895 Compare June 6, 2023 06:49
romayalon
romayalon reviewed Jun 6, 2023
View reviewed changes
src/sdk/namespace_fs.js Outdated Show resolved Hide resolved
src/sdk/namespace_fs.js Show resolved Hide resolved
src/sdk/namespace_fs.js Show resolved Hide resolved
src/sdk/namespace_fs.js Outdated Show resolved Hide resolved
src/sdk/namespace_fs.js Outdated Show resolved Hide resolved
src/sdk/namespace_fs.js Outdated Show resolved Hide resolved
src/sdk/namespace_fs.js Show resolved Hide resolved
@vh05 vh05 force-pushed the nsfs_list_object_versions branch 2 times, most recently from d27cb48 to f4295af Compare June 6, 2023 10:46
@romayalon
Copy link
Contributor

Approved but -

  1. Please squash the commits
  2. Please add the gaps in the next pr

vh05 added 3 commits June 7, 2023 13:50
@vh05
NSFS | versioning | Implement list-object-versions
c3fba59 
list-object-versions returns metadata about all versions
of the objects in a bucket. You can also use request
parameters as selection criteria to return metadata about
a subset of all the object versions.

Moved object sorting under .versions to dir_cache

1. Kept 2 seperate cache for list-objects and list_object_versions
2. Sorting of the all entries done in version_dir_cache
3. handled keymarker and versions id marker
4. Added test cases for list object versions

Issues: Fixed noobaa#7169
Signed-off-by: Vinayakswami Hariharmath <vharihar@redhat.com>
@vh05
Added test to verify the condition of missing key_marker
18b994d 
Signed-off-by: Vinayakswami Hariharmath <vharihar@redhat.com>
@vh05
Patch updated with review comments
6f75571 
Signed-off-by: Vinayakswami Hariharmath <vharihar@redhat.com>
@vh05 vh05 force-pushed the nsfs_list_object_versions branch from f4295af to 6f75571 Compare June 7, 2023 08:20
@vh05 vh05 merged commit c671039 into noobaa:master Jun 7, 2023
6 checks passed
@shirady shirady mentioned this pull request Jul 2, 2023
Merged
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@romayalon romayalon romayalon approved these changes

@jackyalbo jackyalbo Awaiting requested review from jackyalbo

@dannyzaken dannyzaken Awaiting requested review from dannyzaken

@liranmauda liranmauda Awaiting requested review from liranmauda

Assignees
No one assigned
Labels
size/L
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

NSFS | versioning | Implement list-object-versions
4 participants
@vh05 @romayalon @dannyzaken @liranmauda