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.

Sign up for GitHub

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

Jump to bottom

Value based offset #750

Merged
merged 3 commits into from
Aug 18, 2020
Merged

Value based offset #750

merged 3 commits into from
Aug 18, 2020

Conversation

porbas
Copy link
Contributor

@porbas porbas commented Jul 30, 2020

Hi @mpraglowski
I've closed #746 as it was really messed up with commits, merges and rebases. Here is clean commits history.

porbas added 3 commits July 30, 2020 09:16
@porbas
use value based batching in rails_event_store_active_record
0a114bc
@porbas
move RailsEventStoreActiveRecord::BatchEnumerator to its own file
023d840 
* add tests for it
* make it's initializer signature simpler and more consistent
  with existing RubyEventStore::BatchEnumerator
@porbas
kill mutants
e2ecaed 
100% coverage for RailsEventStoreActiveRecord::EventRepositoryReader#read
@mpraglowski
Copy link
Member

@porbas could you make some benchmark of speed improvements? You could base the benchmark code on https://github.com/mlomnicki/res-benchmarks - with my changes in mlomnicki/res-benchmarks#1 - but you will need to adjust the read code to make sure you check reading in batches on a large set of domain events.

@porbas
Copy link
Contributor Author

porbas commented Jul 30, 2020

Yes, I've already started working on benchmarks but it is not easy to set up. Thanks for links. I hope it will be easier now!

@mpraglowski
Copy link
Member

You could also use https://github.com/RailsEventStore/rails_event_store/tree/master/contrib/measure to build your benchmark code.

@porbas
Copy link
Contributor Author

porbas commented Jul 31, 2020 •
edited
Loading

Benchmark prepared https://github.com/porbas/res-benchmarks and executed for 200_000 events (PostgreSQL 12):

Benchmarking master
Calculating -------------------------------------
              master      0.006  (± 0.0%) i/s -      1.000  in 166.054008s
Benchmarking value_based_offset
Calculating -------------------------------------
  value_based_offset      0.008  (± 0.0%) i/s -      1.000  in 126.368918s

Comparison:
  value_based_offset:        0.0 i/s
              master:        0.0 i/s - 1.31x  (± 0.00) slower

real	21m42,669s
user	14m49,223s
sys	0m41,528s

it took about 20minutes to complete.

Now I'm going to run benchmark for 600_000 events

@porbas
Copy link
Contributor Author

porbas commented Jul 31, 2020

Benchmarks for 600_000 events took about 75 minutes. Now difference is about 2x:

Calculating -------------------------------------
              master      0.001  (± 0.0%) i/s -      1.000  in 925.651214s
Benchmarking value_based_offset
Calculating -------------------------------------
  value_based_offset      0.002  (± 0.0%) i/s -      1.000  in 447.880005s

Comparison:
  value_based_offset:        0.0 i/s
              master:        0.0 i/s - 2.07x  (± 0.00) slower

real	74m59,886s
user	45m48,459s
sys	1m56,759s

@porbas
Copy link
Contributor Author

porbas commented Aug 16, 2020

This is just friendly reminder 😃

@mostlyobvious
Copy link
Member

mostlyobvious commented Aug 18, 2020 •
edited
Loading

Good base to improve upon, thanks!

@mostlyobvious mostlyobvious merged commit e4c8936 into RailsEventStore:master Aug 18, 2020
@porbas porbas deleted the value_based_offset branch August 20, 2020 09:10
@adriancb adriancb mentioned this pull request Oct 7, 2020
Closed
mostlyobvious added a commit that referenced this pull request Apr 8, 2021
@mostlyobvious
Fallback to offset-based BatchEnumerator when monotonicity is not gua…
173ea4a 
…ranteed

When monotonicity cannot be guaranteed, use
RubyEventStore::BatchEnumerator operating on offset + limit.

That has some performance penalty of OFFSET + LIMIT in databases engines
on large data sets. Probably something to ignore on smaller streams —
correctness comes first.

#750 (comment)
#1037
#810 (comment)
mostlyobvious added a commit that referenced this pull request Apr 8, 2021
@mostlyobvious
Fallback to offset-based BatchEnumerator when monotonicity is not gua…
34b365f 
…ranteed

When monotonicity cannot be guaranteed, use
RubyEventStore::BatchEnumerator operating on offset + limit.

That has some performance penalty of OFFSET + LIMIT in databases engines
on large data sets. Probably something to ignore on smaller streams —
correctness comes first.

#750 (comment)
#1037
#810 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@porbas @mpraglowski @mostlyobvious