prooph / pdo-event-store

PDO implementation of ProophEventStore http://getprooph.org
BSD 3-Clause "New" or "Revised" License
111 stars 56 forks source link

don't execute count statement on load #208

Closed prolic closed 5 years ago

prolic commented 5 years ago

superseeds https://github.com/prooph/pdo-event-store/pull/207

/cc @basz @fritz-gerneth @codeliner @unhappyby

The tests still pass, so this should work without making any BC. Please let me know what you think.

coveralls commented 5 years ago

Coverage Status

Coverage increased (+0.4%) to 87.48% when pulling d26d91ab5738f77d7485df99b5e7d8cede2e9746 on cnt_statement into 423ff66107ad0531fa6ab08022a3067adcafd2cf on master.

unhappyby commented 5 years ago

I think this is not solving the problem which I describe in this pull request, maybe we need to remove execution in iterator, but not in the event store.

basz commented 5 years ago

Please explain why you believe this would not solve the issue.

The PdoStreamIterator does not call count while iterating. So something like the following simply works.

/** @var EventStore $eventStore */
$eventStore = $container->get(EventStore::class);
$streamIterator = $eventStore->load(new StreamName('stream_name'), 500);

foreach ($streamIterator as $event) {
  // $event 500 to end (even while added after start)
}

The removed code in this PR is here only to be able to return an EmptyIterator (useless in my opinion anyway, not sure why it exists...).

unhappyby commented 5 years ago

Yes, this code working. But when I use event store in two or more processes (for writing and reading in parallel) some times event store lose events for projections. For example: For 20000 events event store lose from 1 to 70 events in my project. That happening because in db, table projections, position sets from count, which I get from $countStatement in load method. So I think that this PR can make things worse because event store will execute $countStatement in PdoStreamIterator and this means that gap in time between select and count statements will be bigger.

basz commented 5 years ago

I understand the issue but am not sure your problem comes from the PdoStreamIterator as that does not use the $countStatement unless you specifically call count() on it. In a normal iteration (foreach) it is not used.

basz commented 5 years ago

Do you use a loop (with count) in a different usecase? (Have code?)

prolic commented 5 years ago

@unhappyby Did you check this PR indepth? As @basz pointed out, the projections don't call count(). This PR should indeed resolve your issue (and also give slightly more performance for querying).

codeliner commented 5 years ago

Going to merge this one and make a new bugfix release together with #210