[DCP Ingestion]Jsonld stream#635
Conversation
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| UnusedCode | 2 medium 1 minor |
| ErrorProne | 2 high |
| CodeStyle | 4 minor |
🟢 Metrics 20 complexity
Metric Results Complexity 20
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
There was a problem hiding this comment.
Code Review
This pull request transitions the MCF importer and JSON-LD stream database to a streaming architecture to reduce memory usage, writing triples in chunks to a local temporary directory during import. The feedback highlights a critical bug in the MCF importer where clearing the local2dcid map mid-stream can cause unresolved local IDs or crashes, and notes that metadata registration should be deferred to the end of the import to handle split blocks. Additionally, it is recommended to replace tempfile.mkdtemp() with tempfile.TemporaryDirectory() to ensure robust cleanup of temporary files even in the event of failures.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request transitions the JSON-LD database and MCF importer to stream triples in chunks to disk and the database, significantly reducing memory usage. The review feedback highlights several critical improvements: avoiding parsing the MCF file twice to reduce CPU and memory overhead, ensuring triple chunking respects subject boundaries to prevent splitting a subject's triples across different database inserts, correcting a directory existence check that is always true, and cleaning up temporary directories in tests to prevent resource leaks.
This PR updates the jsonld streaming DB to really operate as a streaming DB. We only load the 10k nodes until we export to JSONLD and move on to the next batch.
This became a requirement as CloudSufi was experiencing a full crash out of the preprocessor job due to a very large UN dataset. I've tested the same dataset and the memory consumption dropped with peak memory seemingly around 6GB compared to nearly 30 previously on the same dataset.