shard-store doesn't support creating entities with specific id$

[paolochiodi]

On the save method specs outline three different behaviours:

• if id is provided, an existing entity should be updated
• if id isn't provided, a new entity should be created with an auto-generated id
• if id isn't provided but id$ it, a new entity should be created with that specific id

Currently shard-store seems to "try to support" (see here https://github.com/senecajs/seneca-shard-store/blob/master/index.js#L49) but the underlying "sharder" doesn't support it and this will result undefined https://github.com/senecajs/seneca-shard-store/blob/master/index.js#L66

We can solve this by:

• adding support to "sharder" for ids generated outside it (keeps compatibility with older versions)
• remove sharder and use a custom solution that support auto-generated ids (breaks compatibility with older versions because it will distribute entities among shards differently)
• do not support id$ on the shard store (also breaks current version of seneca-store-tests that use id$ to properly setup the store before running tests)

[paolochiodi]

@geek how do you want to proceed on this?

[rjrodger]

we need to update the sharder to support the spec - sharding should be transparent to users :)

[paolochiodi]

What are the secondary goals? I can think of two, but they add up quite a lot of complexity

• backward database compatibility with current implementation
• easy scalability (ability to add shards later)

[mcollina]

@rjrodger the whole idea of this store is that ids encode the shard in which they are being created. This is the logic for which this module was built. So, no externally supported ids.

In order to support external ids, we'd need to look things up on all shards, there is little room around that. The other option (for given ids), we'd need to hash the ids and map them using the hash. But that is not as easy as it sounds, as how do we support adding new shards?

Currently sharder supports up to 256 shards (1 byte) without resharding/regenerating all ids.

[paolochiodi]

@mcollina you perfectly said what I was thinking. Supporting full spec will only come with other drawbacks

[rjrodger]

ah yes good points guys - sharder of course requires control of the ids - so cannot support full spec - but that's ok - it's the tradeoff for sharding

I guess we need a way to parameterise the standard store tests to handle this case?

[mirceaalexandru]

Yes, we might think on that, is a very good idea to parameterise the standard store. Now I am changing riak store and there I have some functions from store that cannot be implemented - like delete all$:true, list and some more so this will be very good for other stores also.

On Mon, Nov 16, 2015 at 7:23 PM, Richard Rodger notifications@github.com wrote:

ah yes good points guys - sharder of course requires control of the ids - so cannot support full spec - but that's ok - it's the tradeoff for sharding

I guess we need a way to parameterise the standard store tests to handle this case?

— Reply to this email directly or view it on GitHub https://github.com/senecajs/seneca-shard-store/issues/11#issuecomment-157107356 .

malex

[paolochiodi]

I wouldn't parameterise too much: the point of having a standard test suite is to make the stores uniform.

Going a bit OT here, but I think we should only have stores that support all the specs (or a specific subset of it). Any database/persistence layer that makes difficult or impossibile to do that signals that storing entities may not be the best use case for such a database and it should be used directly through the driver (or a different abstraction). I'm thinking of Riak, Cassandra, ldap...

[mcollina]

The whole point of this is to use the standard store with sharding. Apart from the ids, this should be compliant to the spec.

I'm ok for dropping this one, but I think it's used...

[paolochiodi]

@mcollina I think we can make an exception for the shard store (given his use case), but I think we shouldn't make exceptions for every store

[mirceaalexandru]

@paolochiodi I think that we should have stores for as much DB engines as possible.

On the other hand for those that do not match exactly the standard store tests because the DB do not allow some functions we should specify this in the Readme. This will make difficult to switch between DB engines or use something like shard, but we should allow them to use Riak store (an example) if they need that.

Imagine that you need to use Riak in your project, then you will need to write that plugin by yourself or go directlyto the driver when you can have the Seneca Store interface for what you can use on the DB.