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

curvefs: support x-amz-storage-class #2926

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

Tianpingan
Copy link

What problem does this PR solve?

Issue Number: #2783

Problem Summary:

What is changed and how it works?

What's Changed:

  1. Add support for PutObjectOptions in PutObject and PutObjectAsync methods, enabling the setting of storage class.
  2. Polish the client to align with updated PutObjectAsync method.
  3. Polish the meta_server s3compaction to align with updated PutObject method.

How it Works:

When creating a request, the configuration for storage class is encompassed within the PutObjectOptions, applied alongside both PutObject and PutObjectAsync methods.
This allows us to employ PutObjectOptions to construct the S3 request, including the storage class information.

Side effects(Breaking backward compatibility? Performance regression?):

Check List

  • Relevant documentation/comments is changed or added
  • I acknowledge that all my contributions will be made under the project's license

@aspirer
Copy link
Contributor

aspirer commented Nov 24, 2023

  1. I don’t see that the s3info-related RPC request uses your newly added storageclass parameter. What is its function?
  2. Is the storage class configured at the file system level or the fuse client level? I saw that the storageclass parameter was added to the rpc parameters of your s3info, but the persistent storage operation was not seen on the mds side. How to use it later? In addition, I see that there is also a configuration item analysis of this parameter. What is the relationship between the two?
  3. Our goal is to configure it at the file system level. Each file system configures the storage class it wants to use.

@h0hmj h0hmj self-requested a review November 27, 2023 03:09
@h0hmj
Copy link
Contributor

h0hmj commented Nov 27, 2023

  1. I don’t see that the s3info-related RPC request uses your newly added storageclass parameter. What is its function?
  2. Is the storage class configured at the file system level or the fuse client level? I saw that the storageclass parameter was added to the rpc parameters of your s3info, but the persistent storage operation was not seen on the mds side. How to use it later? In addition, I see that there is also a configuration item analysis of this parameter. What is the relationship between the two?
  3. Our goal is to configure it at the file system level. Each file system configures the storage class it wants to use.

Q2&3 s3info is part of fsinfo in etcd

@h0hmj
Copy link
Contributor

h0hmj commented Nov 27, 2023

need set storageclass when createfs.
maybe related changes

  1. tool
  2. createfs rpc

@Tianpingan
Copy link
Author

need set storageclass when createfs. maybe related changes

  1. tool
  2. createfs rpc

Got it. since the storage class is already part of S3info, so i decide to use the existing createfs rpc for tools, simply adding the storage class parameter.

@Tianpingan
Copy link
Author

need set storageclass when createfs. maybe related changes

  1. tool
  2. createfs rpc

i have updated the tool in createfs/src/tools and tools-v2 to support storage class

@h0hmj
Copy link
Contributor

h0hmj commented Dec 25, 2023

cicheck

1 similar comment
@h0hmj
Copy link
Contributor

h0hmj commented Dec 25, 2023

cicheck

Signed-off-by: tianpingan <[email protected]>
@Tianpingan
Copy link
Author

cicheck

Signed-off-by: tianpingan <[email protected]>
@h0hmj
Copy link
Contributor

h0hmj commented Dec 26, 2023

cicheck

Signed-off-by: tianpingan <[email protected]>
@h0hmj
Copy link
Contributor

h0hmj commented Dec 26, 2023

cicheck

3 similar comments
@h0hmj
Copy link
Contributor

h0hmj commented Dec 26, 2023

cicheck

@h0hmj
Copy link
Contributor

h0hmj commented Dec 27, 2023

cicheck

@h0hmj
Copy link
Contributor

h0hmj commented Dec 27, 2023

cicheck

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants