NSFS | versioning | content directory versioning - list-object + tagging operations

nadavMiz · nadavMiz · commit ccbf9f66ad84 · 2025-02-26T12:33:38.000+02:00
Signed-off-by: nadav mizrahi &lt;nadav.mizrahi16@gmail.com&gt;
diff --git a/config.js b/config.js
@@ -818,7 +818,6 @@ config.NSFS_RANDOM_DELAY_BASE = 70;
 
 config.NSFS_VERSIONING_ENABLED = true;
 config.NSFS_UPDATE_ISSUES_REPORT_ENABLED = true;
-config.NSFS_CONTENT_DIRECTORY_VERSIONING_ENABLED = false;
 
 config.NSFS_EXIT_EVENTS_TIME_FRAME_MIN = 24 * 60; // per day
 config.NSFS_MAX_EXIT_EVENTS_PER_TIME_FRAME = 10; // allow max 10 failed forks per day
diff --git a/docs/design/NsfsVersioning.md b/docs/design/NsfsVersioning.md
@@ -54,6 +54,29 @@ In the figure, each original directory contains a hidden .versions/ sub- directo
 * When A delete marker is the latest version of an object, it indicates that the object is deleted.
 * A unique version ID will be allocated to the delete marker as for regular versions.
 
+### content Dir versioning
+content dir structure has two modes:
+1. versioning disabled mode,at this mode xattr are located at the directory. we create .folder file inside the directory only if the object has data in it. in that case .folder file contains the objects body.
+example bucket tree:
+```
+bucket
+└── dir <= directory object (dir/), includes xattr
+    └──.folder <= file containing dir/ object body
+```
+2. versioning enabled / suspended mode. at this mode we always create .folder file. xatrr are located at the .folder file. the transition between the disable and enabled/disabled is lazy. meaning that we only change the key structure after a put operation for that key. non-latest versions and delete markers are saved in the directories .versions directory as `.folder-<version-id>`. versioning handling is the same as for non-directory objects. only that the object file used is the  `.folder` file
+example bucket tree:
+```
+bucket
+└── dir
+    ├── .folder <= contains both body (can be empty) and xattr - dir/
+    ├── key1 <= nested object - dir/key1
+    └── .versions
+        ├── .folder_mtime-dzdz6vlnbzblk-ino-4hbc <= version of dir/
+        ├── .folder_mtime-dzdz5vlasfsaw-ino-4hia
+        ├── key1_mtime-d6u6vlnbzklc-ino-3ehc <= version of dir/key1
+        └── key1_mtime-d6u6vnztsiyo-ino-3eik
+```   
+
 ### Posix safe rename
 
 #### In the following cases NooBaa will move files between a directory and its .versions/ directory:
