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

477 history migration not working #496

Merged
merged 39 commits into from
Mar 28, 2022
Merged

Conversation

yflim
Copy link
Collaborator

@yflim yflim commented Mar 8, 2022

SUMMARY

Include historical datoms in datahike.migrate export, so history is not lost in migration as would happen currently. #477

Checks

Bugfix
  • Related issues linked using fixes #number
  • Integration tests added
  • Architecture Decision Record added if design changes necessary
  • Formatting checked
  • CHANGELOG.md updated

@yflim yflim requested review from kordano and jsmassa March 8, 2022 10:11
@yflim
Copy link
Collaborator Author

yflim commented Mar 8, 2022

I'm benchmarking 3 approaches: based on wanderung.datahike, sorting using the Scicloj Tablecloth API, and vanilla Clojure sorting.

Benchmarking parameters used were backend and index type, consisting of (in order) mem i.e. persistent-set or hitchhiker-tree indices, or file; and number of entities, in this case 0, 1000, 10000, 100000, or 1000000. All the approaches triggered an out-of-memory error at the 1M-entity stage, though the Wanderung approach failed sooner, failing at the persistent-set stage, whereas the others failed at hht.

@yflim yflim force-pushed the 477-history-migration-not-working branch 2 times, most recently from dab6494 to b6dc12a Compare March 10, 2022 23:39
Copy link
Collaborator

@jsmassa jsmassa left a comment

Choose a reason for hiding this comment

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

Thanks for cleaning up the benchmark namespaces a bit. In the future, please make things like that a separate PR though with an appropriate subject.

benchmark/src/benchmark/measure.clj Outdated Show resolved Hide resolved
benchmark/src/benchmark/measure.clj Outdated Show resolved Hide resolved
benchmark/src/benchmark/measure.clj Outdated Show resolved Hide resolved
@yflim yflim force-pushed the 477-history-migration-not-working branch from 756605a to 3ee4288 Compare March 13, 2022 00:46
@yflim
Copy link
Collaborator Author

yflim commented Mar 13, 2022

export-db-clj consistently performed far better in speed (and also memory usage, though that hasn't been quantified) than the other approaches, export-db-wanderung and export-db-tc—see attached pretty-printed comparison map. Moreover, (I've just noticed) the latter doesn't support byte arrays, which is likely a dealbreaker. So I've replaced export-db with export-db-clj in commit 33a8c1a.
export-approach-figures.txt

@yflim yflim force-pushed the 477-history-migration-not-working branch from 8d04e9e to b0abb7a Compare March 24, 2022 01:56
@yflim yflim marked this pull request as ready for review March 24, 2022 02:00
@yflim yflim requested a review from jsmassa March 24, 2022 02:00
@yflim yflim force-pushed the 477-history-migration-not-working branch from b0abb7a to 1fb9f03 Compare March 24, 2022 04:40
@yflim yflim force-pushed the 477-history-migration-not-working branch from 1fb9f03 to d44f9c1 Compare March 25, 2022 00:36
yflim added 20 commits March 24, 2022 17:47
…ring each benchmarking iteration into one point
…hmarking measurements on separate database instances
@yflim yflim force-pushed the 477-history-migration-not-working branch from d44f9c1 to 00323f7 Compare March 25, 2022 02:11
src/datahike/migrate.clj Outdated Show resolved Hide resolved
src/datahike/migrate.clj Show resolved Hide resolved
test/datahike/test/migrate_test.clj Outdated Show resolved Hide resolved
@yflim yflim requested a review from kordano March 25, 2022 18:17
@kordano kordano merged commit c2dc343 into main Mar 28, 2022
@kordano kordano deleted the 477-history-migration-not-working branch March 28, 2022 13:33
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