Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: improve memory usage of record import #218

Merged
merged 27 commits into from
Feb 21, 2023

Conversation

tasshi-me
Copy link
Member

@tasshi-me tasshi-me commented Feb 4, 2023

Why

#102

What

  1. read records iterative using Stream instead of reading all records to memory at once.

Design

I abstracted the data source, CSV file on the file system with repository.
It provides the reader to read the LocalRecord iterative.
usecase reads records by chunk size and processes them.
usecase doesn't have any knowledge of details (csv format, encoding, file system, and, command line)

The reader provides an async generator of records.
We read the records from the file system using Stream, but it makes code complicated to use Stream whole program.
So we encapsulated the Stream to an async generator.

How to test

yarn build
yarn test

Checklist

  • Read CONTRIBUTING.md
  • Updated documentation if it is required.
  • Added tests if it is required.
  • Passed yarn lint and yarn test on the root directory.

@tasshi-me tasshi-me force-pushed the feat/improve-memory-usage-of-import branch from ab4e1b2 to 53935f7 Compare February 11, 2023 05:45
@tasshi-me tasshi-me marked this pull request as ready for review February 14, 2023 07:42
@tasshi-me tasshi-me requested review from a team, hung-cybo and ueokande and removed request for a team February 14, 2023 07:42
@tasshi-me tasshi-me force-pushed the feat/improve-memory-usage-of-import branch from 34b13e4 to 8cede92 Compare February 14, 2023 12:33
Copy link
Member

@ueokande ueokande left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I comment at this moment.

src/record/import/repositories/error.ts Show resolved Hide resolved
src/record/import/usecases/interface.ts Show resolved Hide resolved
src/record/import/usecases/upsert/updateKey.ts Outdated Show resolved Hide resolved
src/record/import/utils/file.ts Outdated Show resolved Hide resolved
src/record/import/repositories/parsers/parseCsv/record.ts Outdated Show resolved Hide resolved
src/record/import/usecases/add.ts Outdated Show resolved Hide resolved
src/record/import/types/record.ts Outdated Show resolved Hide resolved
src/record/import/repositories/parsers/parseCsv/index.ts Outdated Show resolved Hide resolved
@tasshi-me tasshi-me force-pushed the feat/improve-memory-usage-of-import branch from 5b40070 to 0336314 Compare February 15, 2023 10:31
@tasshi-me tasshi-me merged commit c38aa94 into main Feb 21, 2023
@tasshi-me tasshi-me deleted the feat/improve-memory-usage-of-import branch February 21, 2023 07:38
This was referenced Jan 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants