Skip to content

Commit 2c23a0c

Browse files
romayalonromayalon
committed
code review fixes
Signed-off-by: Romy <[email protected]>
1 parent 46d8b9f commit 2c23a0c

File tree

1 file changed

+18
-14
lines changed

1 file changed

+18
-14
lines changed

‎src/sdk/namespace_fs.js

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1467,7 +1467,7 @@ class NamespaceFS {
14671467
} catch (err) {
14681468
retries -= 1;
14691469
const should_retry = native_fs_utils.should_retry_link_unlink(is_gpfs, err);
1470-
dbg.warn(`NamespaceFS._move_to_dest_version retrying retries=${retries} should_retry=${should_retry}` +
1470+
dbg.warn(`NamespaceFS._move_to_dest_version error: retries=${retries} should_retry=${should_retry}` +
14711471
` new_ver_tmp_path=${new_ver_tmp_path} latest_ver_path=${latest_ver_path}`, err);
14721472
if (!should_retry || retries <= 0) throw err;
14731473
await P.delay(get_random_delay(config.NSFS_RANDOM_DELAY_BASE, 0, 50));
@@ -2696,7 +2696,7 @@ class NamespaceFS {
26962696
* 1. if version_id is not defined, it returns the key file
26972697
* 2. else,
26982698
* 2.1. check version format
2699-
* 2.2. check if latest version exists and it matches the versio_id parameter the latest version path returns
2699+
* 2.2. check if the latest version exists and it matches the version_id parameter the latest version path returns
27002700
* 2.3. else, return the version path under .versions/
27012701
* @param {import('./nb').NativeFSContext} fs_context
27022702
* @param {{key: string, version_id?: string}} params
@@ -2746,6 +2746,11 @@ class NamespaceFS {
27462746
}
27472747

27482748
/**
2749+
* _delete_single_object_versioned does the following -
2750+
* if the deleted version is the latest - try to delete it from the latest version location
2751+
* if the deleted version is in .versions/ - unlink the version
2752+
* we call check_version_moved() in case of concurrent puts, the version might move to .versions/
2753+
* if the version moved we will retry
27492754
* @param {nb.NativeFSContext} fs_context
27502755
* @param {string} key
27512756
* @param {string} version_id
@@ -2758,7 +2763,6 @@ class NamespaceFS {
27582763
* latest?: boolean;
27592764
* }>}
27602765
*/
2761-
// we can use this function when versioning is enabled or suspended
27622766
async _delete_single_object_versioned(fs_context, key, version_id) {
27632767
let retries = config.NSFS_RENAME_RETRIES;
27642768
const is_gpfs = native_fs_utils._is_gpfs(fs_context);
@@ -2778,15 +2782,15 @@ class NamespaceFS {
27782782
const bucket_tmp_dir_path = this.get_bucket_tmpdir_full_path();
27792783
await native_fs_utils.safe_unlink(fs_context, file_path, version_info,
27802784
gpfs_options?.delete_version, bucket_tmp_dir_path);
2781-
await this._check_version_moved(fs_context, key, version_id, file_path);
2785+
await this._check_version_moved(fs_context, key, version_id);
27822786
return { ...version_info, latest: true };
27832787
} else {
27842788
await native_fs_utils.unlink_ignore_enoent(fs_context, file_path);
2785-
await this._check_version_moved(fs_context, key, version_id, file_path);
2789+
await this._check_version_moved(fs_context, key, version_id);
27862790
}
27872791
return version_info;
27882792
} catch (err) {
2789-
dbg.warn(`NamespaceFS._delete_single_object_versioned: retrying retries=${retries} file_path=${file_path}`, err);
2793+
dbg.warn(`NamespaceFS._delete_single_object_versioned error: retries=${retries} file_path=${file_path}`, err);
27902794
retries -= 1;
27912795
// there are a few concurrency scenarios that might happen we should retry for -
27922796
// 1. the version id is the latest, concurrent put will might move the version id from being the latest to .versions/ -
@@ -2899,14 +2903,14 @@ class NamespaceFS {
28992903
max_past_ver_info, bucket_tmp_dir_path);
29002904
break;
29012905
} catch (err) {
2906+
dbg.warn(`NamespaceFS: _promote_version_to_latest failed error: retries=${retries}`, err);
29022907
retries -= 1;
29032908
if (retries <= 0) throw err;
29042909
if (!native_fs_utils._is_gpfs(fs_context) && err.code === 'EEXIST') {
29052910
dbg.warn('Namespace_fs._delete_version_id: latest version exist - skipping');
29062911
return;
29072912
}
29082913
if (err.code !== 'ENOENT') throw err;
2909-
dbg.warn(`NamespaceFS: _promote_version_to_latest failed retries=${retries}`, err);
29102914
await P.delay(get_random_delay(config.NSFS_RANDOM_DELAY_BASE, 0, 50));
29112915
}
29122916
}
@@ -3003,9 +3007,9 @@ class NamespaceFS {
30033007
gpfs_options?.delete_version, bucket_tmp_dir_path);
30043008
break;
30053009
} catch (err) {
3010+
dbg.warn(`NamespaceFS._delete_null_version_from_versions_directory error: retries=${retries} null_versioned_path=${null_versioned_path}`, err);
30063011
retries -= 1;
30073012
if (retries <= 0 || !native_fs_utils.should_retry_link_unlink(is_gpfs, err)) throw err;
3008-
dbg.warn(`NamespaceFS._delete_null_version_from_versions_directory Retrying retries=${retries} null_versioned_path=${null_versioned_path}`, err);
30093013
await P.delay(get_random_delay(config.NSFS_RANDOM_DELAY_BASE, 0, 50));
30103014
} finally {
30113015
if (gpfs_options) await this._close_files_gpfs(fs_context, gpfs_options.delete_version, undefined, true);
@@ -3039,13 +3043,13 @@ class NamespaceFS {
30393043
await nb_native().fs.rename(fs_context, upload_params.upload_path, file_path);
30403044
return delete_marker_version_id;
30413045
} catch (err) {
3046+
dbg.warn(`NamespaceFS: _create_delete_marker failed error: retries=${retries}`, err);
30423047
retries -= 1;
30433048
if (retries <= 0) throw err;
30443049
if (err.code === 'EEXIST') {
30453050
dbg.warn(`NamespaceFS: _create_delete_marker already exists, success`, err);
30463051
return delete_marker_version_id;
30473052
}
3048-
dbg.warn(`NamespaceFS: _create_delete_marker failed retries=${retries}`, err);
30493053
await P.delay(get_random_delay(config.NSFS_RANDOM_DELAY_BASE, 0, 50));
30503054
} finally {
30513055
if (upload_params) await this.complete_object_upload_finally(undefined, undefined, upload_params.target_file, fs_context);
@@ -3159,18 +3163,18 @@ class NamespaceFS {
31593163

31603164
/**
31613165
* _check_version_moved recieves key and version_id and checks if the version still exists in one of the optional locations
3162-
* latest version location or .versions
3166+
* latest version location or .versions/ directory
31633167
* @param {import('./nb').NativeFSContext} fs_context
31643168
* @param {string} key
31653169
* @param {string} version_id
31663170
*/
31673171
async _check_version_moved(fs_context, key, version_id) {
31683172
const latest_version_path = this._get_file_path({ key });
3169-
const verisoned_path = this._get_version_path(key, version_id);
3170-
const verisoned_path_info = await this._get_version_info(fs_context, verisoned_path);
3171-
if (verisoned_path_info) throw error_utils.new_error_code('VERSION_MOVED', `version file moved from latest ${latest_version_path} to .versions/ ${verisoned_path}, retrying`);
3173+
const versioned_path = this._get_version_path(key, version_id);
3174+
const versioned_path_info = await this._get_version_info(fs_context, versioned_path);
3175+
if (versioned_path_info) throw error_utils.new_error_code('VERSION_MOVED', `version file moved from latest ${latest_version_path} to .versions/ ${versioned_path}, retrying`);
31723176
const latest_ver_info = await this._get_version_info(fs_context, latest_version_path);
3173-
if (latest_ver_info && latest_ver_info.version_id_str === version_id) throw error_utils.new_error_code('VERSION_MOVED', `version file moved from .versions/ ${verisoned_path} to latest ${latest_version_path}, retrying`);
3177+
if (latest_ver_info && latest_ver_info.version_id_str === version_id) throw error_utils.new_error_code('VERSION_MOVED', `version file moved from .versions/ ${versioned_path} to latest ${latest_version_path}, retrying`);
31743178
}
31753179

31763180
async _throw_if_storage_class_not_supported(storage_class) {

0 commit comments

Comments
 (0)