Skip to content

Commit d1e1e63

Browse files
witashwitash
authored
fix(#11490): nouveau results limit (#11494)
1 parent e348da2 commit d1e1e63

6 files changed

Lines changed: 86 additions & 123 deletions

File tree

‎api/src/services/replication/authorization.js‎

Lines changed: 26 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -636,27 +636,33 @@ const getDocsByReplicationKeyNouveau = async (authorizationContext) => {
636636

637637
while (allKeys.length) {
638638
const chunk = allKeys.splice(0, nouveau.BATCH_LIMIT);
639-
const response = await request.post({
640-
uri: `${environment.couchUrl}/_design/medic/_nouveau/docs_by_replication_key`,
641-
body: {
642-
q: `key:(${chunk.map(nouveau.escapeKeys).join(' OR ')})`,
643-
limit: nouveau.RESULTS_LIMIT,
644-
}
645-
});
646-
647-
if (!response.hits || !response.hits.length) {
648-
continue;
649-
}
650-
651-
for (const hit of response.hits) {
652-
hits.push({
653-
id: hit.id,
654-
fields: {
655-
...hit.fields,
656-
key: Array.isArray(hit.fields.key) ? hit.fields.key : [hit.fields.key],
657-
},
639+
const body = {
640+
q: `key:(${chunk.map(nouveau.escapeKeys).join(' OR ')})`,
641+
limit: nouveau.RESULTS_LIMIT,
642+
};
643+
let bookmark;
644+
let previousBookmark;
645+
let pageHits;
646+
647+
do {
648+
previousBookmark = bookmark;
649+
const response = await request.post({
650+
uri: `${environment.couchUrl}/_design/medic/_nouveau/docs_by_replication_key`,
651+
body: { ...body, bookmark },
658652
});
659-
}
653+
654+
bookmark = response.bookmark;
655+
pageHits = response.hits || [];
656+
for (const hit of pageHits) {
657+
hits.push({
658+
id: hit.id,
659+
fields: {
660+
...hit.fields,
661+
key: Array.isArray(hit.fields.key) ? hit.fields.key : [hit.fields.key],
662+
},
663+
});
664+
}
665+
} while (bookmark && bookmark !== previousBookmark && pageHits.length >= nouveau.RESULTS_LIMIT);
660666
}
661667

662668
return hits;

‎api/tests/mocha/services/replication/authorization.spec.js‎

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -590,8 +590,9 @@ describe('Authorization service', () => {
590590
request.post.callCount.should.equal(1);
591591
request.post.args[0].should.deep.equal([ {
592592
body: {
593-
limit: 200000,
594-
q: 'key:(1 OR 2 OR 3)'
593+
limit: 20000,
594+
q: 'key:(1 OR 2 OR 3)',
595+
bookmark: undefined,
595596
},
596597
uri: 'http://localhost:5988/_design/medic/_nouveau/docs_by_replication_key'
597598
}]);
@@ -600,6 +601,27 @@ describe('Authorization service', () => {
600601
});
601602
});
602603

604+
it('should page through results using bookmarks', () => {
605+
const page = (start, length) => Array
606+
.from({ length })
607+
.map((_, idx) => ({ id: `r${start + idx}`, fields: { key: '1', subject: '1' } }));
608+
request.post.onCall(0).resolves({ hits: page(0, 20000), bookmark: 'b1' });
609+
request.post.onCall(1).resolves({ hits: page(20000, 20000), bookmark: 'b2' });
610+
request.post.onCall(2).resolves({ hits: page(40000, 10), bookmark: 'b3' });
611+
612+
return service
613+
.getDocsByReplicationKey({ subjectIds, userCtx: { name: 'user' }})
614+
.then(result => {
615+
request.post.callCount.should.equal(3);
616+
request.post.args.map(([{ body }]) => body).should.deep.equal([
617+
{ limit: 20000, q: 'key:(1 OR 2 OR 3)', bookmark: undefined },
618+
{ limit: 20000, q: 'key:(1 OR 2 OR 3)', bookmark: 'b1' },
619+
{ limit: 20000, q: 'key:(1 OR 2 OR 3)', bookmark: 'b2' },
620+
]);
621+
result.length.should.equal(40010);
622+
});
623+
});
624+
603625
it('merges results from both view, except for sensitive ones, includes ddoc and user doc', () => {
604626
const subjectIds = [ 'sbj1', 'sbj2', 'sbj3', 'sbj4', 'facility_id', 'contact_id', 'c1', 'c2', 'c3', 'c4' ];
605627
request.post.resolves({

‎sentinel/src/lib/purging.js‎

Lines changed: 12 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ const moment = require('moment');
1414
const { SENTINEL_METADATA } = require('@medic/constants');
1515

1616
const MAX_CONTACT_BATCH_SIZE = nouveau.BATCH_LIMIT;
17-
const MAX_BATCH_SIZE = 20 * 1000;
17+
const MAX_BATCH_SIZE = nouveau.RESULTS_LIMIT; // 20k
1818
const MIN_BATCH_SIZE = 5 * 1000;
1919
const MAX_BATCH_SIZE_REACHED = 'max_size_reached';
2020

@@ -193,15 +193,6 @@ const assignContactToGroups = (row, groups, subjectIds) => {
193193
subjectIds.push(...group.subjectIds);
194194
};
195195

196-
const isRelevantRecordEmission = (hit, subjectIds) => {
197-
// reports with `needs_signoff` will emit for every contact from their submitter lineage,
198-
// but we only want to process them once, either associated to their subject, or to their submitter
199-
// when they have no subject or have an invalid subject.
200-
// if the report has a subject, but it's not the same as the emission key, we hit the emit
201-
// for the submitter or submitter lineage via the `needs_signoff` path. Skip.
202-
return !(hit.fields.needs_signoff && !subjectIds.includes(hit.fields.subject));
203-
};
204-
205196
const getRecordGroupInfo = (row, subjectIds) => {
206197
if (row.doc.form) {
207198
const subjectId = registrationUtils.getSubjectId(row.doc);
@@ -258,33 +249,19 @@ const getRecordsForContacts = async (groups, subjectIds) => {
258249
};
259250

260251
const getRecordsForContactsBatch = async (subjectIds) => {
261-
const results = [];
262252
if (!subjectIds.length) {
263-
return results;
253+
return [];
264254
}
265255

266-
let bookmark = '';
267-
let requestNext;
268-
269-
do {
270-
const opts = {
271-
q: `key:(${subjectIds.map(nouveau.escapeKeys).join(' OR ')}) AND type:data_record`,
272-
limit: nouveau.RESULTS_LIMIT,
273-
};
274-
if (bookmark) {
275-
opts.bookmark = bookmark;
276-
}
277-
const result = await request.post({
278-
uri: `${environment.couchUrl}/_design/medic/_nouveau/docs_by_replication_key`,
279-
body: opts
280-
});
281-
282-
bookmark = result.bookmark;
283-
results.push(...result.hits.filter(hit => isRelevantRecordEmission(hit, subjectIds)));
284-
requestNext = bookmark && result.hits.length >= nouveau.RESULTS_LIMIT && results.length < MAX_BATCH_SIZE;
285-
} while (requestNext);
256+
const result = await request.post({
257+
uri: `${environment.couchUrl}/_design/medic/_nouveau/docs_by_replication_key`,
258+
body: {
259+
q: `subject:(${subjectIds.map(nouveau.escapeKeys).join(' OR ')}) AND type:data_record`,
260+
limit: MAX_BATCH_SIZE,
261+
},
262+
});
286263

287-
return results;
264+
return result.hits;
288265
};
289266

290267
const getRecordsByKey = (rows, subjectIds) => {
@@ -577,8 +554,8 @@ const writePurgeLog = (roles, start, error) => {
577554
// - we intentionally skip purging "orphaned" docs (docs that emit in `docs_by_replication_key` but that are not
578555
// retrieved when systematically querying the view with all existent subjects), as these docs would not end up being
579556
// replicated
580-
// - we intentionally skip reports that `needs_signoff` when they are retrieved because of the `needs_signoff`
581-
// submitter lineage emit. As a consequence, orphaned reports with `needs_signoff` will not be purged
557+
// - we query by subject, so reports that `needs_signoff` are not retrieved through their submitter lineage emits.
558+
// As a consequence, orphaned reports with `needs_signoff` will not be purged
582559

583560
const purge = () => {
584561
if (currentlyPurging) {

‎sentinel/tests/unit/lib/purging.spec.js‎

Lines changed: 22 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -625,7 +625,7 @@ describe('ServerSidePurge', () => {
625625
});
626626
});
627627

628-
it('should keep requesting docs_by_replication_key and select only relevant records to purge', () => {
628+
it('should request docs_by_replication_key once per chunk of subject ids when pages are not full', () => {
629629
sinon.stub(db, 'queryMedic');
630630
const contacts = Array.from({ length: 1500 }).map((_, idx) => ({
631631
id: idx,
@@ -636,26 +636,20 @@ describe('ServerSidePurge', () => {
636636
return Promise.resolve({ rows: contacts.slice(startkey_docid, limit + startkey_docid) });
637637
});
638638

639-
// 1 in 5000 records is relevant
640-
const fiveK = 5000;
641-
const reports = Array.from({ length: 430000 }).map((_, idx) => ({
639+
const reports = Array.from({ length: 10600 }).map((_, idx) => ({
642640
id: `id${idx}`,
643641
fields: {
644642
key: `key${idx}`,
645-
subject: 'irrelevant',
643+
subject: `key${idx}`,
646644
type: DOC_TYPES.DATA_RECORD,
647-
needs_signoff: idx % fiveK !== 0
648645
},
649646
}));
650647

651648
sinon.stub(request, 'post');
652-
request.post.onCall(0).resolves({ hits: reports.slice(0, 200000), bookmark: '1' });
653-
request.post.onCall(1).resolves({ hits: reports.slice(200000, 220000) });
654-
655-
request.post.onCall(2).resolves({ hits: reports.slice(220000, 420000), bookmark: '2' });
656-
request.post.onCall(3).resolves({ hits: reports.slice(420000, 430000) });
649+
request.post.onCall(0).resolves({ hits: reports.slice(0, 10000), bookmark: '1' });
650+
request.post.onCall(1).resolves({ hits: reports.slice(10000, 10600), bookmark: '2' });
657651

658-
request.post.onCall(4).resolves({ hits: reports.slice(0, 20000) });
652+
request.post.onCall(2).resolves({ hits: reports.slice(0, 200), bookmark: '3' });
659653

660654
sinon.stub(db.medic, 'allDocs')
661655
.callsFake(({ keys }) => Promise.resolve({ rows: keys.map((key) => ({ doc: { _id: key } }))}));
@@ -665,45 +659,21 @@ describe('ServerSidePurge', () => {
665659

666660
return service.__get__('purgeContacts')(roles, purgeFn).then(() => {
667661
chai.expect(db.queryMedic.callCount).to.equal(3);
668-
chai.expect(request.post.callCount).to.equal(5);
669-
670-
chai.expect(request.post.args[0]).excludingEvery('q').to.deep.equal([{
671-
body: { limit: 200000 },
672-
uri: `${environment.couchUrl}/_design/medic/_nouveau/docs_by_replication_key`
673-
}]);
674-
chai.expect(request.post.args[1]).excludingEvery('q').to.deep.equal([{
675-
body: {
676-
limit: 200000,
677-
bookmark: '1',
678-
},
679-
uri: `${environment.couchUrl}/_design/medic/_nouveau/docs_by_replication_key`
680-
}]);
681-
682-
chai.expect(request.post.args[2]).excludingEvery('q').to.deep.equal([{
683-
body: { limit: 200000, },
684-
uri: `${environment.couchUrl}/_design/medic/_nouveau/docs_by_replication_key`
685-
}]);
686-
chai.expect(request.post.args[3]).excludingEvery('q').to.deep.equal([{
687-
body: { limit: 200000, bookmark: '2' },
688-
uri: `${environment.couchUrl}/_design/medic/_nouveau/docs_by_replication_key`
689-
}]);
690-
691-
chai.expect(request.post.args[4]).excludingEvery('q').to.deep.equal([{
692-
body: { limit: 200000, },
693-
uri: `${environment.couchUrl}/_design/medic/_nouveau/docs_by_replication_key`
694-
}]);
662+
chai.expect(request.post.callCount).to.equal(3);
663+
request.post.args.forEach(args => {
664+
chai.expect(args).excludingEvery('q').to.deep.equal([{
665+
body: { limit: 20000 },
666+
uri: `${environment.couchUrl}/_design/medic/_nouveau/docs_by_replication_key`
667+
}]);
668+
});
695669

696670
chai.expect(db.medic.allDocs.callCount).to.equal(2);
697671

698-
const getRelevantRecordsIds = (length) => Array
699-
.from({ length: length / fiveK })
700-
.map((_, idx) => `id${idx * fiveK}`);
701-
702672
chai.expect(db.medic.allDocs.args[0]).to.deep.equal([{
703-
keys: getRelevantRecordsIds(reports.length), include_docs: true,
673+
keys: reports.map(report => report.id), include_docs: true,
704674
}]);
705675
chai.expect(db.medic.allDocs.args[1]).to.deep.equal([{
706-
keys: getRelevantRecordsIds(20000), include_docs: true,
676+
keys: reports.slice(0, 200).map(report => report.id), include_docs: true,
707677
}]);
708678
chai.expect(service.__get__('contactsBatchSize')).to.equal(1000);
709679
});
@@ -876,8 +846,8 @@ describe('ServerSidePurge', () => {
876846
chai.expect(request.post.args[0]).to.deep.equal([{
877847
uri: `${environment.couchUrl}/_design/medic/_nouveau/docs_by_replication_key`,
878848
body: {
879-
q: 'key:(first OR f1 OR s1 OR f2 OR f4 OR s4) AND type:data_record',
880-
limit: 200000
849+
q: 'subject:(first OR f1 OR s1 OR f2 OR f4 OR s4) AND type:data_record',
850+
limit: 20000
881851
},
882852
} ]);
883853
chai.expect(db.medic.allDocs.callCount).to.equal(1);
@@ -1032,8 +1002,8 @@ describe('ServerSidePurge', () => {
10321002
chai.expect(request.post.args[0]).to.deep.equal([{
10331003
uri: `${environment.couchUrl}/_design/medic/_nouveau/docs_by_replication_key`,
10341004
body: {
1035-
q: 'key:(first OR f1 OR s1 OR f2 OR s2) AND type:data_record',
1036-
limit: 200000
1005+
q: 'subject:(first OR f1 OR s1 OR f2 OR s2) AND type:data_record',
1006+
limit: 20000
10371007
},
10381008
}]);
10391009
chai.expect(db.medic.allDocs.callCount).to.equal(1);
@@ -1088,7 +1058,7 @@ describe('ServerSidePurge', () => {
10881058
});
10891059
});
10901060

1091-
it('should correctly ignore reports with needs_signoff when they emit submitter lineage', () => {
1061+
it('should query reports with needs_signoff by subject rather than submitter lineage', () => {
10921062
sinon.stub(db, 'queryMedic');
10931063
db.queryMedic.onCall(0).resolves({ rows: [
10941064
{ id: 'first', key: 'district_hospital', doc: { _id: 'first', type: CONTACT_TYPES.DISTRICT_HOSPITAL } },
@@ -1112,22 +1082,10 @@ describe('ServerSidePurge', () => {
11121082
fields: { key: 's1', type: DOC_TYPES.DATA_RECORD, subject: 's1', needs_signoff: true, submitter: 'f1' }
11131083
},
11141084
{ id: 'f1-m1', fields: { key: 'f1', type: DOC_TYPES.DATA_RECORD, subject: 'f1', submitter: 'f1' } },
1115-
{
1116-
id: 'f2-r1',
1117-
fields: { key: 'f2', type: DOC_TYPES.DATA_RECORD, subject: 'random', needs_signoff: true, submitter: 'f2' }
1118-
},
1119-
{
1120-
id: 'f2-r2',
1121-
fields: { key: 'f2', type: DOC_TYPES.DATA_RECORD, subject: 'random', needs_signoff: true, submitter: 'other' }
1122-
},
11231085
{
11241086
id: 'f2-r3',
11251087
fields: { key: 'f2', type: DOC_TYPES.DATA_RECORD, needs_signoff: true, submitter: 'f2', subject: 'f2' }
11261088
},
1127-
{
1128-
id: 'f2-r4',
1129-
fields: { key: 'f2', type: DOC_TYPES.DATA_RECORD, needs_signoff: true, submitter: 'f2', subject: 'f6' }
1130-
},
11311089
] });
11321090
sinon.stub(db.medic, 'allDocs').onCall(0).resolves({ rows: [
11331091
{ id: 'f1-r1', doc: { _id: 'f1-r1',
@@ -1143,8 +1101,8 @@ describe('ServerSidePurge', () => {
11431101
chai.expect(request.post.args[0]).to.deep.equal([{
11441102
uri: `${environment.couchUrl}/_design/medic/_nouveau/docs_by_replication_key`,
11451103
body: {
1146-
q: 'key:(first OR f1 OR s1 OR f2) AND type:data_record',
1147-
limit: 200000
1104+
q: 'subject:(first OR f1 OR s1 OR f2) AND type:data_record',
1105+
limit: 20000
11481106
},
11491107
}]);
11501108
chai.expect(db.medic.allDocs.callCount).to.equal(1);

‎shared-libs/nouveau/src/index.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,5 +33,5 @@ const escapeKeys = (key) => {
3333
module.exports = {
3434
escapeKeys,
3535
BATCH_LIMIT: 1000,
36-
RESULTS_LIMIT: 200 * 1000,
36+
RESULTS_LIMIT: 20 * 1000,
3737
};

‎shared-libs/nouveau/test/index.spec.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ const { PREFIXES } = require('@medic/constants');
66
describe('nouveau utils', () => {
77
it('should return limits', () => {
88
expect(lib.BATCH_LIMIT).to.equal(1000);
9-
expect(lib.RESULTS_LIMIT).to.equal(200000);
9+
expect(lib.RESULTS_LIMIT).to.equal(20000);
1010
});
1111

1212
describe('escaping special characters', () => {

0 commit comments

Comments
 (0)