Skip to content

Commit

Permalink
Merge remote-tracking branch 'origin/improvement/ARSN-366/listing-sca…
Browse files Browse the repository at this point in the history 
…nned-limit' into w/8.1/improvement/ARSN-366/listing-scanned-limit
  • Loading branch information
@nicolas2bert
nicolas2bert committed Sep 27, 2023
2 parents dcf0f90 + d84cc97 commit 5fd675a
Show file tree
Hide file tree
Showing 7 changed files with 117 additions and 88 deletions.
16 changes: 5 additions & 11 deletions lib/algos/list/delimiterCurrent.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@ type ResultObject = {
NextMarker ?: string;
};

const DELIMITER_TIMEOUT_MS = 10 * 1000; // 10s

/**
* Handle object listing with parameters. This extends the base class DelimiterMaster
* to return the master/current versions.
Expand All @@ -22,6 +20,7 @@ class DelimiterCurrent extends DelimiterMaster {
* @param {Object} parameters - listing parameters
* @param {String} parameters.beforeDate - limit the response to keys older than beforeDate
* @param {String} parameters.excludedDataStoreName - excluded datatore name
* @param {Number} parameters.maxScannedLifecycleListingEntries - max number of entries to be scanned
* @param {RequestLogger} logger - The logger of the request
* @param {String} [vFormat] - versioning key format
*/
Expand All @@ -30,8 +29,7 @@ class DelimiterCurrent extends DelimiterMaster {

this.beforeDate = parameters.beforeDate;
this.excludedDataStoreName = parameters.excludedDataStoreName;
// used for monitoring
this.start = null;
this.maxScannedLifecycleListingEntries = parameters.maxScannedLifecycleListingEntries;
this.scannedKeys = 0;
}

Expand All @@ -51,10 +49,6 @@ class DelimiterCurrent extends DelimiterMaster {
}
}

// The genMDParamsV1() function calls genMDParamsV0() in the Delimiter class,
// making sure that this.start is set for both v0 and v1 bucket formats
this.start = Date.now();

return params;
}

Expand All @@ -80,11 +74,11 @@ class DelimiterCurrent extends DelimiterMaster {
return FILTER_END;
}

