Compare commits

...

1 Commits

Author SHA1 Message Date
Jeremy Desanlis 72c77c5561 ZENKO-1124: fix delimiterMaster::filter delete markers handlings.
A specific dataset described and attached in the linked Jira issue shows
a bug in that the current delimiterMaster::filter handling of the delete
marker on a master version and the use of this function into MongoDB
client leads to an infinite loop.

The input dataset, in a versionedis in the form:
- one object with a master version and some others,
- 10k objects with a delete marker on the master version and some other
  versions for each object.
The expected result for this input contains only the first object.

What happens in the delimiterMaster:
- filtering the first object results to adds it to the result and set
  NextMarker to its key. The following versions of this objets are
  skipped and the NextMarker is not modified.
- filtering the others objets results to skip return values.

In the MongoDB client class, after counting 100 skipped values in a row,
the skipscan mechanism is triggered. The problem is that the new range
is computed using NextMarker, which in this case is set to the key of
the 1st object. The listing is done on the same range (well, the first
object is skipped at least).

This commit modifies the skipping value to avoid a client to loop.
2018-09-09 21:44:55 -07:00
2 changed files with 20 additions and 5 deletions

View File

@ -25,6 +25,7 @@ class DelimiterMaster extends Delimiter {
// non-PHD master version or a version whose master is a PHD version // non-PHD master version or a version whose master is a PHD version
this.prvKey = undefined; this.prvKey = undefined;
this.prvPHDKey = undefined; this.prvPHDKey = undefined;
this.markerParams = parameters.marker;
} }
/** /**
@ -116,7 +117,7 @@ class DelimiterMaster extends Delimiter {
} }
skipping() { skipping() {
if (this.NextMarker) { if (this.NextMarker && this.NextMarker !== this.markerParams) {
// next marker: // next marker:
// - foo/ : skipping foo/ // - foo/ : skipping foo/
// - foo : skipping foo. // - foo : skipping foo.

View File

@ -36,13 +36,25 @@ describe('Delimiter All masters listing algorithm', () => {
assert.strictEqual(delimiter.skipping(), SKIP_NONE); assert.strictEqual(delimiter.skipping(), SKIP_NONE);
}); });
it('should return SKIP_NONE for DelimiterMaster when NextMarker is ' +
'set to listing marker parameter value', () => {
const markerParams = 'marker';
const delimiter = new DelimiterMaster({
delimiter: '/',
marker: markerParams,
});
/* When there is no NextMarker, it should return SKIP_NONE. */
delimiter.NextMarker = markerParams;
assert.strictEqual(delimiter.skipping(), SKIP_NONE);
});
it('should return <key><VersionIdSeparator> for DelimiterMaster when ' + it('should return <key><VersionIdSeparator> for DelimiterMaster when ' +
'NextMarker is set and there is a delimiter', () => { 'NextMarker is set and there is a delimiter', () => {
const key = 'key'; const key = 'key';
const delimiter = new DelimiterMaster({ delimiter: '/', marker: key }); const delimiter = new DelimiterMaster({ delimiter: '/' });
/* Filter a master version to set NextMarker. */ /* Filter a master version to set NextMarker. */
// TODO: useless once S3C-1628 is fixed.
delimiter.filter({ key, value: '' }); delimiter.filter({ key, value: '' });
assert.strictEqual(delimiter.NextMarker, key); assert.strictEqual(delimiter.NextMarker, key);
@ -58,12 +70,14 @@ describe('Delimiter All masters listing algorithm', () => {
const keyWithEndingDelimiter = `key${delimiterChar}`; const keyWithEndingDelimiter = `key${delimiterChar}`;
const delimiter = new DelimiterMaster({ const delimiter = new DelimiterMaster({
delimiter: delimiterChar, delimiter: delimiterChar,
marker: keyWithEndingDelimiter,
}); });
/* Filter a master version to set NextMarker. */
delimiter.filter({ key: keyWithEndingDelimiter, value: '' });
assert.strictEqual(delimiter.NextMarker, keyWithEndingDelimiter);
/* When a delimiter is set and the NextMarker ends with the /* When a delimiter is set and the NextMarker ends with the
* delimiter it should return the next marker value. */ * delimiter it should return the next marker value. */
assert.strictEqual(delimiter.NextMarker, keyWithEndingDelimiter);
assert.strictEqual(delimiter.skipping(), keyWithEndingDelimiter); assert.strictEqual(delimiter.skipping(), keyWithEndingDelimiter);
}); });