Skip to content

Commit 7c34845

Browse files
authored
fix(compass-import-export): stream import errors to the log file with proper back pressure COMPASS-7820 (#6151)
* stream import errors to file with back pressure * port import tests * update expectation to two errors * one callback per error now * rm comment * fix some bugs in our error transformation * ImportWriter unit tests
1 parent 48c875a commit 7c34845

File tree

15 files changed

+917
-1202
lines changed

15 files changed

+917
-1202
lines changed

packages/compass-e2e-tests/tests/collection-import.test.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1210,12 +1210,13 @@ describe('Collection import', function () {
12101210
.$(Selectors.closeToastButton(Selectors.ImportToast))
12111211
.waitForDisplayed();
12121212

1213-
// Displays first error in the toast and view log.
1213+
// Displays first two errors in the toast and view log.
1214+
// (It tries to display two, but it also limits the text)
12141215
const toastText = await toastElement.getText();
12151216
expect(toastText).to.include('Import completed 0/3 with errors:');
12161217
expect(
12171218
(toastText.match(/E11000 duplicate key error collection/g) || []).length
1218-
).to.equal(1);
1219+
).to.equal(2);
12191220
expect(toastText).to.include('VIEW LOG');
12201221

12211222
const logFilePath = path.resolve(

packages/compass-import-export/src/components/import-modal.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -274,7 +274,7 @@ function ImportModal({
274274
const mapStateToProps = (state: RootImportState) => ({
275275
ns: state.import.namespace,
276276
isOpen: state.import.isOpen,
277-
errors: state.import.errors,
277+
errors: state.import.firstErrors,
278278
fileType: state.import.fileType,
279279
fileName: state.import.fileName,
280280
status: state.import.status,

packages/compass-import-export/src/import/import-csv.spec.ts

Lines changed: 37 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -113,19 +113,9 @@ describe('importCSV', function () {
113113
});
114114

115115
expect(omit(result, 'biggestDocSize')).to.deep.equal({
116+
docsErrored: 0,
116117
docsProcessed: totalRows,
117118
docsWritten: totalRows,
118-
dbErrors: [],
119-
dbStats: {
120-
insertedCount: totalRows,
121-
matchedCount: 0,
122-
modifiedCount: 0,
123-
deletedCount: 0,
124-
upsertedCount: 0,
125-
ok: Math.ceil(totalRows / 1000),
126-
writeConcernErrors: [],
127-
writeErrors: [],
128-
},
129119
hasUnboundArray: false,
130120
});
131121

@@ -264,19 +254,9 @@ describe('importCSV', function () {
264254
});
265255

266256
expect(omit(result, 'biggestDocSize')).to.deep.equal({
257+
docsErrored: 0,
267258
docsProcessed: totalRows,
268259
docsWritten: totalRows,
269-
dbErrors: [],
270-
dbStats: {
271-
insertedCount: totalRows,
272-
matchedCount: 0,
273-
modifiedCount: 0,
274-
deletedCount: 0,
275-
upsertedCount: 0,
276-
ok: Math.ceil(totalRows / 1000),
277-
writeConcernErrors: [],
278-
writeErrors: [],
279-
},
280260
hasUnboundArray: false,
281261
});
282262

@@ -356,19 +336,9 @@ describe('importCSV', function () {
356336
expect(errorLog).to.equal('');
357337

358338
expect(omit(result, 'biggestDocSize')).to.deep.equal({
339+
docsErrored: 0,
359340
docsProcessed: totalRows,
360341
docsWritten: totalRows,
361-
dbErrors: [],
362-
dbStats: {
363-
insertedCount: totalRows,
364-
matchedCount: 0,
365-
modifiedCount: 0,
366-
deletedCount: 0,
367-
upsertedCount: 0,
368-
ok: Math.ceil(totalRows / 1000),
369-
writeConcernErrors: [],
370-
writeErrors: [],
371-
},
372342
hasUnboundArray: false,
373343
});
374344

@@ -429,19 +399,9 @@ describe('importCSV', function () {
429399
expect(errorLog).to.equal('');
430400

431401
expect(omit(result, 'biggestDocSize')).to.deep.equal({
402+
docsErrored: 0,
432403
docsProcessed: 2000,
433404
docsWritten: 2000,
434-
dbErrors: [],
435-
dbStats: {
436-
insertedCount: 2000,
437-
matchedCount: 0,
438-
modifiedCount: 0,
439-
deletedCount: 0,
440-
upsertedCount: 0,
441-
ok: 2, // expected two batches
442-
writeConcernErrors: [],
443-
writeErrors: [],
444-
},
445405
hasUnboundArray: false,
446406
});
447407

@@ -670,7 +630,12 @@ describe('importCSV', function () {
670630
errorCallback,
671631
});
672632

673-
expect(result.dbStats.insertedCount).to.equal(1);
633+
expect(omit(result, 'biggestDocSize')).to.deep.equal({
634+
docsErrored: 2,
635+
docsProcessed: 3,
636+
docsWritten: 1,
637+
hasUnboundArray: false,
638+
});
674639

675640
expect(progressCallback.callCount).to.equal(3);
676641
expect(errorCallback.callCount).to.equal(2);
@@ -778,43 +743,45 @@ describe('importCSV', function () {
778743
errorCallback,
779744
});
780745

781-
expect(result.dbStats.insertedCount).to.equal(0);
746+
expect(omit(result, 'biggestDocSize')).to.deep.equal({
747+
docsErrored: 3,
748+
docsProcessed: 3,
749+
docsWritten: 0,
750+
hasUnboundArray: false,
751+
});
782752

783753
expect(progressCallback.callCount).to.equal(3);
784-
expect(errorCallback.callCount).to.equal(1); // once for the batch
754+
expect(errorCallback.callCount).to.equal(3);
785755

786756
const expectedErrors: ErrorJSON[] = [
787757
{
788-
name: 'MongoBulkWriteError',
758+
name: 'WriteError',
789759
message: 'Document failed validation',
760+
index: 0,
761+
code: 121,
762+
},
763+
{
764+
name: 'WriteError',
765+
message: 'Document failed validation',
766+
index: 1,
767+
code: 121,
768+
},
769+
{
770+
name: 'WriteError',
771+
message: 'Document failed validation',
772+
index: 2,
790773
code: 121,
791-
numErrors: 3,
792774
},
793775
];
794776

795777
const errors = errorCallback.args.map((args) => args[0]);
778+
for (const [index, error] of errors.entries()) {
779+
expect(error.op).to.exist;
780+
// cheat and copy them over because it is big and with buffers
781+
expectedErrors[index].op = error.op;
782+
}
796783
expect(errors).to.deep.equal(expectedErrors);
797784

798-
// the log file has one for each error in the bulk write too
799-
expectedErrors.push({
800-
name: 'WriteConcernError',
801-
message: 'Document failed validation',
802-
index: 0,
803-
code: 121,
804-
});
805-
expectedErrors.push({
806-
name: 'WriteConcernError',
807-
message: 'Document failed validation',
808-
index: 1,
809-
code: 121,
810-
});
811-
expectedErrors.push({
812-
name: 'WriteConcernError',
813-
message: 'Document failed validation',
814-
index: 2,
815-
code: 121,
816-
});
817-
818785
const errorsText = await fs.promises.readFile(output.path, 'utf8');
819786
expect(errorsText).to.equal(formatErrorLines(expectedErrors));
820787
});
@@ -842,19 +809,9 @@ describe('importCSV', function () {
842809
// only looked at the first row because we aborted before even starting
843810
expect(omit(result, 'biggestDocSize')).to.deep.equal({
844811
aborted: true,
812+
docsErrored: 0,
845813
docsProcessed: 0,
846814
docsWritten: 0,
847-
dbErrors: [],
848-
dbStats: {
849-
insertedCount: 0,
850-
matchedCount: 0,
851-
modifiedCount: 0,
852-
deletedCount: 0,
853-
upsertedCount: 0,
854-
ok: 0,
855-
writeConcernErrors: [],
856-
writeErrors: [],
857-
},
858815
hasUnboundArray: false,
859816
});
860817
});

0 commit comments

Comments
 (0)