Skip to content

Commit 950f8a2

Browse files
committed
support newline and backslash character in Glacier logs
Signed-off-by: Utkarsh Srivastava <[email protected]>
1 parent ed5f39b commit 950f8a2

File tree

4 files changed

+210
-15
lines changed

4 files changed

+210
-15
lines changed

src/sdk/glacier.js

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -300,6 +300,32 @@ class Glacier {
300300
return restore_status.state === Glacier.RESTORE_STATUS_CAN_RESTORE;
301301
}
302302

303+
/**
304+
* encode_log takes in data intended for the backend and encodes
305+
* it.
306+
*
307+
* This method must be overwritten for all the backends if they need
308+
* different encodings for their logs.
309+
* @param {string} data
310+
* @returns {string}
311+
*/
312+
encode_log(data) {
313+
return data;
314+
}
315+
316+
/**
317+
* decode_log takes in data intended for the backend and decodes
318+
* it.
319+
*
320+
* This method must be overwritten for all the backends if they need
321+
* different encodings for their logs.
322+
* @param {string} data
323+
* @returns {string}
324+
*/
325+
decode_log(data) {
326+
return data;
327+
}
328+
303329
/**
304330
* get_restore_status returns status of the object at the given
305331
* file_path

src/sdk/glacier_tapecloud.js

Lines changed: 58 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,8 @@ class TapeCloudUtils {
195195
}
196196

197197
class TapeCloudGlacier extends Glacier {
198+
static LOG_DELIM = ' -- ';
199+
198200
/**
199201
* @param {nb.NativeFSContext} fs_context
200202
* @param {LogFile} log_file
@@ -204,8 +206,14 @@ class TapeCloudGlacier extends Glacier {
204206
async stage_migrate(fs_context, log_file, failure_recorder) {
205207
dbg.log2('TapeCloudGlacier.stage_migrate starting for', log_file.log_path);
206208

209+
// Wrap failure recorder to make sure we correctly encode the entries
210+
// before appending them to the failure log
211+
const encoded_failure_recorder = async failure => failure_recorder(this.encode_log(failure));
212+
207213
try {
208214
await log_file.collect(Glacier.MIGRATE_STAGE_WAL_NAME, async (entry, batch_recorder) => {
215+
entry = this.decode_log(entry);
216+
209217
let entry_fh;
210218
let should_migrate = true;
211219
try {
@@ -234,7 +242,7 @@ class TapeCloudGlacier extends Glacier {
234242
// Can't really do anything if this fails - provider
235243
// needs to make sure that appropriate error handling
236244
// is being done there
237-
await failure_recorder(entry);
245+
await encoded_failure_recorder(entry);
238246
return;
239247
}
240248

@@ -244,14 +252,14 @@ class TapeCloudGlacier extends Glacier {
244252
// Mark the file staged
245253
try {
246254
await entry_fh.replacexattr(fs_context, { [Glacier.XATTR_STAGE_MIGRATE]: Date.now().toString() });
247-
await batch_recorder(entry);
255+
await batch_recorder(this.encode_log(entry));
248256
} catch (error) {
249257
dbg.error('failed to mark the entry migrate staged', error);
250258

251259
// Can't really do anything if this fails - provider
252260
// needs to make sure that appropriate error handling
253261
// is being done there
254-
await failure_recorder(entry);
262+
await encoded_failure_recorder(entry);
255263
} finally {
256264
entry_fh?.close(fs_context);
257265
}
@@ -272,16 +280,23 @@ class TapeCloudGlacier extends Glacier {
272280
*/
273281
async migrate(fs_context, log_file, failure_recorder) {
274282
dbg.log2('TapeCloudGlacier.migrate starting for', log_file.log_path);
283+
284+
// Wrap failure recorder to make sure we correctly encode the entries
285+
// before appending them to the failure log
286+
const encoded_failure_recorder = async failure => failure_recorder(this.encode_log(failure));
287+
275288
try {
276289
// This will throw error only if our eeadm error handler
277290
// panics as well and at that point it's okay to
278291
// not handle the error and rather keep the log file around
279-
await this._migrate(log_file.log_path, failure_recorder);
292+
await this._migrate(log_file.log_path, encoded_failure_recorder);
280293

281294
// Un-stage all the files - We don't need to deal with the cases
282295
// where some files have migrated and some have not as that is
283296
// not important for staging/un-staging.
284297
await log_file.collect_and_process(async entry => {
298+
entry = this.decode_log(entry);
299+
285300
let fh;
286301
try {
287302
fh = await nb_native().fs.open(fs_context, entry);
@@ -297,7 +312,7 @@ class TapeCloudGlacier extends Glacier {
297312
// Add the enty to the failure log - This could be wasteful as it might
298313
// add entries which have already been migrated but this is a better
299314
// retry.
300-
await failure_recorder(entry);
315+
await encoded_failure_recorder(entry);
301316
} finally {
302317
await fh?.close(fs_context);
303318
}
@@ -319,8 +334,14 @@ class TapeCloudGlacier extends Glacier {
319334
async stage_restore(fs_context, log_file, failure_recorder) {
320335
dbg.log2('TapeCloudGlacier.stage_restore starting for', log_file.log_path);
321336

337+
// Wrap failure recorder to make sure we correctly encode the entries
338+
// before appending them to the failure log
339+
const encoded_failure_recorder = async failure => failure_recorder(this.encode_log(failure));
340+
322341
try {
323342
await log_file.collect(Glacier.RESTORE_STAGE_WAL_NAME, async (entry, batch_recorder) => {
343+
entry = this.decode_log(entry);
344+
324345
let fh;
325346
try {
326347
fh = await nb_native().fs.open(fs_context, entry);
@@ -347,9 +368,9 @@ class TapeCloudGlacier extends Glacier {
347368
// 3. If we read corrupt value then either the file is getting staged or is
348369
// getting un-staged - In either case we must requeue.
349370
if (stat.xattr[Glacier.XATTR_STAGE_MIGRATE]) {
350-
await failure_recorder(entry);
371+
await encoded_failure_recorder(entry);
351372
} else {
352-
await batch_recorder(entry);
373+
await batch_recorder(this.encode_log(entry));
353374
}
354375
} catch (error) {
355376
if (error.code === 'ENOENT') {
@@ -361,7 +382,7 @@ class TapeCloudGlacier extends Glacier {
361382
'adding log entry', entry,
362383
'to failure recorder due to error', error,
363384
);
364-
await failure_recorder(entry);
385+
await encoded_failure_recorder(entry);
365386
} finally {
366387
await fh?.close(fs_context);
367388
}
@@ -383,25 +404,32 @@ class TapeCloudGlacier extends Glacier {
383404
async restore(fs_context, log_file, failure_recorder) {
384405
dbg.log2('TapeCloudGlacier.restore starting for', log_file.log_path);
385406

407+
// Wrap failure recorder to make sure we correctly encode the entries
408+
// before appending them to the failure log
409+
const encoded_failure_recorder = async failure => failure_recorder(this.encode_log(failure));
410+
386411
try {
387412
const success = await this._recall(
388413
log_file.log_path,
389414
async entry_path => {
415+
entry_path = this.decode_log(entry_path);
390416
dbg.log2('TapeCloudGlacier.restore.partial_failure - entry:', entry_path);
391-
await failure_recorder(entry_path);
417+
await encoded_failure_recorder(entry_path);
392418
},
393419
async entry_path => {
420+
entry_path = this.decode_log(entry_path);
394421
dbg.log2('TapeCloudGlacier.restore.partial_success - entry:', entry_path);
395-
await this._finalize_restore(fs_context, entry_path, failure_recorder);
422+
await this._finalize_restore(fs_context, entry_path, encoded_failure_recorder);
396423
}
397424
);
398425

399426
// We will iterate through the entire log file iff and we get a success message from
400427
// the recall call.
401428
if (success) {
402429
await log_file.collect_and_process(async (entry_path, batch_recorder) => {
430+
entry_path = this.decode_log(entry_path);
403431
dbg.log2('TapeCloudGlacier.restore.batch - entry:', entry_path);
404-
await this._finalize_restore(fs_context, entry_path, failure_recorder);
432+
await this._finalize_restore(fs_context, entry_path, encoded_failure_recorder);
405433
});
406434
}
407435

@@ -425,6 +453,25 @@ class TapeCloudGlacier extends Glacier {
425453
return result.toLowerCase().trim() === 'true';
426454
}
427455

456+
/**
457+
*
458+
* @param {string} data
459+
* @returns
460+
*/
461+
encode_log(data) {
462+
const encoded = data.replace(/\\/g, '\\\\').replace(/\n/g, '\\n');
463+
return `${TapeCloudGlacier.LOG_DELIM}${encoded}`;
464+
}
465+
466+
/**
467+
*
468+
* @param {string} data
469+
* @returns
470+
*/
471+
decode_log(data) {
472+
return data.substring(TapeCloudGlacier.LOG_DELIM.length).replace(/\\n/g, '\n').replace(/\\\\/g, '\\');
473+
}
474+
428475
// ============= PRIVATE FUNCTIONS =============
429476

430477
/**

src/sdk/namespace_fs.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3697,13 +3697,13 @@ class NamespaceFS {
36973697
async append_to_migrate_wal(entry) {
36983698
if (!config.NSFS_GLACIER_LOGS_ENABLED) return;
36993699

3700-
await NamespaceFS.migrate_wal.append(entry);
3700+
await NamespaceFS.migrate_wal.append(Glacier.getBackend().encode_log(entry));
37013701
}
37023702

37033703
async append_to_restore_wal(entry) {
37043704
if (!config.NSFS_GLACIER_LOGS_ENABLED) return;
37053705

3706-
await NamespaceFS.restore_wal.append(entry);
3706+
await NamespaceFS.restore_wal.append(Glacier.getBackend().encode_log(entry));
37073707
}
37083708

37093709
static get migrate_wal() {

src/test/unit_tests/nsfs/test_nsfs_glacier_backend.js

Lines changed: 124 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,8 @@ mocha.describe('nsfs_glacier', function() {
166166
});
167167

168168
mocha.describe('nsfs_glacier_tapecloud', async function() {
169+
const restore_key_spcl_char_1 = 'restore_key_2_\n';
170+
const restore_key_spcl_char_2 = 'restore_key_2_\n_2';
169171
const upload_key = 'upload_key_1';
170172
const restore_key = 'restore_key_1';
171173
const xattr = { key: 'value', key2: 'value2' };
@@ -280,10 +282,10 @@ mocha.describe('nsfs_glacier', function() {
280282
failure_backend._process_expired = async () => { /**noop*/ };
281283
failure_backend._recall = async (_file, failure_recorder, success_recorder) => {
282284
// This unintentionally also replicates duplicate entries in WAL
283-
await failure_recorder(failed_file_path);
285+
await failure_recorder(failure_backend.encode_log(failed_file_path));
284286

285287
// This unintentionally also replicates duplicate entries in WAL
286-
await success_recorder(success_file_path);
288+
await success_recorder(failure_backend.encode_log(success_file_path));
287289

288290
return false;
289291
};
@@ -324,6 +326,126 @@ mocha.describe('nsfs_glacier', function() {
324326
assert(failure_stats.xattr[Glacier.XATTR_RESTORE_REQUEST]);
325327
});
326328

329+
mocha.it('restore-object should successfully restore objects with special characters', async function() {
330+
const now = Date.now();
331+
const data = crypto.randomBytes(100);
332+
const all_params = [
333+
{
334+
bucket: upload_bkt,
335+
key: restore_key_spcl_char_1,
336+
storage_class: s3_utils.STORAGE_CLASS_GLACIER,
337+
xattr,
338+
days: 1,
339+
source_stream: buffer_utils.buffer_to_read_stream(data)
340+
},
341+
{
342+
bucket: upload_bkt,
343+
key: restore_key_spcl_char_2,
344+
storage_class: s3_utils.STORAGE_CLASS_GLACIER,
345+
xattr,
346+
days: 1,
347+
source_stream: buffer_utils.buffer_to_read_stream(data)
348+
}
349+
];
350+
351+
for (const params of all_params) {
352+
const upload_res = await glacier_ns.upload_object(params, dummy_object_sdk);
353+
console.log('upload_object response', inspect(upload_res));
354+
355+
const restore_res = await glacier_ns.restore_object(params, dummy_object_sdk);
356+
assert(restore_res);
357+
358+
// Issue restore
359+
await backend.perform(glacier_ns.prepare_fs_context(dummy_object_sdk), "RESTORE");
360+
361+
362+
// Ensure object is restored
363+
const md = await glacier_ns.read_object_md(params, dummy_object_sdk);
364+
365+
assert(!md.restore_status.ongoing);
366+
367+
const expected_expiry = Glacier.generate_expiry(new Date(), params.days, '', config.NSFS_GLACIER_EXPIRY_TZ);
368+
assert(expected_expiry.getTime() >= md.restore_status.expiry_time.getTime());
369+
assert(now <= md.restore_status.expiry_time.getTime());
370+
}
371+
});
372+
373+
mocha.it('restore-object should not restore failed item with special characters', async function() {
374+
const now = Date.now();
375+
const data = crypto.randomBytes(100);
376+
const failed_restore_key = `${restore_key_spcl_char_1}_failured`;
377+
const success_restore_key = `${restore_key_spcl_char_1}_success`;
378+
379+
const failed_params = {
380+
bucket: upload_bkt,
381+
key: failed_restore_key,
382+
storage_class: s3_utils.STORAGE_CLASS_GLACIER,
383+
xattr,
384+
days: 1,
385+
source_stream: buffer_utils.buffer_to_read_stream(data)
386+
};
387+
388+
const success_params = {
389+
bucket: upload_bkt,
390+
key: success_restore_key,
391+
storage_class: s3_utils.STORAGE_CLASS_GLACIER,
392+
xattr,
393+
days: 1,
394+
source_stream: buffer_utils.buffer_to_read_stream(data)
395+
};
396+
397+
const failed_file_path = glacier_ns._get_file_path(failed_params);
398+
const success_file_path = glacier_ns._get_file_path(success_params);
399+
400+
const failure_backend = new TapeCloudGlacier();
401+
failure_backend._migrate = async () => true;
402+
failure_backend._process_expired = async () => { /**noop*/ };
403+
failure_backend._recall = async (_file, failure_recorder, success_recorder) => {
404+
// This unintentionally also replicates duplicate entries in WAL
405+
await failure_recorder(failure_backend.encode_log(failed_file_path));
406+
407+
// This unintentionally also replicates duplicate entries in WAL
408+
await success_recorder(failure_backend.encode_log(success_file_path));
409+
410+
return false;
411+
};
412+
413+
const upload_res_1 = await glacier_ns.upload_object(failed_params, dummy_object_sdk);
414+
console.log('upload_object response', inspect(upload_res_1));
415+
416+
const upload_res_2 = await glacier_ns.upload_object(success_params, dummy_object_sdk);
417+
console.log('upload_object response', inspect(upload_res_2));
418+
419+
const restore_res_1 = await glacier_ns.restore_object(failed_params, dummy_object_sdk);
420+
assert(restore_res_1);
421+
422+
const restore_res_2 = await glacier_ns.restore_object(success_params, dummy_object_sdk);
423+
assert(restore_res_2);
424+
425+
const fs_context = glacier_ns.prepare_fs_context(dummy_object_sdk);
426+
427+
// Issue restore
428+
await failure_backend.perform(glacier_ns.prepare_fs_context(dummy_object_sdk), "RESTORE");
429+
430+
// Ensure success object is restored
431+
const success_md = await glacier_ns.read_object_md(success_params, dummy_object_sdk);
432+
433+
assert(!success_md.restore_status.ongoing);
434+
435+
const expected_expiry = Glacier.generate_expiry(new Date(), success_params.days, '', config.NSFS_GLACIER_EXPIRY_TZ);
436+
assert(expected_expiry.getTime() >= success_md.restore_status.expiry_time.getTime());
437+
assert(now <= success_md.restore_status.expiry_time.getTime());
438+
439+
// Ensure failed object is NOT restored
440+
const failure_stats = await nb_native().fs.stat(
441+
fs_context,
442+
failed_file_path,
443+
);
444+
445+
assert(!failure_stats.xattr[Glacier.XATTR_RESTORE_EXPIRY] || failure_stats.xattr[Glacier.XATTR_RESTORE_EXPIRY] === '');
446+
assert(failure_stats.xattr[Glacier.XATTR_RESTORE_REQUEST]);
447+
});
448+
327449
mocha.it('_finalize_restore should tolerate deleted objects', async function() {
328450
// should not throw error if the path does not exist
329451
await backend._finalize_restore(glacier_ns.prepare_fs_context(dummy_object_sdk), '/path/does/not/exist');

0 commit comments

Comments
 (0)