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 : S3 rm with --recursive option does not delete all the objects #8317

Closed
wants to merge 1 commit into from

Conversation

naveenpaul1
Copy link
Contributor

Explain the changes

  1. Issue is with list_object api, list_object is failing with specific dir strucute and not returning complete items in that bucket.
  2. dir_key and marker_dir comparison will fail when there are dir with structure slimier to "-/migrateDir.popFSDir.3803140/" and "/migrateDir.popFSDir.3803140.OldSet/" because "/" considered greater than "." to fix this all the backslash is replaced with space. This updated marker and dir_key used only for comparison.

Dir structure like this is will fail

/populatefs_dir
--/migrateDir.popFSDir.3803140
----/sparseDir
-----lnk.0.qqqqqqqqqqqqqq
-----lnk.1.qqqqqqqqqqqqqq
-----lnk.2.qqqqqqqqqqqqqq
--/migrateDir.popFSDir.3803140.OldSet
----/sparseDir
-----lnk.0.qqqqqqqqqqqqqq
-----lnk.1.qqqqqqqqqqqqqq
-----lnk.2.qqqqqqqqqqqqqq

Issues: Fixed #xxx / Gap #xxx

  1. S3 upload is successful for a data set (mix of all type of objects, sparse files, files with long name, compressed files etc)
  2. When download via "aws-alias s3 rm s3://bucket-10909 --recursive" , this still displays the left over directories and objects in the bucket. I checked from the FS that the dataset still exists there.

#8197

Testing Instructions:

  1. run NC_CORETEST=true node ./node_modules/.bin/mocha ./src/test/unit_tests/test_namespace_fs.js
  • Doc added/updated
  • Tests added

@pull-request-size pull-request-size bot added size/M and removed size/S labels Aug 29, 2024
@naveenpaul1 naveenpaul1 force-pushed the object_delete_issue branch 2 times, most recently from bc10334 to 4e5d502 Compare August 30, 2024 09:23
@naveenpaul1 naveenpaul1 requested review from romayalon and guymguym and removed request for romayalon August 30, 2024 09:27
// dir_key and marker_dir comparison will fail when there are folder with structure slimier to
// "dir_prfix.12345/" and "dir_prfix.12345.old/" because "/" considered greater than "." to fix this
// all the backslash is replaced with space. This updated marker and dir_key used only for comparison.
const updated_marker_dir = marker_dir.replaceAll('/', ' ');
Copy link
Contributor

Choose a reason for hiding this comment

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

I think that instead of replacing the value of / we need to have a better comparison function, instead of comparing dir_key < marker_dir and so on, we will call that comparison function. WDYT?

src/test/unit_tests/test_namespace_fs.js Show resolved Hide resolved
@@ -258,6 +258,11 @@ function get_entry_name(e) {
return e.name;
}

function get_entry_for_sorting(e) {
const check = e.key.includes('.') ? e.key.slice(e.key.indexOf(".")).includes('/') : false;
Copy link
Contributor

Choose a reason for hiding this comment

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

is '.' the only special char that causing this issue?

@naveenpaul1
Copy link
Contributor Author

PR : 8324 have few overlapping changes, Will update this PR once 8324 is merged,

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