if (this.start && Date.now() - this.start > DELIMITER_TIMEOUT_MS) {
if (this.maxScannedLifecycleListingEntries && this.scannedKeys >= this.maxScannedLifecycleListingEntries) {
this.IsTruncated = true;
this.logger.info('listing stopped after expected internal timeout',
this.logger.info('listing stopped due to reaching the maximum scanned entries limit',
{
timeoutMs: DELIMITER_TIMEOUT_MS,
maxScannedLifecycleListingEntries: this.maxScannedLifecycleListingEntries,
scannedKeys: this.scannedKeys,
});
return FILTER_END;
Expand Down
20 changes: 6 additions & 14 deletions lib/algos/list/delimiterNonCurrent.js
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
const { DelimiterVersions } = require('./delimiterVersions');
const { FILTER_END, FILTER_SKIP } = require('./tools');

// TODO: find an acceptable timeout value.
const DELIMITER_TIMEOUT_MS = 10 * 1000; // 10s
const TRIM_METADATA_MIN_BLOB_SIZE = 10000;

/**
Expand All @@ -17,7 +15,8 @@ class DelimiterNonCurrent extends DelimiterVersions {
* @param {String} parameters.versionIdMarker - version id marker
* @param {String} parameters.beforeDate - limit the response to keys with stale date older than beforeDate.
* “stale date” is the date on when a version becomes non-current.
* @param {String} parameters.excludedDataStoreName - exclude dataStoreName matches from the versions
* @param {Number} parameters.maxScannedLifecycleListingEntries - max number of entries to be scanned
* @param {String} parameters.excludedDataStoreName - exclude dataStoreName matches from the versions
* @param {RequestLogger} logger - The logger of the request
* @param {String} [vFormat] - versioning key format
*/
Expand All @@ -26,22 +25,15 @@ class DelimiterNonCurrent extends DelimiterVersions {

this.beforeDate = parameters.beforeDate;
this.excludedDataStoreName = parameters.excludedDataStoreName;
this.maxScannedLifecycleListingEntries = parameters.maxScannedLifecycleListingEntries;

// internal state
this.prevKey = null;
this.staleDate = null;

// used for monitoring
this.scannedKeys = 0;
}

genMDParamsV0() {
// The genMDParamsV1() function calls genMDParamsV0()in the DelimiterVersions class,
// making sure that this.start is set for both v0 and v1 bucket formats
this.start = Date.now();
return super.genMDParamsV0();
}

getLastModified(value) {
let lastModified;
try {
Expand Down Expand Up @@ -78,11 +70,11 @@ class DelimiterNonCurrent extends DelimiterVersions {
}

filter(obj) {
if (this.start && Date.now() - this.start > DELIMITER_TIMEOUT_MS) {
if (this.maxScannedLifecycleListingEntries && this.scannedKeys >= this.maxScannedLifecycleListingEntries) {
this.IsTruncated = true;
this.logger.info('listing stopped after expected internal timeout',
this.logger.info('listing stopped due to reaching the maximum scanned entries limit',
{
timeoutMs: DELIMITER_TIMEOUT_MS,
maxScannedLifecycleListingEntries: this.maxScannedLifecycleListingEntries,
scannedKeys: this.scannedKeys,
});
return FILTER_END;
Expand Down
49 changes: 28 additions & 21 deletions lib/algos/list/delimiterOrphanDeleteMarker.js
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
const DelimiterVersions = require('./delimiterVersions').DelimiterVersions;
const { FILTER_END } = require('./tools');
const DELIMITER_TIMEOUT_MS = 10 * 1000; // 10s
const TRIM_METADATA_MIN_BLOB_SIZE = 10000;
/**
* Handle object listing with parameters. This extends the base class DelimiterVersions
Expand All @@ -13,6 +12,7 @@ class DelimiterOrphanDeleteMarker extends DelimiterVersions {
* Delimiter listing of orphan delete markers.
* @param {Object} parameters - listing parameters
* @param {String} parameters.beforeDate - limit the response to keys older than beforeDate
* @param {Number} parameters.maxScannedLifecycleListingEntries - max number of entries to be scanned
* @param {RequestLogger} logger - The logger of the request
* @param {String} [vFormat] - versioning key format
*/
Expand All @@ -22,6 +22,7 @@ class DelimiterOrphanDeleteMarker extends DelimiterVersions {
maxKeys,
prefix,
beforeDate,
maxScannedLifecycleListingEntries,
} = parameters;

const versionParams = {
Expand All @@ -33,20 +34,13 @@ class DelimiterOrphanDeleteMarker extends DelimiterVersions {
};
super(versionParams, logger, vFormat);

this.maxScannedLifecycleListingEntries = maxScannedLifecycleListingEntries;
this.beforeDate = beforeDate;
this.keyName = null;
this.value = null;
// used for monitoring
this.scannedKeys = 0;
}

genMDParamsV0() {
// The genMDParamsV1() function calls genMDParamsV0() in the DelimiterVersions class,
// making sure that this.start is set for both v0 and v1 bucket formats
this.start = Date.now();
return super.genMDParamsV0();
}

_reachedMaxKeys() {
if (this.keys >= this.maxKeys) {
return true;
Expand Down Expand Up @@ -106,14 +100,23 @@ class DelimiterOrphanDeleteMarker extends DelimiterVersions {
}
return s;
}
/**
* The purpose of _isMaxScannedEntriesReached is to restrict the number of scanned entries,
* thus controlling resource overhead (CPU...).
* @return {boolean} isMaxScannedEntriesReached - true if the maximum limit on the number
* of entries scanned has been reached, false otherwise.
*/
_isMaxScannedEntriesReached() {
return this.maxScannedLifecycleListingEntries && this.scannedKeys >= this.maxScannedLifecycleListingEntries;
}

filter(obj) {
if (this.start && Date.now() - this.start > DELIMITER_TIMEOUT_MS) {
if (this._isMaxScannedEntriesReached()) {
this.nextKeyMarker = this.keyName;
this.IsTruncated = true;
this.logger.info('listing stopped after expected internal timeout',
this.logger.info('listing stopped due to reaching the maximum scanned entries limit',
{
timeoutMs: DELIMITER_TIMEOUT_MS,
maxScannedLifecycleListingEntries: this.maxScannedLifecycleListingEntries,
scannedKeys: this.scannedKeys,
});
return FILTER_END;
Expand Down Expand Up @@ -159,16 +162,20 @@ class DelimiterOrphanDeleteMarker extends DelimiterVersions {
}

result() {
// The following check makes sure the last orphan delete marker is not forgotten.
if (this.keys < this.maxKeys) {
if (this.value) {
this._addOrphan();
// Only check for remaining last orphan delete marker if the listing is not interrupted.
// This will help avoid false positives.
if (!this._isMaxScannedEntriesReached()) {
// The following check makes sure the last orphan delete marker is not forgotten.
if (this.keys < this.maxKeys) {
if (this.value) {
this._addOrphan();
}
// The following make sure that if makeKeys is reached, isTruncated is set to true.
// We moved the "isTruncated" from _reachedMaxKeys to make sure we take into account the last entity
// if listing is truncated right before the last entity and the last entity is a orphan delete marker.
} else {
this.IsTruncated = this.maxKeys > 0;
}
// The following make sure that if makeKeys is reached, isTruncated is set to true.
// We moved the "isTruncated" from _reachedMaxKeys to make sure we take into account the last entity
// if listing is truncated right before the last entity and the last entity is a orphan delete marker.
} else {
this.IsTruncated = this.maxKeys > 0;
}

const result = {
Expand Down
2 changes: 1 addition & 1 deletion package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"engines": {
"node": ">=16"
},
"version": "8.1.110",
"version": "8.1.111",
"description": "Common utilities for the S3 project components",
"main": "build/index.js",
"repository": {
Expand Down
20 changes: 9 additions & 11 deletions tests/unit/algos/list/delimiterCurrent.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@ const VSConst =
require('../../../../lib/versioning/constants').VersioningConstants;
const { DbPrefixes } = VSConst;

const DELIMITER_TIMEOUT_MS = 10 * 1000; // 10s

const EmptyResult = {
Contents: [],
IsTruncated: false,
Expand Down Expand Up @@ -46,11 +44,13 @@ function getListingKey(key, vFormat) {
const marker = 'premark';
const beforeDate = '1970-01-01T00:00:00.005Z';
const excludedDataStoreName = 'location1';
const maxScannedLifecycleListingEntries = 2;
const delimiter = new DelimiterCurrent({
prefix,
marker,
beforeDate,
excludedDataStoreName,
maxScannedLifecycleListingEntries,
}, fakeLogger, v);

const expectedParams = {
Expand All @@ -64,7 +64,7 @@ function getListingKey(key, vFormat) {
lt: getListingKey('prf', v),
};
assert.deepStrictEqual(delimiter.genMDParams(), expectedParams);
assert(delimiter.start);
assert.strictEqual(delimiter.maxScannedLifecycleListingEntries, 2);
});

it('should accept entry starting with prefix', () => {
Expand Down Expand Up @@ -353,9 +353,10 @@ function getListingKey(key, vFormat) {
assert.deepStrictEqual(delimiter.result(), expectedResult);
});

it('should return the objects pushed before timeout', () => {
it('should return the objects pushed before max scanned entries value is reached', () => {
const beforeDate = '1970-01-01T00:00:00.003Z';
const delimiter = new DelimiterCurrent({ beforeDate }, fakeLogger, v);
const maxScannedLifecycleListingEntries = 2;
const delimiter = new DelimiterCurrent({ beforeDate, maxScannedLifecycleListingEntries }, fakeLogger, v);

const masterKey1 = 'key1';
const date1 = '1970-01-01T00:00:00.000Z';
Expand All @@ -375,8 +376,6 @@ function getListingKey(key, vFormat) {
value: value2,
}), FILTER_ACCEPT);

delimiter.start = Date.now() - (DELIMITER_TIMEOUT_MS + 1);

const masterKey3 = 'key3';
const date3 = '1970-01-01T00:00:00.002Z';
const value3 = `{"last-modified": "${date3}"}`;
Expand Down Expand Up @@ -404,9 +403,10 @@ function getListingKey(key, vFormat) {
assert.deepStrictEqual(delimiter.result(), expectedResult);
});

it('should return empty content after timeout', () => {
it('should return empty content after max scanned entries value is reached', () => {
const beforeDate = '1970-01-01T00:00:00.003Z';
const delimiter = new DelimiterCurrent({ beforeDate }, fakeLogger, v);
const maxScannedLifecycleListingEntries = 2;
const delimiter = new DelimiterCurrent({ beforeDate, maxScannedLifecycleListingEntries }, fakeLogger, v);

const masterKey1 = 'key1';
const date1 = '1970-01-01T00:00:00.004Z';
Expand All @@ -426,8 +426,6 @@ function getListingKey(key, vFormat) {
value: value2,
}), FILTER_ACCEPT);

delimiter.start = Date.now() - (DELIMITER_TIMEOUT_MS + 1);

const masterKey3 = 'key3';
const date3 = '1970-01-01T00:00:00.006Z';
const value3 = `{"last-modified": "${date3}"}`;
Expand Down
23 changes: 9 additions & 14 deletions tests/unit/algos/list/delimiterNonCurrent.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,6 @@ const VSConst =
require('../../../../lib/versioning/constants').VersioningConstants;
const { DbPrefixes } = VSConst;

// TODO: find an acceptable timeout value.
const DELIMITER_TIMEOUT_MS = 10 * 1000; // 10s

const VID_SEP = VSConst.VersionId.Separator;
const EmptyResult = {
Contents: [],
Expand Down Expand Up @@ -48,10 +45,12 @@ function getListingKey(key, vFormat) {
const prefix = 'pre';
const keyMarker = 'premark';
const versionIdMarker = 'vid1';
const maxScannedLifecycleListingEntries = 2;
const delimiter = new DelimiterNonCurrent({
prefix,
keyMarker,
versionIdMarker,
maxScannedLifecycleListingEntries,
}, fakeLogger, v);

let expectedParams;
Expand All @@ -70,7 +69,7 @@ function getListingKey(key, vFormat) {
];
}
assert.deepStrictEqual(delimiter.genMDParams(), expectedParams);
assert(delimiter.start);
assert.strictEqual(delimiter.maxScannedLifecycleListingEntries, 2);
});
it('should accept entry starting with prefix', () => {
const delimiter = new DelimiterNonCurrent({ prefix: 'prefix' }, fakeLogger, v);
Expand Down Expand Up @@ -286,8 +285,9 @@ function getListingKey(key, vFormat) {
assert.deepStrictEqual(delimiter.result(), expectedResult);
});

it('should end filtering if delimiter timeout', () => {
const delimiter = new DelimiterNonCurrent({ }, fakeLogger, v);
it('should return the non-current versions pushed before max scanned entries value is reached', () => {
const maxScannedLifecycleListingEntries = 2;
const delimiter = new DelimiterNonCurrent({ maxScannedLifecycleListingEntries }, fakeLogger, v);

const masterKey = 'key';

Expand All @@ -313,9 +313,6 @@ function getListingKey(key, vFormat) {
value: value2,
}), FILTER_ACCEPT);

// force delimiter to timeout.
delimiter.start = Date.now() - (DELIMITER_TIMEOUT_MS + 1);

// filter third version
const versionId3 = 'version3';
const versionKey3 = `${masterKey}${VID_SEP}${versionId3}`;
Expand Down Expand Up @@ -343,8 +340,9 @@ function getListingKey(key, vFormat) {
assert.deepStrictEqual(delimiter.result(), expectedResult);
});

it('should end filtering if delimiter timeout with empty content', () => {
const delimiter = new DelimiterNonCurrent({ }, fakeLogger, v);
it('should return empty content after max scanned entries value is reached', () => {
const maxScannedLifecycleListingEntries = 2;
const delimiter = new DelimiterNonCurrent({ maxScannedLifecycleListingEntries }, fakeLogger, v);

// filter current version
const masterKey1 = 'key1';
Expand All @@ -370,9 +368,6 @@ function getListingKey(key, vFormat) {
value: value2,
}), FILTER_ACCEPT);

// force delimiter to timeout.
delimiter.start = Date.now() - (DELIMITER_TIMEOUT_MS + 1);

// filter current version
const masterKey3 = 'key3';
const versionId3 = 'version3';
Expand Down
Loading

0 comments on commit 5fd675a

Please sign in to comment.