Improving Implement buffered document importing for improved performance#137
Merged
Conversation
Co-authored-by: xingfan-git <50465320+xingfan-git@users.noreply.github.com>
Co-authored-by: xingfan-git <50465320+xingfan-git@users.noreply.github.com>
- Copy documentBuffer.ts from sample PR, excluding cosmos-related code - Update ClustersClient.ts InsertDocumentsResult and insertDocuments method - Update importDocuments.ts with buffering logic following sample PR - Rename insertDocumentIntoCluster to insertDocumentWithBufferIntoCluster - Update l10n bundle with new error message Co-authored-by: xingfan-git <50465320+xingfan-git@users.noreply.github.com>
…-documentdb into copilot/fix-130
Co-authored-by: tnaum-ms <171359267+tnaum-ms@users.noreply.github.com>
Co-authored-by: tnaum-ms <171359267+tnaum-ms@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR adds a buffering layer for batched document inserts to improve performance and updates import logic to leverage that buffer, while also enhancing error handling and progress reporting.
- Introduces
DocumentBufferwith configurable thresholds and a factory for MongoDB defaults. - Extends
ClustersClientto handle unordered bulk writes and log detailed write errors. - Refactors
importDocumentsto use the buffer for bulk inserts, adjust progress increments, and localize final messages.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/utils/documentBuffer.ts | Adds new DocumentBuffer class and createMongoDbBuffer utility for batched writes. |
| src/documentdb/ClustersClient.ts | Wraps insertMany in try/catch, logs bulk write errors. |
| src/commands/importDocuments/importDocuments.ts | Refactors import flow to use buffering, updates progress reporting and error accumulation. |
| l10n/bundle.l10n.json | Updates localized strings for new final messages and write errors. |
Comments suppressed due to low confidence (3)
src/utils/documentBuffer.ts:93
- Add unit tests for DocumentBuffer to cover insertion, flushing, and error scenarios (e.g., oversized documents, empty documents).
export class DocumentBuffer<T> {
src/utils/documentBuffer.ts:138
- [nitpick]
documentsToProcessisundefinedon success but an array in other cases; consider always returning an array (empty on success) for consistency and simpler downstream handling.
return { ...insertResult, documentsToProcess: undefined };
src/documentdb/ClustersClient.ts:496
- Calling
ext.outputChannel.show()inside the loop can trigger the UI repeatedly; move this call after logging all write errors to show the output channel only once.
ext.outputChannel.show();
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #130
Fixes #136