Skip to content

Commit 5d51c2c

Browse files
committed
NC | NSFS | Add stat to bucket_namespace_cache
1. Create config option NC_ENABLE_BUCKET_NS_CACHE_STAT_VALIDATION to enable/disable this addition. 2. Add in read_bucket_sdk_info inside bucketspace FS property stat, and add a method stat_bucket that would only implemented in FS (as we stat cofig files, and in NB with DB we don't need this). 3. Edit the _validate_bucket_namespace so before checking the validation by time, in case it is bucketspace FS we will compare the stat and return false (invalidate) in case the config file was changed. 4. Changes in nc_coretest create the functions start_nsfs_process and stop_nsfs_process based on existing implementation to allow us to restart nsfs. 5. Rename the function check_bucket_config to check_stat_bucket_storage_path that it would be less confusing. 6. Edit the printings of "Could not log bucket operation" to have the details of where they were printed. 7. Adding a basic test of running with a couple of forks. 8. Change the default number of forks in the Ceph NSFS tests in the CI to 2. 9. Rename file test_bucketspace.js to test_nsfs_integration.js and the nc_core test log files from src/logfile to nsfs_integration_test_log. 10. Small changes in s3_bucket_logging.js regarding bucket deletion: bucket existed and bucket that did not exist - see NC | NSFS | Bucket Logging After Delete Bucket #8540. Signed-off-by: shirady <57721533+shirady@users.noreply.github.com>
1 parent f924d96 commit 5d51c2c

File tree

14 files changed

+230
-42
lines changed

14 files changed

+230
-42
lines changed

config.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -637,6 +637,9 @@ config.INLINE_MAX_SIZE = 4096;
637637
// Object SDK bucket cache expiration time
638638
config.OBJECT_SDK_BUCKET_CACHE_EXPIRY_MS = 60000;
639639

640+
// Object SDK bucket_namespace_cache allow stat of the config file
641+
config.NC_ENABLE_BUCKET_NS_CACHE_STAT_VALIDATION = true;
642+
640643
//////////////////////////////
641644
// OPERATOR RELATED //
642645
//////////////////////////////

docs/NooBaaNonContainerized/CI&Tests.md

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ The following is a list of `NC mocha test` files -
9090
1. `test_nc_nsfs_cli.js` - Tests NooBaa CLI.
9191
2. `test_nc_nsfs_health` - Tests NooBaa Health CLI.
9292
3. `test_nsfs_glacier_backend.js` - Tests NooBaa Glacier Backend.
93+
4. `test_nc_with_a_couple_of_forks.js` - Tests the `bucket_namespace_cache` when running with a couple of forks. Please notice that it uses `nc_coretest` with setup that includes a couple of forks.
9394

9495
#### NC Jest test files
9596
The following is a list of `NC jest tests` files -
@@ -114,7 +115,7 @@ The following is a list of `NC jest tests` files -
114115
#### nc_coretest.js File
115116
* The `nc_coretest.js` is a file that runs setup and teardown before/after NC integration tests run.
116117
* Moreover, `nc_coretest.js` includes mapping between RPC API calls to NooBaa CLI calls in order to be able to run same integration tests on both containerized and non containerized deployments.
117-
* Use `NC_CORETEST=true` environment variable when running NC NSFS integration test (test_bucketspace.js).
118+
* Use `NC_CORETEST=true` environment variable when running NC NSFS integration test (test_nsfs_integration.js).
118119

119120
##### Differences Between Containerized and Non Containerized
120121
* `new_buckets_path` -
@@ -164,7 +165,7 @@ Consequently, there are now distinct test files, each with a unique scope -
164165
1. `test_namespace_fs.js` - Tests NamespaceFS API.
165166
2. `test_ns_list_objects.js` - Tests NamespaceFS list objects API.
166167
3. `test_nsfs_access.js` - Tests uid and gid accessibility of Napi native code.
167-
4. `test_bucketspace.js` - Tests s3 flows on top of NSFS namespace resources.
168+
4. `test_nsfs_integration.js` - Tests s3 flows on top of NSFS namespace resources.
168169
5. `test_nb_native_fs.js` - Tests Napi native code.
169170
6. `test_nb_native_gpfs.js` - Tests Napi native code on top of GPFS.
170171
7. `test_nsfs_versioning.js` - Tests NamespaceFS versioning API.

src/endpoint/s3/s3_bucket_logging.js

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ const {compose_notification_req, check_notif_relevant} = require('../../util/not
1111
async function send_bucket_op_logs(req, res) {
1212
if (req.params && req.params.bucket &&
1313
!(req.op_name === 'put_bucket' ||
14+
req.op_name === 'delete_bucket' ||
1415
req.op_name === 'put_bucket_notification' ||
1516
req.op_name === 'get_bucket_notification'
1617
)) {
@@ -19,8 +20,14 @@ async function send_bucket_op_logs(req, res) {
1920
//so we aggregate and issue the writes only in the end
2021
const writes_aggregate = [];
2122

22-
const bucket_info = await req.object_sdk.read_bucket_sdk_config_info(req.params.bucket);
23-
dbg.log2("read_bucket_sdk_config_info = ", bucket_info);
23+
let bucket_info;
24+
try {
25+
bucket_info = await req.object_sdk.read_bucket_sdk_config_info(req.params.bucket);
26+
dbg.log2("read_bucket_sdk_config_info = ", bucket_info);
27+
} catch (err) {
28+
dbg.warn(`send_bucket_op_logs of bucket ${req.params.bucket} got an error:`, err);
29+
return;
30+
}
2431

2532
if (is_bucket_logging_enabled(bucket_info)) {
2633
dbg.log2("Bucket logging is enabled for Bucket : ", req.params.bucket);

src/endpoint/s3/s3_rest.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ async function s3_rest(req, res) {
7373
try {
7474
await s3_logging.send_bucket_op_logs(req, res); // logging again with error
7575
} catch (err1) {
76-
dbg.error("Could not log bucket operation:", err1);
76+
dbg.error("Could not log bucket operation (after handle_error):", err1);
7777
}
7878
}
7979
}
@@ -134,7 +134,7 @@ async function handle_request(req, res) {
134134
try {
135135
await s3_logging.send_bucket_op_logs(req); // logging intension - no result
136136
} catch (err) {
137-
dbg.error("Could not log bucket operation:", err);
137+
dbg.error(`Could not log bucket operation (before operation ${req.op_name}):`, err);
138138
}
139139
}
140140

@@ -165,7 +165,7 @@ async function handle_request(req, res) {
165165
try {
166166
await s3_logging.send_bucket_op_logs(req, res); // logging again with result
167167
} catch (err) {
168-
dbg.error("Could not log bucket operation:", err);
168+
dbg.error(`Could not log bucket operation (after operation ${req.op_name}):`, err);
169169
}
170170

171171
}

src/sdk/bucketspace_fs.js

Lines changed: 30 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -104,9 +104,9 @@ class BucketSpaceFS extends BucketSpaceSimpleFS {
104104
const bucket = await this.config_fs.get_bucket_by_name(name);
105105
nsfs_schema_utils.validate_bucket_schema(bucket);
106106

107-
const is_valid = await this.check_bucket_config(bucket);
107+
const is_valid = await this.check_stat_bucket_storage_path(bucket.path);
108108
if (!is_valid) {
109-
dbg.warn('BucketSpaceFS: one or more bucket config check is failed for bucket : ', name);
109+
dbg.warn('BucketSpaceFS: invalid storage path for bucket : ', name);
110110
}
111111

112112
const nsr = {
@@ -155,6 +155,11 @@ class BucketSpaceFS extends BucketSpaceSimpleFS {
155155
}
156156
}
157157
}
158+
try {
159+
bucket.stat = await this.config_fs.stat_bucket_config_file(bucket.name.unwrap());
160+
} catch (err) {
161+
dbg.warn(`BucketspaceFS.read_bucket_sdk_info could not stat_bucket_config_file ${bucket.name.unwrap()}`);
162+
}
158163
return bucket;
159164
} catch (err) {
160165
const rpc_error = translate_error_codes(err, entity_enum.BUCKET);
@@ -164,17 +169,37 @@ class BucketSpaceFS extends BucketSpaceSimpleFS {
164169
}
165170
}
166171

167-
async check_bucket_config(bucket) {
168-
const bucket_storage_path = bucket.path;
172+
/**
173+
* check_stat_bucket_storage_path will return the true
174+
* if there is stat output on the bucket storage path
175+
* (in case the stat throws an error it would return false)
176+
* @param {string} bucket_storage_path
177+
* @returns {Promise<boolean>}
178+
*/
179+
async check_stat_bucket_storage_path(bucket_storage_path) {
169180
try {
170181
await nb_native().fs.stat(this.fs_context, bucket_storage_path);
171-
//TODO: Bucket owner check
172182
return true;
173183
} catch (err) {
174184
return false;
175185
}
176186
}
177187

188+
/**
189+
* check_same_stat will return true the config file was not changed
190+
* @param {nb.NativeFSStats} bucket_stat
191+
* @returns Promise<{boolean>}
192+
*/
193+
async check_same_stat(bucket_name, bucket_stat) {
194+
try {
195+
const current_stat = await this.config_fs.stat_bucket_config_file(bucket_name);
196+
if (current_stat) {
197+
return current_stat.ino === bucket_stat.ino && current_stat.mtimeNsBigint === bucket_stat.mtimeNsBigint;
198+
}
199+
} catch (err) {
200+
dbg.warn('check_same_stat: current_stat got an error', err, 'ignoring...');
201+
}
202+
}
178203

179204
////////////
180205
// BUCKET //

src/sdk/config_fs.js

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -885,6 +885,17 @@ class ConfigFS {
885885
return new_path;
886886
}
887887

888+
/**
889+
* stat_bucket_config_file will return the stat output on bucket config file
890+
* please notice that stat might throw an error - you should wrap it with try-catch and handle the error
891+
* @param {string} bucket_name
892+
* @returns {Promise<nb.NativeFSStats>}
893+
*/
894+
stat_bucket_config_file(bucket_name) {
895+
const bucket_config_path = this.get_bucket_path_by_name(bucket_name);
896+
return nb_native().fs.stat(this.fs_context, bucket_config_path);
897+
}
898+
888899
/**
889900
* is_bucket_exists returns true if bucket config path exists in config dir
890901
* @param {string} bucket_name

src/sdk/nb.d.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -824,6 +824,7 @@ interface BucketSpace {
824824

825825
read_account_by_access_key({ access_key: string }): Promise<any>;
826826
read_bucket_sdk_info({ name: string }): Promise<any>;
827+
check_same_stat(bucket_name: string, bucket_stat: nb.NativeFSStats); // only implemented in bucketspace_fs
827828

828829
list_buckets(params: object, object_sdk: ObjectSDK): Promise<any>;
829830
read_bucket(params: object): Promise<any>;

src/sdk/object_sdk.js

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -286,6 +286,15 @@ class ObjectSDK {
286286

287287
async _validate_bucket_namespace(data, params) {
288288
const time = Date.now();
289+
// stat check (only in bucketspace FS)
290+
const bs = this._get_bucketspace();
291+
const ns_allow_stat_bucket = Boolean(bs.check_same_stat);
292+
if (ns_allow_stat_bucket && config.NC_ENABLE_BUCKET_NS_CACHE_STAT_VALIDATION) {
293+
const same_stat = await bs.check_same_stat(params.name, data.bucket.stat);
294+
if (!same_stat) { // config file of bucket was changed
295+
return false;
296+
}
297+
}
289298
if (time <= data.valid_until) return true;
290299
const bucket = await this._get_bucketspace().read_bucket_sdk_info({ name: params.name });
291300
if (_.isEqual(bucket, data.bucket)) {

src/test/system_tests/ceph_s3_tests/run_ceph_nsfs_test_on_test_container.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ export FS_ROOT_2=/tmp/nsfs_root2/
3030
# 1. Create configuration directory
3131
# 2. Create config.json file
3232
mkdir -p ${CONFIG_DIR}
33-
config='{"ALLOW_HTTP":true}'
33+
config='{"ALLOW_HTTP":true, "ENDPOINT_FORKS":2}'
3434
echo "$config" > ${CONFIG_DIR}/config.json
3535

3636
# 1. Create root directory for bucket creation

src/test/unit_tests/nc_coretest.js

Lines changed: 77 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ dbg.set_module_level(dbg_level, 'core');
4141

4242
const P = require('../../util/promise');
4343
let _setup = false;
44+
let _nsfs_process = false;
4445

4546
const SYSTEM = NC_CORETEST;
4647
const EMAIL = NC_CORETEST;
@@ -58,13 +59,17 @@ const NC_CORETEST_CONFIG_FS = new ConfigFS(NC_CORETEST_CONFIG_DIR_PATH);
5859

5960
const nsrs_to_root_paths = {};
6061
let nsfs_process;
62+
let current_setup_options = {};
6163

6264

6365
/**
6466
* setup will setup nc coretest
65-
* @param {object} options
67+
* currently the setup_options we use are for the nsfs process: fork and debug
68+
* @param {object} setup_options
6669
*/
67-
function setup(options = {}) {
70+
function setup(setup_options = {}) {
71+
console.log(`in setup - variables values: _setup ${_setup} _nsfs_process ${_nsfs_process}`);
72+
current_setup_options = setup_options;
6873
if (_setup) return;
6974
_setup = true;
7075

@@ -76,39 +81,17 @@ function setup(options = {}) {
7681
await config_dir_setup();
7782
await admin_account_creation();
7883
await first_bucket_creation();
79-
80-
await announce('start nsfs script');
81-
const logStream = fs.createWriteStream('src/logfile.txt', { flags: 'a' });
82-
83-
nsfs_process = child_process.spawn('node', ['src/cmd/nsfs.js'], {
84-
detached: true
85-
});
86-
nsfs_process.stdout.pipe(logStream);
87-
nsfs_process.stderr.pipe(logStream);
88-
89-
90-
nsfs_process.on('exit', (code, signal) => {
91-
dbg.error(`nsfs.js exited code=${code}, signal=${signal}`);
92-
logStream.end();
93-
});
94-
95-
nsfs_process.on('error', err => {
96-
dbg.error(`nsfs.js exited with error`, err);
97-
logStream.end();
98-
});
84+
await start_nsfs_process(setup_options);
9985

10086
// TODO - run health
101-
// wait 2 seconds before announcing nc coretes is ready
102-
await P.delay(2000);
10387
await announce(`nc coretest ready... (took ${((Date.now() - start) / 1000).toFixed(1)} sec)`);
10488
});
10589

10690

10791
mocha.after('nc-coretest-after', async function() {
10892
this.timeout(60000); // eslint-disable-line no-invalid-this
10993
try {
110-
await announce('stop nsfs script');
111-
if (nsfs_process) nsfs_process.kill('SIGKILL');
94+
await stop_nsfs_process();
11295
await config_dir_teardown();
11396
await announce('nc coretest done ...');
11497
} catch (err) {
@@ -117,6 +100,71 @@ function setup(options = {}) {
117100
});
118101
}
119102

103+
/**
104+
* start_nsfs_process starts the NSFS process and attach the logs in a file
105+
* the setup_options we support:
106+
* 1. forks (number of processes to run)
107+
* 2. debug (for logs printing level)
108+
* @param {object} setup_options
109+
*/
110+
async function start_nsfs_process(setup_options) {
111+
console.log(`in start_nsfs_process - variables values: _setup ${_setup} _nsfs_process ${_nsfs_process}`);
112+
const { forks, debug } = setup_options;
113+
console.log(`setup_options: forks ${forks} debug ${debug}`);
114+
if (_nsfs_process) return;
115+
await announce('start nsfs script');
116+
const logStream = fs.createWriteStream('nsfs_integration_test_log.txt', { flags: 'a' });
117+
118+
const arguments_for_command = ['src/cmd/nsfs.js'];
119+
if (forks) {
120+
arguments_for_command.push('--forks');
121+
arguments_for_command.push(`${forks}`);
122+
}
123+
if (debug) {
124+
arguments_for_command.push('--debug');
125+
arguments_for_command.push(`${debug}`);
126+
}
127+
nsfs_process = child_process.spawn('node', arguments_for_command, {
128+
detached: true
129+
});
130+
131+
_nsfs_process = true;
132+
133+
nsfs_process.stdout.pipe(logStream);
134+
nsfs_process.stderr.pipe(logStream);
135+
136+
137+
nsfs_process.on('exit', (code, signal) => {
138+
dbg.error(`nsfs.js exited code=${code}, signal=${signal}`);
139+
logStream.end();
140+
});
141+
142+
nsfs_process.on('error', err => {
143+
dbg.error(`nsfs.js exited with error`, err);
144+
logStream.end();
145+
});
146+
// wait for the process to be ready (else would see ECONNREFUSED issue)
147+
await P.delay(5000);
148+
}
149+
150+
/**
151+
* stop_nsfs_process stops the NSFS
152+
*/
153+
async function stop_nsfs_process() {
154+
console.log(`in stop_nsfs_process - variables values: _setup ${_setup} _nsfs_process ${_nsfs_process}`);
155+
_nsfs_process = false;
156+
await announce('stop nsfs script');
157+
if (nsfs_process) nsfs_process.kill('SIGKILL');
158+
}
159+
160+
/**
161+
* get_current_setup_options returns the current_setup_options
162+
* currently the setup_options we use are for the nsfs process: fork and debug
163+
*/
164+
function get_current_setup_options() {
165+
return current_setup_options;
166+
}
167+
120168
/**
121169
* announce will pretty print msg
122170
* @param {string} msg
@@ -472,6 +520,9 @@ const rpc_cli_funcs_to_manage_nsfs_cli_cmds = {
472520
};
473521

474522
exports.setup = setup;
523+
exports.get_current_setup_options = get_current_setup_options;
524+
exports.stop_nsfs_process = stop_nsfs_process;
525+
exports.start_nsfs_process = start_nsfs_process;
475526
exports.no_setup = _.noop;
476527
exports.log = log;
477528
exports.SYSTEM = SYSTEM;

0 commit comments

Comments
 (0)