diff --git a/src/sdk/namespace_fs.js b/src/sdk/namespace_fs.js
@@ -736,12 +736,12 @@ class NamespaceFS {
                 /**
                  * @param {fs.Dirent} ent
                  */
-                const process_entry = async ent => {
+                const process_entry = async (ent, is_disabled_dir_content) => {
                     // dbg.log0('process_entry', dir_key, ent.name);
-                    if ((!ent.name.startsWith(prefix_ent) ||
+                    if (!ent.name.startsWith(prefix_ent) ||
                         ent.name < marker_curr ||
                         ent.name === this.get_bucket_tmpdir_name() ||
-                        ent.name === config.NSFS_FOLDER_OBJECT_NAME) ||
+                        (ent.name === config.NSFS_FOLDER_OBJECT_NAME && is_disabled_dir_content) ||
                         this._is_hidden_version_path(ent.name)) {
                         return;
                     }
@@ -756,7 +756,7 @@ class NamespaceFS {
                         };
                     } else {
                         r = {
-                            key: this._get_entry_key(dir_key, ent, isDir),
+                            key: this._get_entry_key(dir_key, ent, isDir, is_disabled_dir_content),
                             common_prefix: isDir,
                             is_latest: true
                         };
@@ -781,8 +781,8 @@ class NamespaceFS {
 
                 // insert dir object to objects list if its key is lexicographicly bigger than the key marker &&
                 // no delimiter OR prefix is the current directory entry
-                const is_dir_content = cached_dir.stat.xattr && cached_dir.stat.xattr[XATTR_DIR_CONTENT];
-                if (is_dir_content && dir_key > key_marker && (!delimiter || dir_key === prefix)) {
+                const is_disabled_dir_content = cached_dir.stat.xattr && cached_dir.stat.xattr[XATTR_DIR_CONTENT];
+                if (is_disabled_dir_content && dir_key > key_marker && (!delimiter || dir_key === prefix)) {
                     const r = { key: dir_key, common_prefix: false };
                     await insert_entry_to_results_arr(r);
                 }
@@ -835,7 +835,7 @@ class NamespaceFS {
                         if (ent.name === config.NSFS_FOLDER_OBJECT_NAME && dir_key === marker_dir) {
                             continue;
                         }
-                        await process_entry(ent);
+                        await process_entry(ent, is_disabled_dir_content);
                         // since we traverse entries in sorted order,
                         // we can break as soon as enough keys are collected.
                         if (is_truncated) break;
@@ -883,6 +883,17 @@ class NamespaceFS {
                 }
             };
 
+            const format_key_name = obj_info => {
+                if (this._is_hidden_version_path(obj_info.key)) {
+                    obj_info.key = path.normalize(obj_info.key.replace(HIDDEN_VERSIONS_PATH + '/', ''));
+                    obj_info.key = _get_filename(obj_info.key);
+                    set_latest_delete_marker(obj_info);
+                }
+                if (obj_info.key.endsWith(config.NSFS_FOLDER_OBJECT_NAME)) {
+                    obj_info.key = obj_info.key.slice(0, -config.NSFS_FOLDER_OBJECT_NAME.length);
+                }
+            };
+
             const prefix_dir_key = prefix.slice(0, prefix.lastIndexOf('/') + 1);
             await process_dir(prefix_dir_key);
             await Promise.all(results.map(async r => {
@@ -908,11 +919,7 @@ class NamespaceFS {
                     if (!list_versions && obj_info.delete_marker) {
                         continue;
                     }
-                    if (this._is_hidden_version_path(obj_info.key)) {
-                        obj_info.key = path.normalize(obj_info.key.replace(HIDDEN_VERSIONS_PATH + '/', ''));
-                        obj_info.key = _get_filename(obj_info.key);
-                        set_latest_delete_marker(obj_info);
-                    }
+                    format_key_name(obj_info);
                     res.objects.push(obj_info);
                     previous_key = obj_info.key;
                 }
@@ -1215,7 +1222,7 @@ class NamespaceFS {
         try {
             await this._check_path_in_bucket_boundaries(fs_context, file_path);
 
-            if (this.should_use_empty_content_dir_optimization() && this.empty_dir_content_flow(file_path, params)) {
+            if (this._is_versioning_disabled() && this.empty_dir_content_flow(file_path, params)) {
                 const content_dir_info = await this._create_empty_dir_content(fs_context, params, file_path);
                 return content_dir_info;
             }
@@ -1353,8 +1360,7 @@ class NamespaceFS {
         const part_upload = file_path === upload_path;
         const same_inode = params.copy_source && copy_res === COPY_STATUS_ENUM.SAME_INODE;
         const should_replace_xattr = params.copy_source ? copy_res === COPY_STATUS_ENUM.FALLBACK : true;
-        const is_dir_content_optimized_flow = this._is_directory_content(file_path, params.key) &&
-            this.should_use_empty_content_dir_optimization();
+        const is_disabled_dir_content = this._is_directory_content(file_path, params.key) && this._is_versioning_disabled();
 
         const stat = await target_file.stat(fs_context);
         this._verify_encryption(params.encryption, this._get_encryption_info(stat));
@@ -1397,21 +1403,20 @@ class NamespaceFS {
                 });
             }
         }
-        if (fs_xattr && !is_dir_content_optimized_flow && should_replace_xattr) {
+        if (fs_xattr && !is_disabled_dir_content && should_replace_xattr) {
             await target_file.replacexattr(fs_context, fs_xattr);
         }
         // fsync
         if (config.NSFS_TRIGGER_FSYNC) await target_file.fsync(fs_context);
         dbg.log1('NamespaceFS._finish_upload:', open_mode, file_path, upload_path, fs_xattr);
 
         if (!same_inode && !part_upload) {
-            await this._move_to_dest(fs_context, upload_path, file_path, target_file, open_mode, params.key,
-                is_dir_content_optimized_flow);
+            await this._move_to_dest(fs_context, upload_path, file_path, target_file, open_mode, params.key);
         }
 
         // when object is a dir, xattr are set on the folder itself and the content is in .folder file
         // we still should put the xattr if copy is link/same inode because we put the xattr on the directory
-        if (is_dir_content_optimized_flow) {
+        if (is_disabled_dir_content) {
             await this._assign_dir_content_to_xattr(fs_context, fs_xattr, { ...params, size: stat.size }, copy_xattr);
         }
         stat.xattr = { ...stat.xattr, ...fs_xattr };
@@ -1439,14 +1444,14 @@ class NamespaceFS {
     }
 
     // move to dest GPFS (wt) / POSIX (w / undefined) - non part upload
-    async _move_to_dest(fs_context, source_path, dest_path, target_file, open_mode, key, is_dir_content_optimized_flow) {
-        dbg.log2('_move_to_dest', fs_context, source_path, dest_path, target_file, open_mode, key, is_dir_content_optimized_flow);
+    async _move_to_dest(fs_context, source_path, dest_path, target_file, open_mode, key) {
+        dbg.log2('_move_to_dest', fs_context, source_path, dest_path, target_file, open_mode, key);
         let retries = config.NSFS_RENAME_RETRIES;
         // will retry renaming a file in case of parallel deleting of the destination path
         for (;;) {
             try {
                 await native_fs_utils._make_path_dirs(dest_path, fs_context);
-                if (this._is_versioning_disabled() || is_dir_content_optimized_flow) {
+                if (this._is_versioning_disabled()) {
                     if (open_mode === 'wt') {
                         await target_file.linkfileat(fs_context, dest_path);
                     } else {
@@ -1989,7 +1994,7 @@ class NamespaceFS {
             if (is_key_dir_path && !params.key.endsWith('/')) {
                 return {};
             }
-            if (this._is_versioning_disabled() || (is_key_dir_path && this.should_use_empty_content_dir_optimization())) {
+            if (this._is_versioning_disabled()) {
                 // TODO- Directory object (key/) is currently can't co-exist while key (without slash) exists. see -https://github.com/noobaa/noobaa-core/issues/8320
                 await this._delete_single_object(fs_context, file_path, params);
             } else {
@@ -2111,13 +2116,8 @@ class NamespaceFS {
 
     async delete_object_tagging(params, object_sdk) {
         dbg.log0('NamespaceFS.delete_object_tagging:', params);
-        let file_path;
-        if (params.version_id && this._is_versioning_enabled()) {
-            file_path = this._get_version_path(params.key, params.version_id);
-        } else {
-            file_path = this._get_file_path(params);
-        }
         const fs_context = this.prepare_fs_context(object_sdk);
+        const file_path = await this._find_version_path(fs_context, params, true);
         try {
             await this._clear_user_xattr(fs_context, file_path, XATTR_TAG);
         } catch (err) {
@@ -2133,13 +2133,8 @@ class NamespaceFS {
         for (const [xattr_key, xattr_value] of Object.entries(tagging)) {
               fs_xattr[XATTR_TAG + xattr_key] = xattr_value;
         }
-        let file_path;
-        if (params.version_id && this._is_versioning_enabled()) {
-            file_path = this._get_version_path(params.key, params.version_id);
-        } else {
-            file_path = this._get_file_path(params);
-        }
         const fs_context = this.prepare_fs_context(object_sdk);
+        const file_path = await this._find_version_path(fs_context, params, true);
         dbg.log0('NamespaceFS.put_object_tagging: fs_xattr ', fs_xattr, 'file_path :', file_path);
         try {
             // remove existng tag before putting new tags
@@ -2439,8 +2434,8 @@ class NamespaceFS {
      * @param {fs.Dirent} ent
      * @returns {string}
      */
-    _get_entry_key(dir_key, ent, isDir) {
-        if (ent.name === config.NSFS_FOLDER_OBJECT_NAME) return dir_key;
+    _get_entry_key(dir_key, ent, isDir, is_disabled_dir_content) {
+        if (ent.name === config.NSFS_FOLDER_OBJECT_NAME && is_disabled_dir_content) return dir_key;
         return dir_key + ent.name + (isDir ? '/' : '');
     }
 
@@ -2450,7 +2445,6 @@ class NamespaceFS {
      * @returns {string}
      */
      _get_version_entry_key(dir_key, ent) {
-        if (ent.name === config.NSFS_FOLDER_OBJECT_NAME) return dir_key;
         return dir_key + HIDDEN_VERSIONS_PATH + '/' + ent.name;
     }
 
@@ -2765,9 +2759,6 @@ class NamespaceFS {
         return is_dir_content && params.size === 0;
     }
 
-    should_use_empty_content_dir_optimization() {
-        return this._is_versioning_disabled() || !config.NSFS_CONTENT_DIRECTORY_VERSIONING_ENABLED;
-    }
     /**
      * returns if should force md5 calculation for the bucket/account.
      * first check if defined for bucket / account, if not use global default
diff --git a/src/test/unit_tests/coretest.js b/src/test/unit_tests/coretest.js
@@ -27,7 +27,6 @@ config.test_mode = true;
 config.NODES_FREE_SPACE_RESERVE = 10 * 1024 * 1024;
 config.NSFS_VERSIONING_ENABLED = true;
 config.OBJECT_SDK_BUCKET_CACHE_EXPIRY_MS = 1;
-config.NSFS_CONTENT_DIRECTORY_VERSIONING_ENABLED = true;
 
 config.ROOT_KEY_MOUNT = '/tmp/noobaa-server/root_keys';
 
diff --git a/src/test/unit_tests/jest_tests/test_versioning_concurrency.test.js b/src/test/unit_tests/jest_tests/test_versioning_concurrency.test.js
@@ -10,8 +10,6 @@ const buffer_utils = require('../../../util/buffer_utils');
 const { TMP_PATH, IS_GPFS, TEST_TIMEOUT } = require('../../system_tests/test_utils');
 const { crypto_random_string } = require('../../../util/string_utils');
 const endpoint_stats_collector = require('../../../sdk/endpoint_stats_collector');
-const config = require('../../../../config');
-config.NSFS_CONTENT_DIRECTORY_VERSIONING_ENABLED = true;
 
 function make_dummy_object_sdk(nsfs_config, uid, gid) {
     return {
@@ -580,8 +578,7 @@ describe('test versioning concurrency', () => {
         expect(successful_operations).toHaveLength(num_of_concurrency);
         expect(failed_operations).toHaveLength(0);
         const versions = await nsfs.list_object_versions({ bucket: bucket }, DUMMY_OBJECT_SDK);
-        //TODO should be num_of_concurrency + 1 (the null version). list-object-version currently ignores the latest .folder file
-        expect(versions.objects.length).toBe(num_of_concurrency);
+        expect(versions.objects.length).toBe(num_of_concurrency + 1);
     }, TEST_TIMEOUT);
 });
 
diff --git a/src/test/unit_tests/nc_coretest.js b/src/test/unit_tests/nc_coretest.js
@@ -205,7 +205,6 @@ async function config_dir_setup() {
         // DO NOT CHANGE - setting VACCUM_ANALYZER_INTERVAL=1 needed for failing the tests
         // in case where vaccumAnalyzer is being called before setting process.env.NC_NSFS_NO_DB_ENV = 'true' on nsfs.js
         VACCUM_ANALYZER_INTERVAL: 1,
-        NSFS_CONTENT_DIRECTORY_VERSIONING_ENABLED: true
     }));
     await fs.promises.mkdir(FIRST_BUCKET_PATH, { recursive: true });
 }
diff --git a/src/test/unit_tests/test_bucketspace_versioning.js b/src/test/unit_tests/test_bucketspace_versioning.js
@@ -451,6 +451,7 @@ mocha.describe('bucketspace namespace_fs - versioning', function() {
             await s3_uid6.putObject({ Bucket: content_dir_bucket_name, Key: head_object_key, Body: body1 });
             await s3_uid6.putObject({ Bucket: content_dir_bucket_name, Key: get_object_key, Body: body1 });
             await s3_uid6.putObject({ Bucket: content_dir_bucket_name, Key: delete_latest_object_key, Body: body1 });
+            await s3_uid6.putObject({ Bucket: content_dir_bucket_name, Key: delete_latest_object_key_suspended, Body: body1 });
 
             await s3_uid6.putBucketVersioning({ Bucket: content_dir_bucket_name, VersioningConfiguration: { MFADelete: 'Disabled', Status: 'Enabled' } });
         });
@@ -639,7 +640,63 @@ mocha.describe('bucketspace namespace_fs - versioning', function() {
 
             const get_res = await s3_uid6.getObject({ Bucket: content_dir_bucket_name, Key: key });
             assert.equal(get_res.VersionId, res2.VersionId);
+        });
+
+        mocha.it('content directory - object tagging', async function() {
+            const dir_tagging_key = 'dir_tagging/';
+            const tag_set1 = { TagSet: [{ Key: "key1", Value: "Value1" }] };
+            const tag_set2 = { TagSet: [{ Key: "key2", Value: "Value2" }] };
 
+            await s3_uid6.putBucketVersioning({ Bucket: content_dir_bucket_name, VersioningConfiguration: { MFADelete: 'Disabled', Status: 'Enabled' } });
+            const res_put = await s3_uid6.putObject({ Bucket: content_dir_bucket_name, Key: dir_tagging_key, Body: body1 });
+            await s3_uid6.putObject({ Bucket: content_dir_bucket_name, Key: dir_tagging_key, Body: body1 });
+            const version_id = res_put.VersionId;
+
+            await s3_uid6.putObjectTagging({ Bucket: content_dir_bucket_name, Key: dir_tagging_key, Tagging: tag_set1 });
+            let res = await s3_uid6.getObjectTagging({ Bucket: content_dir_bucket_name, Key: dir_tagging_key });
+            assert.deepEqual(res.TagSet, tag_set1.TagSet);
+
+            await s3_uid6.putObjectTagging({ Bucket: content_dir_bucket_name, Key: dir_tagging_key,
+                Tagging: tag_set2, VersionId: version_id });
+            res = await s3_uid6.getObjectTagging({ Bucket: content_dir_bucket_name, Key: dir_tagging_key });
+            assert.notDeepEqual(res.TagSet, tag_set2);
+
+            const version_res = await s3_uid6.getObjectTagging({ Bucket: content_dir_bucket_name,
+                Key: dir_tagging_key, VersionId: version_id });
+            assert.deepEqual(version_res.TagSet, tag_set2.TagSet);
+
+            await s3_uid6.deleteObjectTagging({ Bucket: content_dir_bucket_name, Key: dir_tagging_key });
+            res = await s3_uid6.getObjectTagging({ Bucket: content_dir_bucket_name, Key: dir_tagging_key });
+            assert.equal(res.TagSet.length, 0);
+
+            await s3_uid6.deleteObjectTagging({ Bucket: content_dir_bucket_name, Key: dir_tagging_key, VersionId: version_id });
+            res = await s3_uid6.getObjectTagging({ Bucket: content_dir_bucket_name, Key: dir_tagging_key, VersionId: version_id });
+            assert.equal(res.TagSet.length, 0);
+        });
+
+        mocha.it('content directory - list object', async function() {
+            const res = await s3_uid6.listObjects({ Bucket: content_dir_bucket_name });
+            assert.equal(res.Contents.length, 10);
+        });
+
+        mocha.it('content directory - list object versions', async function() {
+            const key = "list_key/";
+            const version_ids = new Set();
+            let push_res = await s3_uid6.putObject({ Bucket: content_dir_bucket_name, Key: key, Body: body1 });
+            version_ids.add(push_res.VersionId);
+            push_res = await s3_uid6.putObject({ Bucket: content_dir_bucket_name, Key: key, Body: body1 });
+            version_ids.add(push_res.VersionId);
+            push_res = await s3_uid6.putObject({ Bucket: content_dir_bucket_name, Key: key, Body: body1 });
+            version_ids.add(push_res.VersionId);
+            const list_res = await s3_uid6.listObjectVersions({ Bucket: content_dir_bucket_name });
+            const key_list_versions = list_res.Versions.filter(v => v.Key === key);
+            assert.equal(key_list_versions.length, version_ids.size);
+            key_list_versions.forEach(v => {
+                assert(version_ids.has(v.VersionId));
+                if (v.VersionId === push_res.VersionId) {
+                    assert.equal(v.IsLatest, true);
+                }
+            });
         });
     });
 
diff --git a/src/test/unit_tests/test_namespace_fs.js b/src/test/unit_tests/test_namespace_fs.js
@@ -42,8 +42,6 @@ const DEFAULT_FS_CONFIG = get_process_fs_context();
 const empty_data = crypto.randomBytes(0);
 const empty_stream = () => buffer_utils.buffer_to_read_stream(empty_data);
 
-config.NSFS_CONTENT_DIRECTORY_VERSIONING_ENABLED = true;
-
 function make_dummy_object_sdk(config_root) {
     return {
         requesting_account: {

Original file line number	Diff line number	Diff line change
`@@ -205,7 +205,6 @@ async function config_dir_setup() {`
`205`	`205`	`// DO NOT CHANGE - setting VACCUM_ANALYZER_INTERVAL=1 needed for failing the tests`
`206`	`206`	`// in case where vaccumAnalyzer is being called before setting process.env.NC_NSFS_NO_DB_ENV = 'true' on nsfs.js`
`207`	`207`	`VACCUM_ANALYZER_INTERVAL: 1,`
`208`		`- NSFS_CONTENT_DIRECTORY_VERSIONING_ENABLED: true`
`209`	`208`	`}));`
`210`	`209`	`await fs.promises.mkdir(FIRST_BUCKET_PATH, { recursive: true });`
`211`	`210`	`}`