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.

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

[NSFS] Add support for restore-object #7507

Merged

Conversation

tangledbytes
Copy link
Member

Explain the changes

This PR adds

  1. Primitive support for restore-object API in NSFS.
  2. Support for storage-class in head/get-object.
  • Doc added/updated
  • Tests added

@tangledbytes tangledbytes force-pushed the utkarsh/add/restore-api-iter2 branch 3 times, most recently from cdb3bc8 to 057611a Compare September 25, 2023 18:32
src/endpoint/s3/s3_utils.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 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 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
@romayalon
Copy link
Contributor

@tangledbytes what about adding a few unit tests? :)

@tangledbytes tangledbytes force-pushed the utkarsh/add/restore-api-iter2 branch 3 times, most recently from 77a8d88 to 4e341f0 Compare September 26, 2023 12:52
@tangledbytes tangledbytes force-pushed the utkarsh/add/restore-api-iter2 branch 2 times, most recently from 8a7f4b9 to 428533e Compare September 27, 2023 15:42
Copy link
Member

@guymguym guymguym left a comment

Choose a reason for hiding this comment

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

some comments, but overall looks good!

src/endpoint/s3/ops/s3_post_object_restore.js Show resolved Hide resolved
src/endpoint/s3/ops/s3_post_object_restore.js Outdated Show resolved Hide resolved
src/endpoint/s3/ops/s3_post_object_restore.js Show resolved Hide resolved
src/endpoint/s3/ops/s3_post_object_restore.js Outdated Show resolved Hide resolved
src/endpoint/s3/ops/s3_get_object.js Outdated Show resolved Hide resolved
const restore = [`ongoing-request="${object_md.restore_status.ongoing}"`];
if (!object_md.restore_status.ongoing && object_md.restore_status.expiry_time) {
// Expiry time is in UTC format
const expiry_date = new Date(object_md.restore_status.expiry_time).toUTCString();
Copy link
Member

Choose a reason for hiding this comment

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

but what is the date format of the xattr user.noobaa.restore.expiry ?
is it a number? worth noting it here in the comment at least.
in nb.d.ts I see this is a Date... So unclear.

Copy link
Member Author

Choose a reason for hiding this comment

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

Mentioned the format in the xattr comment (All the data types are mentioned in the comments).

Comment on lines +67 to +78
/**
* XATTR_RESTORE_EXPIRY is set to a ISO DATE by the underlying restore process or by
* NooBaa (in case restore is issued again while the object is on disk).
* This is read by the underlying "disk evict" process to determine if the object
* should be evicted from the disk or not.
*
* NooBaa will use this date to determine if the object is on disk or not, if the
* expiry date is in the future, the object is on disk, if the expiry date is in
* the past, the object is not on disk. This may or may not represent the actual
* state of the object on disk, but is probably good enough for NooBaa's purposes
* assuming that restore request for already restored objects fails gracefully.
*/
Copy link
Member

Choose a reason for hiding this comment

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

noice comment! I wish we had such comments on all our xattrs...

src/sdk/namespace_fs.js Show resolved Hide resolved
@tangledbytes tangledbytes force-pushed the utkarsh/add/restore-api-iter2 branch 2 times, most recently from b66ad0b to 5ac2631 Compare September 29, 2023 06:14
Signed-off-by: Utkarsh Srivastava <srivastavautkarsh8097@gmail.com>

add changes to nb.d.ts

Signed-off-by: Utkarsh Srivastava <srivastavautkarsh8097@gmail.com>

fix errors when no storage class is provided - treat as STANDARD

Signed-off-by: Utkarsh Srivastava <srivastavautkarsh8097@gmail.com>

fix storage class assignment

Signed-off-by: Utkarsh Srivastava <srivastavautkarsh8097@gmail.com>

address minor comments on the PR

Signed-off-by: Utkarsh Srivastava <srivastavautkarsh8097@gmail.com>

add unit tests and minor fixes

Signed-off-by: Utkarsh Srivastava <srivastavautkarsh8097@gmail.com>

fix unit tests

Signed-off-by: Utkarsh Srivastava <srivastavautkarsh8097@gmail.com>

fix lint error

Signed-off-by: Utkarsh Srivastava <srivastavautkarsh8097@gmail.com>

fix _is_storage_class_supported

Signed-off-by: Utkarsh Srivastava <srivastavautkarsh8097@gmail.com>

fix unit test

Signed-off-by: Utkarsh Srivastava <srivastavautkarsh8097@gmail.com>

revamp the implementation to adhere more closely to S3 behaviours

Signed-off-by: Utkarsh Srivastava <srivastavautkarsh8097@gmail.com>

add assertions to ensure xattrs aren't leaked into objectmds

Signed-off-by: Utkarsh Srivastava <srivastavautkarsh8097@gmail.com>

cleanup restore_status logic and fix expiry extension

Signed-off-by: Utkarsh Srivastava <srivastavautkarsh8097@gmail.com>

fix broken get_object

Signed-off-by: Utkarsh Srivastava <srivastavautkarsh8097@gmail.com>
@tangledbytes tangledbytes merged commit 27a078a into noobaa:master Sep 29, 2023
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants