Turning the problem upside down, I wonder if IndexShardingStrategy should be deprecated and have SharIdentifierProvider as the API a user would implement. It makes for simpler things. What would we lose feature wise?
Emmanuel On Fri 2013-09-20 17:30, Hardy Ferentschik wrote: > Hi, > > here comes a follow up on my previous email regarding configuration of > dynamic sharding. > > Now I would like to get some feedback on ShardIdentifierProvider. This > interface was added for dynamic sharding on > top of DynamicShardingStrategy. Gunnar and I had a discussion around it today > [1] and we came to the conclusion that > this interface is actually not needed and just adds confusion in the API. > > Really ShardIdentifierProvider is a IndexShardingStrategy in disguise. Add > 'forAddtion' and 'forDeletion' to the two 'getShardIdentifier' methods > and you have (almost) an IndexShardingStrategy. The problem seems to be, that > in order to implement dynamic sharding the > IndexManagerHolder and EntityIndexBinding are needed. > IndexShardingStrategy#initialize does not provide access to these objects > which maybe > led to the current design. > > In DynamicShardingStrategy this is taken care of by passing > IndexManagerHolder and EntityIndexBinding as part of the constructor > arguments. > Otherwise DynamicShardingStrategy is just a very thin wrapper delegating to > the ShardIdentifierProvider. > > If the current IndexShardingStrategy#initialize method would get passed the > required information for dynamic sharding, there would be no need > for an additional interface like ShardIdentifierProvider. Dynamic sharding > could be implemented with the existing extension points and implementations > would fit better into the current pattern of providing custom > implementations. > > What we could do is to make DynamicShardingStrategy an interface extending > IndexShardingStrategy and adding a second initialise contract > of sorts. This would keep backwards compatibility, but also allow for dynamic > sharding by users implementing DynamicShardingStrategy. > > At the downside the user would have to write a bit more code, but I think > that's acceptable given the more consistent approach towards sharding. > > Thoughts? > > --Hardy > > P.S. In case you guys think that we really should hold on to > ShardIdentifierProvider, I would at least suggest to either rename > the two 'getShardIdentifier' methods adding 'forAddition' and 'forDeletion' > or even collapse the two into a single method (not sure > whether this is easily possible) > > > [1] > http://transcripts.jboss.org/channel/irc.freenode.org/%23hibernate-dev/2013/%23hibernate-dev.2013-09-20.log.html > _______________________________________________ > hibernate-dev mailing list > hibernate-dev@lists.jboss.org > https://lists.jboss.org/mailman/listinfo/hibernate-dev _______________________________________________ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev