Skip to content

Commit aca3c0b

Browse files
nadavMiznadavMiz
authored
Merge pull request #8502 from nadavMiz/versioning-concurrency-conditions
NSFS | versioning | remove seperation between GPFS and POSIX errors for concurrency retries
2 parents 07f1582 + d095c3a commit aca3c0b

File tree

2 files changed

+12
-16
lines changed

2 files changed

+12
-16
lines changed

‎src/sdk/namespace_fs.js

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -934,7 +934,6 @@ class NamespaceFS {
934934
let stat;
935935
let isDir;
936936
let retries = (this._is_versioning_enabled() || this._is_versioning_suspended()) ? config.NSFS_RENAME_RETRIES : 0;
937-
const is_gpfs = native_fs_utils._is_gpfs(fs_context);
938937
try {
939938
for (;;) {
940939
try {
@@ -962,7 +961,7 @@ class NamespaceFS {
962961
} catch (err) {
963962
dbg.warn(`NamespaceFS.read_object_md: retrying retries=${retries} file_path=${file_path}`, err);
964963
retries -= 1;
965-
if (retries <= 0 || !native_fs_utils.should_retry_link_unlink(is_gpfs, err)) throw err;
964+
if (retries <= 0 || !native_fs_utils.should_retry_link_unlink(err)) throw err;
966965
await P.delay(get_random_delay(config.NSFS_RANDOM_DELAY_BASE, 0, 50));
967966
}
968967
}
@@ -1002,7 +1001,6 @@ class NamespaceFS {
10021001
try {
10031002
await this._load_bucket(params, fs_context);
10041003
let retries = (this._is_versioning_enabled() || this._is_versioning_suspended()) ? config.NSFS_RENAME_RETRIES : 0;
1005-
const is_gpfs = native_fs_utils._is_gpfs(fs_context);
10061004
let stat;
10071005
for (;;) {
10081006
try {
@@ -1033,7 +1031,7 @@ class NamespaceFS {
10331031
file = null;
10341032
}
10351033
retries -= 1;
1036-
if (retries <= 0 || !native_fs_utils.should_retry_link_unlink(is_gpfs, err)) {
1034+
if (retries <= 0 || !native_fs_utils.should_retry_link_unlink(err)) {
10371035
new NoobaaEvent(NoobaaEvent.OBJECT_GET_FAILED).create_event(params.key,
10381036
{bucket_path: this.bucket_path, object_name: params.key}, err);
10391037
throw err;
@@ -1526,7 +1524,7 @@ class NamespaceFS {
15261524
break;
15271525
} catch (err) {
15281526
retries -= 1;
1529-
const should_retry = native_fs_utils.should_retry_link_unlink(is_gpfs, err);
1527+
const should_retry = native_fs_utils.should_retry_link_unlink(err);
15301528
dbg.warn(`NamespaceFS._move_to_dest_version error: retries=${retries} should_retry=${should_retry}` +
15311529
` new_ver_tmp_path=${new_ver_tmp_path} latest_ver_path=${latest_ver_path}`, err);
15321530
if (!should_retry || retries <= 0) throw err;
@@ -2837,7 +2835,6 @@ class NamespaceFS {
28372835
*/
28382836
async _delete_single_object_versioned(fs_context, key, version_id) {
28392837
let retries = config.NSFS_RENAME_RETRIES;
2840-
const is_gpfs = native_fs_utils._is_gpfs(fs_context);
28412838
const latest_version_path = this._get_file_path({ key });
28422839
for (;;) {
28432840
let file_path;
@@ -2874,11 +2871,11 @@ class NamespaceFS {
28742871
// there are a few concurrency scenarios that might happen we should retry for -
28752872
// 1. the version id is the latest, concurrent put will might move the version id from being the latest to .versions/ -
28762873
// will throw safe unlink failed on non matching fd (on GPFS) or inode/mtime (on POSIX).
2877-
// 2. the version id is the second latest and stays under .versions/ - on concurrent delete of the latest,
2874+
// 2. the version id is the second latest and stays under .versions/ - on concurrent delete of the latest,
28782875
// the version id might move to be the latest and we will get ENOENT
2879-
// 3. concurrent delete of this version - will get ENOENT, doing a retry will return successfully
2876+
// 3. concurrent delete of this version - will get ENOENT, doing a retry will return successfully
28802877
// after we will see that the version was already deleted
2881-
if (retries <= 0 || !native_fs_utils.should_retry_link_unlink(is_gpfs, err)) throw err;
2878+
if (retries <= 0 || !native_fs_utils.should_retry_link_unlink(err)) throw err;
28822879
await P.delay(get_random_delay(config.NSFS_RANDOM_DELAY_BASE, 0, 50));
28832880
} finally {
28842881
if (gpfs_options) await this._close_files_gpfs(fs_context, gpfs_options.delete_version, undefined, true);
@@ -3053,7 +3050,7 @@ class NamespaceFS {
30533050
} catch (err) {
30543051
dbg.warn(`NamespaceFS._delete_latest_version error: retries=${retries} latest_ver_path=${latest_ver_path}`, err);
30553052
retries -= 1;
3056-
if (retries <= 0 || !native_fs_utils.should_retry_link_unlink(is_gpfs, err)) throw err;
3053+
if (retries <= 0 || !native_fs_utils.should_retry_link_unlink(err)) throw err;
30573054
await P.delay(get_random_delay(config.NSFS_RANDOM_DELAY_BASE, 0, 50));
30583055
} finally {
30593056
if (gpfs_options) await this._close_files_gpfs(fs_context, gpfs_options.delete_version, undefined, true);
@@ -3069,9 +3066,8 @@ class NamespaceFS {
30693066

30703067
// We can have only one versioned object with null version ID per key.
30713068
// It can be latest version, old version in .version/ directory or delete marker
3072-
// This function removes an object version or delete marker with a null version ID inside .version/ directory
3069+
// This function removes an object version or delete marker with a null version ID inside .version/ directory
30733070
async _delete_null_version_from_versions_directory(key, fs_context) {
3074-
const is_gpfs = native_fs_utils._is_gpfs(fs_context);
30753071
const null_versioned_path = this._get_version_path(key, NULL_VERSION_ID);
30763072
await this._check_path_in_bucket_boundaries(fs_context, null_versioned_path);
30773073
let gpfs_options;
@@ -3091,7 +3087,7 @@ class NamespaceFS {
30913087
} catch (err) {
30923088
dbg.warn(`NamespaceFS._delete_null_version_from_versions_directory error: retries=${retries} null_versioned_path=${null_versioned_path}`, err);
30933089
retries -= 1;
3094-
if (retries <= 0 || !native_fs_utils.should_retry_link_unlink(is_gpfs, err)) throw err;
3090+
if (retries <= 0 || !native_fs_utils.should_retry_link_unlink(err)) throw err;
30953091
await P.delay(get_random_delay(config.NSFS_RANDOM_DELAY_BASE, 0, 50));
30963092
} finally {
30973093
if (gpfs_options) await this._close_files_gpfs(fs_context, gpfs_options.delete_version, undefined, true);

‎src/util/native_fs_utils.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -268,11 +268,11 @@ async function safe_unlink_gpfs(fs_context, to_delete_path, to_delete_file, dir_
268268
}
269269
}
270270

271-
function should_retry_link_unlink(is_gpfs, err) {
271+
function should_retry_link_unlink(err) {
272272
const should_retry_general = ['ENOENT', 'EEXIST', 'VERSION_MOVED', 'MISMATCH_VERSION'].includes(err.code);
273273
const should_retry_gpfs = [gpfs_link_unlink_retry_err, gpfs_unlink_retry_catch].includes(err.code);
274274
const should_retry_posix = [posix_link_retry_err, posix_unlink_retry_err].includes(err.message);
275-
return should_retry_general || (is_gpfs ? should_retry_gpfs : should_retry_posix);
275+
return should_retry_general || should_retry_gpfs || should_retry_posix;
276276
}
277277

278278
////////////////////////
@@ -434,7 +434,7 @@ async function update_config_file(fs_context, schema_dir, config_path, config_da
434434
break;
435435
} catch (err) {
436436
retries -= 1;
437-
if (retries <= 0 || !should_retry_link_unlink(is_gpfs, err)) throw err;
437+
if (retries <= 0 || !should_retry_link_unlink(err)) throw err;
438438
dbg.warn(`native_fs_utils.update_config_file: Retrying failed move to dest retries=${retries}` +
439439
` source_path=${open_path} dest_path=${config_path}`, err);
440440
if (is_gpfs) {

0 commit comments

Comments
 (0)