Fix #721, Add batching to flat file imports - #845
Merged
Conversation
whilo
added a commit
that referenced
this pull request
Aug 12, 2026
`update-max-tx` and `*import-batch-size*` are not new: they date from af6fe0c, are on origin/main, and are in tags back to v0.3.6-SNAPSHOT. This branch did not delete them — it moved them to `datahike.migrate.legacy` along with the single-file reader they serve. The two are not equal in standing, though, and the move treated them as if they were. CHANGELOG line 129 announces one of them by its fully-qualified name: **Imports are now batched** — `datahike.migrate/import-db` now imports flat-files in configurable batches (`datahike.migrate/*import-batch-size*`, default `10000`) instead of one transaction. ([#845]) So `*import-batch-size*` goes back to `datahike.migrate`. Its value is passed to `import-db-legacy` as an argument rather than read there, so it is read at the call site and therefore inside the caller's `binding` scope; `import-db` still detects a legacy dump and routes to it, so a binding written against that CHANGELOG entry behaves exactly as before. Two things ruled out, both recorded in the docstring so they are not retried: it cannot be defined in `legacy` and referred to from here, because `datahike.migrate` already requires that namespace and the reverse require is a cycle; and it cannot be aliased, because an alias is a NEW var — `(binding [datahike.migrate/*import-batch-size* 5] …)` would set something nothing reads, which is a silent no-op where today's behaviour is a loud compile error. `update-max-tx` stays in `legacy`, deprecated: never documented anywhere, and `max-tx` is maintained by `load-entities` now. CHANGELOG records the move. Line 129 needs no correction — with the var back, it is true as written. The test that covered this was decorative. It bound the var and then asserted only on the imported datoms, which are identical at any batch size, so it passed whether or not the binding was plumbed through. Two candidate observables do not work either: transaction entities, and the datoms' `:tx` — a legacy import PRESERVES the tx from the dump (that is what `update-max-tx` is for) and every datom in the fixture carries 536870913. The batch sizes handed to `transact` are the honest signal. Red-checked by bypassing the binding: `got [12]` against `[5 5 2]`. JVM 3022/35076/0, Node 252/1511/0.
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
2 participants
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.
SUMMARY
Println for
migrate/import-dbmentioned batching transactions but wasn't actually implemented, leading to file watcher crashes #721. PR implements actual batching with a configurable var and sets the default to 10k per the #721 (comment). Used a var instead of a config map to maintain the fn signature, but I could go either way.I did see #727 and ultimately it's the better long-term solution. But, I do feel adding just batching keeps the scope smaller and the other changes can be worked in later.
Checks
Feature
fixes #721ADDITIONAL INFORMATION