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

NC | NSFS | Add stat to account_id_cache #8642

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

Conversation

shirady
Copy link
Contributor

@shirady shirady commented Jan 1, 2025

Explain the changes

  1. In config.js move the definition of ACCOUNTS_ID_CACHE_EXPIRY near the the configurations of OBJECT_SDK_BUCKET_CACHE_EXPIRY_MS and OBJECT_SDK_ACCOUNT_CACHE_EXPIRY_MS.
  2. In accountspace_fs.js:
    • Remove the Number conversion as the value is number (was Number(config.ACCOUNTS_ID_CACHE_EXPIRY)).
    • Change the JSDoc of params so it will be easier to read.
    • Change the load inside function to be get_identity_by_id_and_stat_file so we will have the identity with the stat as property inside it.
    • Add the validate method, and add the functions _validate_account_id and check_same_stat_account that were partially copied from object_sdk.
  3. In config_fs add the function get_identity_by_id_and_stat_file and stat_account_config_file_by_identity that would try to stat the account config file (covering the identity, accounts_by_name - new path, accounts - old path).
  4. In object_sdk.js add a new line between anonymous_access_key bucket_namespace_cache for easier readability, fix a typo in a comment.

Note: the changes are based on the suggesting written as a comment.

Issues:

  1. It is a mentioned GAP in NC | NSFS | Add stat to bucket_namespace_cache #8527 and NC | NSFS | Add stat to account_cache #8585.

Testing Instructions:

Automatic Test:

Please run: sudo NC_CORETEST=true node ./node_modules/mocha/bin/mocha ./src/test/unit_tests/test_nc_with_a_couple_of_forks.js

Manual Test:

  1. Create an account with the CLI: sudo node src/cmd/manage_nsfs account add --name <account-name> --new_buckets_path /Users/buckets/ --access_key <access-key> --secret_key <secret-key> --uid <uid> --gid <gid>
    Note: before creating the account need to give permission to the new_buckets_path: chmod 777 /Users/buckets/.
  2. Add the line: dbg.log0('SDSD same_stat', same_stat); before if (!same_stat) { // config file of bucket was changed and start the NSFS server with: sudo node src/cmd/nsfs --debug 5
  3. Create the alias for S3 service:alias nc-user-1-s3=‘AWS_ACCESS_KEY_ID=<access-key> AWS_SECRET_ACCESS_KEY=<secret-key> aws --no-verify-ssl --endpoint-url https://localhost:6443’.
  4. Check the connection to the endpoint and try to list the buckets (should be empty): nc-user-1-s3 s3 ls; echo $?
  5. Add bucket to the account using AWS CLI: nc-user-1-s3 s3 mb s3://bucket-01 (bucket-01 is the bucket name in this example)
  6. Head the bucket: nc-user-1-s3 s3api head-bucket --bucket bucket-01 (expect to see "SDSD same_stat true")
  7. Edit the account config file, for example: sudo node src/cmd/manage_nsfs account update --name <account-name> --fs_backend GPFS (it was '' and we change it to GPFS).
  8. Head the bucket again: nc-user-1-s3 s3api head-bucket --bucket bucket-01 (expect to see "SDSD same_stat false").
  • Doc added/updated
  • Tests added

@shirady shirady marked this pull request as draft January 1, 2025 14:32
@shirady shirady marked this pull request as ready for review January 2, 2025 07:08
@shirady shirady self-assigned this Jan 2, 2025
src/sdk/config_fs.js Outdated Show resolved Hide resolved
src/sdk/config_fs.js Outdated Show resolved Hide resolved
src/sdk/config_fs.js Outdated Show resolved Hide resolved
src/sdk/config_fs.js Outdated Show resolved Hide resolved
@shirady shirady force-pushed the nsfs-nc-account-id-cache-with-stat branch from 6f2284e to 0785809 Compare January 6, 2025 07:27
@shirady shirady requested a review from nadavMiz January 6, 2025 09:17
@shirady shirady requested a review from nadavMiz January 6, 2025 11:57
Signed-off-by: shirady <57721533+shirady@users.noreply.github.com>
…stat_account_config_file by access key

Signed-off-by: shirady <57721533+shirady@users.noreply.github.com>
Signed-off-by: shirady <57721533+shirady@users.noreply.github.com>
Signed-off-by: shirady <57721533+shirady@users.noreply.github.com>
1. get_identity_by_id_and_stat_file simplify and avoid the same try-catch as stat_account_config_file_by_identity
2. stat_account_config_file_by_identity avoid the nested try-catch

Signed-off-by: shirady <57721533+shirady@users.noreply.github.com>
Signed-off-by: shirady <57721533+shirady@users.noreply.github.com>
Signed-off-by: shirady <57721533+shirady@users.noreply.github.com>
Signed-off-by: shirady <57721533+shirady@users.noreply.github.com>
…_by_name)

Signed-off-by: shirady <57721533+shirady@users.noreply.github.com>
@shirady shirady force-pushed the nsfs-nc-account-id-cache-with-stat branch from d38faac to daaa60b Compare January 9, 2025 10:37
Copy link
Contributor

@nadavMiz nadavMiz left a comment

Choose a reason for hiding this comment

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

LGTM

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.

2 participants