[WIP] adds some first implementations #1
Conversation
oqq
commented
Feb 11, 2017
•
oqq
commented
- todos in code
- add unit tests
- some integration tests fails
- performance would be bad
- add persistence strategies
- missing travis and cs fixer configs
- missing doc headers
- implement missing methods
- enhance integration tests (with implementation)
|
@codeliner @prolic @sandrokeil |
src/RedisEventStore.php
Outdated
| $this->redisClient->zAdd('event_version:'.$streamNameKey, $aggregateVersion, $storageKey); | ||
|
|
||
| # todo: maybe we using a hash here? | ||
| $this->redisClient->set($storageKey, json_encode([ |
There was a problem hiding this comment.
is it possible to set multiple events at once, to reduce overhead over the wire?
There was a problem hiding this comment.
Yep.. i have to use Lua scripts. Would be implemented with persistence strategies.
Also, how I understand it, if a transaction is started requests would only be done on commit.
There was a problem hiding this comment.
Okay, I have no big knowledge of redis, but I compare it to a mysql example:
INSERT INTO foo (bar, baz) VALUES (1,2);
INSERT INTO foo (bar, baz) VALUES (2,4);
INSERT INTO foo (bar, baz) VALUES (3,6);
executed as single query-string (so there is only one request going over wire) is still much slower than:
INSERT INTO foo (bar, baz) VALUES (1,2), (2,4), (3,6);
src/RedisEventStore.php
Outdated
| if (false === $eventData) { | ||
| # todo: data for key was not found. Throw an exception? | ||
| throw new RuntimeException(); | ||
| continue; |
There was a problem hiding this comment.
continue not needed, if exception is thrown
src/RedisEventStore.php
Outdated
| MetadataMatcher $metadataMatcher = null | ||
| ): Iterator { | ||
| $streamNameKey = $this->getKeyFromStreamName($streamName); | ||
| $result = new \ArrayIterator(); |
There was a problem hiding this comment.
this is kind of bad - imagine you have 5mio events in a single stream - you would need to load all of them into memory. maybe something like https://github.com/prooph/pdo-event-store/blob/master/src/PdoStreamIterator.php should be used here.
There was a problem hiding this comment.
Sure.. I have started with yield but some tests would rewind the iterator. Redis has scan methods for batching.
|
@prolic i have add a travis configuration. Please activate the hook for travis in repository config. |
|
@oqq coveralls, travis and packagist is created now. |
|
great work @oqq Will review in detail next week |
|
@oqq please note prooph/event-store#258 - some interface additions are done. |
|
Changes Unknown when pulling d7f9189 on develop into ** on master**. |
|
Changes Unknown when pulling 85c63b0 on develop into ** on master**. |
|
Changes Unknown when pulling 85c63b0 on develop into ** on master**. |
