Skip to content

Commit 57cd502

Browse files
nadavMiznadavMiz
authored
Merge pull request #8351 from nadavMiz/remove-prev-attribute
NSFS | remove previous version external attribute
2 parents a116848 + 6ffaa8a commit 57cd502

File tree

2 files changed

+8
-44
lines changed

2 files changed

+8
-44
lines changed

‎src/sdk/namespace_fs.js

Lines changed: 7 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,6 @@ const XATTR_PART_OFFSET = XATTR_NOOBAA_INTERNAL_PREFIX + 'part_offset';
5959
const XATTR_PART_SIZE = XATTR_NOOBAA_INTERNAL_PREFIX + 'part_size';
6060
const XATTR_PART_ETAG = XATTR_NOOBAA_INTERNAL_PREFIX + 'part_etag';
6161
const XATTR_VERSION_ID = XATTR_NOOBAA_INTERNAL_PREFIX + 'version_id';
62-
const XATTR_PREV_VERSION_ID = XATTR_NOOBAA_INTERNAL_PREFIX + 'prev_version_id';
6362
const XATTR_DELETE_MARKER = XATTR_NOOBAA_INTERNAL_PREFIX + 'delete_marker';
6463
const XATTR_DIR_CONTENT = XATTR_NOOBAA_INTERNAL_PREFIX + 'dir_content';
6564
const XATTR_TAG = XATTR_NOOBAA_INTERNAL_PREFIX + 'tag.';
@@ -1314,8 +1313,7 @@ class NamespaceFS {
13141313
fs_xattr = await this._assign_part_props_to_fs_xattr(fs_context, params.size, digest, offset, fs_xattr);
13151314
}
13161315
if (!part_upload && (this._is_versioning_enabled() || this._is_versioning_suspended())) {
1317-
const cur_ver_info = await this._get_version_info(fs_context, file_path);
1318-
fs_xattr = await this._assign_versions_to_fs_xattr(fs_context, cur_ver_info, stat, params.key, fs_xattr);
1316+
fs_xattr = await this._assign_versions_to_fs_xattr(stat, fs_xattr, undefined);
13191317
}
13201318
if (!part_upload && params.storage_class) {
13211319
fs_xattr = Object.assign(fs_xattr || {}, {
@@ -2175,14 +2173,11 @@ class NamespaceFS {
21752173
return fs_xattr;
21762174
}
21772175

2178-
async _assign_versions_to_fs_xattr(fs_context, prev_ver_info, new_ver_stat, key, fs_xattr, delete_marker) {
2179-
if (!prev_ver_info) prev_ver_info = await this.find_max_version_past(fs_context, key);
2180-
2176+
async _assign_versions_to_fs_xattr(new_ver_stat, fs_xattr, delete_marker) {
21812177
fs_xattr = Object.assign(fs_xattr || {}, {
21822178
[XATTR_VERSION_ID]: this._get_version_id_by_mode(new_ver_stat)
21832179
});
21842180

2185-
if (prev_ver_info) fs_xattr[XATTR_PREV_VERSION_ID] = prev_ver_info.version_id_str;
21862181
if (delete_marker) fs_xattr[XATTR_DELETE_MARKER] = delete_marker;
21872182

21882183
return fs_xattr;
@@ -2661,7 +2656,6 @@ class NamespaceFS {
26612656
// mtimeNsBigint - modified timestmap in bigint - last time the content of the file was modified
26622657
// ino - refers to the data stored in a particular location
26632658
// delete_marker - specifies if the version is a delete marker
2664-
// prev_version_id - specifies the previous version of the wanted version
26652659
// path - specifies the path to version
26662660
// if version xattr contains version info - return info by xattr
26672661
// else - it's a null version - return stat
@@ -2676,7 +2670,6 @@ class NamespaceFS {
26762670
...(ver_info_by_xattr || stat),
26772671
version_id_str,
26782672
delete_marker: stat.xattr[XATTR_DELETE_MARKER],
2679-
prev_version_id: stat.xattr[XATTR_PREV_VERSION_ID],
26802673
path: version_path
26812674
};
26822675
} catch (err) {
@@ -2732,13 +2725,12 @@ class NamespaceFS {
27322725
}
27332726

27342727
/**
2735-
* @param {nb.NativeFSContext} fs_context
2736-
* @param {string} key
2737-
* @param {string} version_id
2728+
* @param {nb.NativeFSContext} fs_context
2729+
* @param {string} key
2730+
* @param {string} version_id
27382731
* @returns {Promise<{
27392732
* version_id_str: any;
27402733
* delete_marker: string;
2741-
* prev_version_id: string;
27422734
* path: any;
27432735
* mtimeNsBigint: bigint;
27442736
* ino: number;
@@ -2857,17 +2849,13 @@ class NamespaceFS {
28572849
// condition 2 guards on situations where we don't want to try move max version past to latest
28582850
async _promote_version_to_latest(fs_context, params, deleted_version_info, latest_ver_path) {
28592851
dbg.log1('Namespace_fs._promote_version_to_latest', params, deleted_version_info, latest_ver_path);
2860-
const deleted_latest = deleted_version_info && deleted_version_info.path === latest_ver_path;
2861-
const prev_version_id = deleted_latest && deleted_version_info.prev_version_id;
28622852

28632853
let retries = config.NSFS_RENAME_RETRIES;
28642854
for (;;) {
28652855
try {
28662856
const latest_version_info = await this._get_version_info(fs_context, latest_ver_path);
28672857
if (latest_version_info) return;
2868-
const max_past_ver_info = (prev_version_id &&
2869-
(await this.get_prev_version_info(fs_context, params.key, prev_version_id))) ||
2870-
(await this.find_max_version_past(fs_context, params.key));
2858+
const max_past_ver_info = await this.find_max_version_past(fs_context, params.key);
28712859

28722860
if (!max_past_ver_info || max_past_ver_info.delete_marker) return;
28732861
// 2 - if deleted file is a delete marker and is older than max past version - no need to promote max - return
@@ -3013,16 +3001,7 @@ class NamespaceFS {
30133001
}
30143002
const file_path = this._get_version_path(params.key, delete_marker_version_id);
30153003

3016-
let fs_xattr;
3017-
if (this._is_versioning_suspended() &&
3018-
(deleted_version_info?.version_id_str === NULL_VERSION_ID)) {
3019-
fs_xattr = await this._assign_versions_to_fs_xattr(fs_context, undefined,
3020-
stat, params.key, undefined, true);
3021-
} else {
3022-
// the previous version will be the deleted version
3023-
fs_xattr = await this._assign_versions_to_fs_xattr(fs_context, deleted_version_info,
3024-
stat, params.key, undefined, true);
3025-
}
3004+
const fs_xattr = await this._assign_versions_to_fs_xattr(stat, undefined, true);
30263005
if (fs_xattr) await upload_params.target_file.replacexattr(fs_context, fs_xattr);
30273006
// create .version in case we don't have it yet
30283007
await native_fs_utils._make_path_dirs(file_path, fs_context);
@@ -3042,12 +3021,6 @@ class NamespaceFS {
30423021
}
30433022
}
30443023

3045-
async get_prev_version_info(fs_context, key, prev_version_id) {
3046-
const prev_path = this._get_version_path(key, prev_version_id);
3047-
const prev_path_info = await this._get_version_info(fs_context, prev_path);
3048-
return prev_path_info;
3049-
}
3050-
30513024
// try find prev version by hint or by iterating on .versions/ dir
30523025
async find_max_version_past(fs_context, key) {
30533026
const versions_dir = path.normalize(path.join(this.bucket_path, path.dirname(key), HIDDEN_VERSIONS_PATH));

‎src/test/unit_tests/test_bucketspace_versioning.js

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ coretest.setup({});
2121

2222
const XATTR_INTERNAL_NOOBAA_PREFIX = 'user.noobaa.';
2323
const XATTR_VERSION_ID = XATTR_INTERNAL_NOOBAA_PREFIX + 'version_id';
24-
const XATTR_PREV_VERSION_ID = XATTR_INTERNAL_NOOBAA_PREFIX + 'prev_version_id';
2524
const XATTR_DELETE_MARKER = XATTR_INTERNAL_NOOBAA_PREFIX + 'delete_marker';
2625
const NULL_VERSION_ID = 'null';
2726

@@ -916,7 +915,6 @@ mocha.describe('bucketspace namespace_fs - versioning', function() {
916915
assert.ok(is_dm);
917916
const version_path = path.join(suspended_full_path, '.versions', key_to_delete3 + '_' + latest_dm_version);
918917
const version_info = await stat_and_get_all(version_path, '');
919-
assert.equal(version_info.xattr[XATTR_PREV_VERSION_ID], prev_dm.VersionId);
920918
assert.equal(version_info.xattr[XATTR_VERSION_ID], NULL_VERSION_ID);
921919
});
922920
});
@@ -975,8 +973,6 @@ mocha.describe('bucketspace namespace_fs - versioning', function() {
975973
const max_version0 = await find_max_version_past(full_delete_path, key1, '');
976974
const cur_version_id1 = await stat_and_get_version_id(full_delete_path, key1);
977975
assert.equal(upload_res_arr[2].VersionId, cur_version_id1);
978-
const cur_ver_info = await stat_and_get_all(full_delete_path, key1);
979-
assert.equal(cur_ver_info.xattr[XATTR_PREV_VERSION_ID], max_version0);
980976
const is_dm = await is_delete_marker(full_delete_path, '', key1, max_version0);
981977
assert.ok(is_dm);
982978

@@ -1292,8 +1288,6 @@ mocha.describe('bucketspace namespace_fs - versioning', function() {
12921288

12931289
const cur_version_id1 = await stat_and_get_version_id(full_delete_path, key1);
12941290
assert.equal(upload_res_arr[2].VersionId, cur_version_id1);
1295-
const cur_ver_info = await stat_and_get_all(full_delete_path, key1);
1296-
assert.equal(cur_ver_info.xattr[XATTR_PREV_VERSION_ID], max_version0);
12971291
const is_dm = await is_delete_marker(full_delete_path, '', key1, max_version0);
12981292
assert.ok(is_dm);
12991293

@@ -2832,13 +2826,11 @@ async function compare_version_ids(full_path, key, put_result_version_id, prev_v
28322826
}
28332827
assert.equal(new_version_id, xattr_version_id);
28342828
if (prev_version_id) {
2835-
const xattr_prev_version_id = get_version_id_by_xattr(stat, true);
28362829
if (is_enabled) {
28372830
// When versioning is Enabled the version IDs are unique.
28382831
// Hence, the new version ID must be different than the previous one.
28392832
assert.notEqual(new_version_id, prev_version_id);
28402833
}
2841-
assert.equal(xattr_prev_version_id, prev_version_id);
28422834
}
28432835
return true;
28442836
}
@@ -2847,8 +2839,7 @@ function get_version_id_by_stat(stat) {
28472839
return 'mtime-' + stat.mtimeNsBigint.toString(36) + '-ino-' + stat.ino.toString(36);
28482840
}
28492841

2850-
function get_version_id_by_xattr(stat, prev) {
2851-
if (prev) return stat && stat.xattr[XATTR_PREV_VERSION_ID];
2842+
function get_version_id_by_xattr(stat) {
28522843
return (stat && stat.xattr[XATTR_VERSION_ID]) || 'null';
28532844
}
28542845

0 commit comments

Comments
 (0)