Redis SagaStore implementation bug: it creates multiple entries in redis for the same saga-id

[rakamoviz]

I found this while I was trying the timeouted-saga feature; it keeps returning the first instance of the saga (that has no timeout, a.k.a.: keyed in redis as {prefix}:{createdtimestamp}:Infinity:{thesaga-id} . Not I was expecting; I expected there would be only one instance of saga store for a given saga-id.

Then I read mongo's saga store for comparison; and this one meet my expectation. I then went on trying running my apps using mongo saga store instead of redis, and the problem I was having when using redis saga store is gone.

Ref.:

Redis Saga store: https://github.com/adrai/node-cqrs-saga/blob/9770440df0e50b5a3897607a07e0252315b25edf/lib/store/databases/redis.js#L183

Here you can see when saga is saved, the key is constructed based on current timestamp, and saga timeout. This is the root of the problems; as it register another record, for the same saga.

When you do get using a given saga id, it matches with the first found (and the match is performed only by the last token, the saga id itself, excluding the timestamp & saga timeout): https://github.com/adrai/node-cqrs-saga/blob/9770440df0e50b5a3897607a07e0252315b25edf/lib/store/databases/redis.js#L325

The problem I was having (because of that) was: during getTimeoutedSagas, it keeps giving me (only) the first record for that saga-id, which in this the one with "Infinity" timeout, which is already wrong: https://github.com/adrai/node-cqrs-saga/blob/9770440df0e50b5a3897607a07e0252315b25edf/lib/store/databases/redis.js#L453

All the problem does not exist in mongodb saga store, because it's using the given sage id as index in the database. It guarantes: one sagaid, one record: https://github.com/adrai/node-cqrs-saga/blob/9770440df0e50b5a3897607a07e0252315b25edf/lib/store/databases/mongodb.js#L196 ... and that, provides the correct behavior.

[nanov]

I'm not a redis expert, would you mind purposing a fix ( PR maybe ) ?

I can implement some tests that ensure right behaviour.

[rakamoviz]

Hi,

Since we are using dynamodb for our document database, we decided to write the dynamodb saga store first.

I hope later I can find time to look into the redis saga store.

So far: https://github.com/rakamoviz/node-cqrs-saga/blob/master/lib/store/databases/dynamodb.js

Will test it now. If it works I'd be happy to merge this file to this repo.

[rakamoviz]

@nanov : I have tested with a couple of scenarios (simple one with correct order of events coming into the sage, ..., also with incorrect order of events...., and lastly the case with timeouted sagas). They all work as expected.

You can take the code for dynamodb saga-store from here: https://github.com/rakamoviz/node-cqrs-saga/blob/4e592e750d0ab11dec135def29a51a05bab41a38/lib/store/databases/dynamodb.js

Example config:

sagaStore: {
  type: 'dynamodb',
  sagaTableName: 'sagas',
  primaryReadCapacityUnits: 10,
  primaryWriteCapacityUnits: 10,
  commitStampReadCapacityUnits: 10,
  commitStampWriteCapacityUnits: 10,
  timeoutAtReadCapacityUnits: 10,
  timeoutAtWriteCapacityUnits: 10,
},   

And need to have env variables like this when running the app: AWS_DYNAMODB_ENDPOINT=http://localhost:8000 AWS_REGION=us-north-0 (and of course the AWS key and secret)

[nanov]

Wow, good job!

I will take a deeper look during the weekend and we could merge it!