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

block_store_fs enhanced to use xattrs and migrate blocks to tier2 #7283

Merged
merged 2 commits into from
May 5, 2023

Conversation

guymguym
Copy link
Member

@guymguym guymguym commented Apr 27, 2023

Explain the changes

  1. Enhance backing_store_fs to use xattr to store block_md json instead of in separate .meta files.
  2. This allows reducing the number of files needed on the file system by half.
  3. Add new config option (false by default) - config.BLOCK_STORE_FS_TIER2_ENABLED - enable to set custom xattr trigger to signal the filesystem to migrate the data to archive tier (for filesystems that support that).
  4. Added new config option (false by default) - config.BLOCK_STORE_FS_MAPPING_INFO_ENABLED - enable to pass on extra recovery information (which bucket, object, offset, etc) to be added in block_md xattr, for cases when recovery without the db is needed.

Issues: Fixed #xxx / Gap #xxx

  1. NA

Testing Instructions:

  1. Regression of backing_store_fs (PV pools from the operator).
  2. Enable the new configs to test the new options and check the xattr of the blocks (e.g xattr -l <filename> on mac).

src/native/fs/fs_napi.cpp Outdated Show resolved Hide resolved
src/sdk/namespace_fs.js Outdated Show resolved Hide resolved
src/sdk/namespace_fs.js Show resolved Hide resolved
Signed-off-by: Guy Margalit <guymguym@gmail.com>
Signed-off-by: Guy Margalit <guymguym@gmail.com>
}

try {
await nb_native().fs.writeFile(fs_context, block_path, data, {
Copy link
Member

Choose a reason for hiding this comment

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

I am just trying to understand here. What happens if the unlink succeeds but this fails and _test_root_path_exists throws as well?

Copy link
Member Author

Choose a reason for hiding this comment

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

The usage is not going to update in that case, which is indeed undesired, but since usage calculation is not atomic anyhow, any failure can cause this.

I explained to @romayalon's question in the comment above that the overwrite block case is not an actual flow we invoke on purpose.

The reason we decided that write_block should handle overwrites was that we didn't want to fail user uploads on one hand, and we didn't want the usage to start getting skewed because of retries so we had to check if this is an overwrite first and "reduce" that replaced size.

I guess that simply failing the request if the same block is written with different data could just be better.

Copy link
Member Author

Choose a reason for hiding this comment

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

For now I will keep the existing behavior and we can revise as needed in the future.

@guymguym guymguym merged commit eca37db into noobaa:master May 5, 2023
6 checks passed
guymguym added a commit to guymguym/noobaa-core that referenced this pull request Jun 21, 2023
guymguym added a commit to guymguym/noobaa-core that referenced this pull request Jun 21, 2023
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