Remove node streams-based ingest code to prepare for post-ingest notifications

[jwalgran]

Related Issue(s):

• add publish SNS for newly ingested Items #45

Proposed Changes:

1. Remove logic from ingestItem and have it call ingestItems with a single-item list.
2. Remove node streams-based ingest by extraction functionality into lib/ingest.js and calling it directly from ingestItems
3. Remove batch processing functionality (at least temporarily)

My initial approach to preparing the code to support post-ingest notifications was to make a minimally invasive change that expanded the existing node streams-based implementation. Theoretically converting the Writable subclass to a Duplex subclass so that items would be written to the DB and then the results would be made available to downstream consumer ( eventually an SNS notification) would be small in scope.

I abandoned this approach for a number of reasons.

• Unresolved hanging problem in the Duplex implementation. The first item would be processed but failed to move forward.
  • Google searching did not lead to an effective solution
  • I also tried using ChatGPT to suggest solution that resulted in the same hanging issues
• The current project maintainers do not have a regular working knowledge of the node streams API
• The stream processing switched between single and bulk operations in a way that is was not directly controllable. Code within the node streams API decides when to call _write to process a single item or _writev to process multiple items.
• Streams are most helpful when dealing with large inputs because they allow avoiding the need to hold the entire set of data to be processed in memory. In this case the data to be processed is already all in memory.
• The INGEST_BATCH_SIZE variable was misleading. It was only used to set highWaterMark of the SearchDatabaseWritableStream
  • This is a threshold used by the buffering mechanism of node streams https://nodejs.org/dist/latest-v16.x/docs/api/stream.html#buffering
  • Based on my reading it does not directly control batch size https://stackoverflow.com/questions/67435206/nodejs-writable-streams-writev-alternating-between-1-and-highwatermark-chunks

This PR removes the node-stream based processing by extracting functionality from the lib/databaseStream module and moving it into lib/ingest where it can be called directly.

Unmerged PR #131 was a valuable reference but I did not adapt it directly because it was opened against a much older version of the codebase and it mixed removing the node streams-based implementation with SNS notifications. Follow up PRs to add this notification functionality will likely borrow more heavily from this previous attempt.

ingestItem and ingestItems differed only in their logging, so to reduce duplication I have removed all the logic from ingestItem and it now just calls ingestItems

Batch processing is removed from this implementation. ingestItems saves items to the database sequentially, one at a time. Allowing items to be created in parallel without using the bulk API resulted in test failures. If this is a significant performance issue in a high-volume setting it can be restored in a future pull request. Though not a load test, I did time the running of ./bin/system-tests.sh 3 time on both the main branch and this branch as a minimal test that this did not change performance in small-load situations.

main
real 1m4.807s
real 0m58.459s
real 0m59.376s

jcw/remove-stream-based-ingest
real 0m57.250s
real 0m57.398s
real 0m58.861s

PR Checklist:

• I have added my changes to the CHANGELOG or a CHANGELOG entry is not required.

[jwalgran]

Thanks for the review @philvarner. I pushed 0d22f41 with removes ingestItem. It turns out this was not used anywhere in the codebase so there was no need to change any other code.

I implemented a fix for #486 in separate PR #492 and targeted it at this branch, just to clearly separate the two.

Both this PR and #492 are ready for review.