[hibernate-dev] [HSEARCH] Usage of ShardIdentifierProvider

[Sanne Grinovero]

On 24 April 2016 at 19:00, Gunnar Morling <gunnar at hibernate.org> wrote:
>
>
> 2016-04-22 12:55 GMT+02:00 Sanne Grinovero <sanne at hibernate.org>:
>>
>> On 21 April 2016 at 18:35, Gunnar Morling <gunnar at hibernate.org> wrote:
>> > Hi,
>> >
>> > As IndexShardingStrategy is deprecated, I thought I'd use
>> > ShardIdentifierProvider and friends in new code I write. It's not clear
>> > to
>> > me though, how it's meant to be used.
>> >
>> > Some questions:
>> >
>> > * Is it correct that EntityIndexBinding#getShardIdentifierProvider()
>> > returns null is sharding is not used for this entity?
>>
>> Yes, as the javadoc suggests.
>
>
> Ok, thanks; The current comment is rather vague, it should more clearly
> state when the return value will be null.
>
>> I don't like to return "null"s so if
>>
>> > I suppose in that
>> > case I simply can use EntityIndexBinding.getIndexManagers()[0]?
>>
>> I'm not understanding how this related to the previous question. What
>> are you up to?
>
>
> E.g. look at DefaultBatchBackend. Working on improving batch performance for
> the ES backend, I came across the deprecation of IndexShardingStrategy and
> wondered what it'd take to get rid of that.
>
>> N.B. the intent of the sharding strategies is not to give end users a
>> reference to the IndexManagers.
>> So the `ShardIdentifierProvider` contract lets people identify the
>> target indexes by naming them.
>
>
> The deprecation comment on IndexShardingStrategy says "Use
> ShardIdentifierProvider instead". Are you saying that this cannot be done
> actually? That's what I tried: "use SIP" and from the shard name obtain an
> index manager. But your next comment below makes me wonder whether that's
> actually desirable.
>>
>>
>> > * What's the envisioned way to get the IM for a given shard once I know
>> > the
>> > shard id? I found IndexManagerHolder.getOrCreateIndexManager(), but this
>> > expects a *DynamicSharding*EntityIndexBinding, so how would it work for
>> > non-dynamic sharding?
>>
>> The envisioned way is that the user doesn't get a reference to the
>> IndexManager,
>> what he needs is to identify the indexes - not act on them.
>
>
> See above. I'm confused by the deprecation comment on IndexShardingStrategy.
> Right now I don't see how ShardIdentifierProvider can be used instead.
>>
>>
>> >
>> > It's tough to see how the pieces are meant to fit together, now that
>> > IndexShardingStrategy and ShardIdentifierProvider are there. I hope we
>> > can
>> > get rid of the former soon, simplifying the code a bit?
>>
>> Yes. It has been deprecated for long enough, we could give a shot of
>> removing the old one if this helps something you're doing?
>>
>> I wouldn't remove it yet if it's not in the way of our 5.6 requirements.
>>
>> BTW the contract "IndexShardingStrategy" is deprecated so that users
>> of sharding move to the new API, but that interface still is the
>> foundation of the implementation even for dynamic sharding.. it's not
>> being removed from internals.
>
>
> Aaah, so are you saying that IndexShardingStrategy is only deprecated for
> use by Hibernate Search users, but internally it's not meant to go away?
> I.e. it'll just be removed from the public API but will continue to exist in
> some "impl" package?
>
> If so, some more explicit comment about the nature of this deprecation would
> help :)

I don't think this was clear when we decided that dynamic sharding
would need to hide references to IndexManager instances from the
user's sharding strategy code; discussions back then were focusing on
user experience and API.

I just came to this conclusion when looking at how we use the contract
internally, from a point of view of trying to guess what your
questions were aiming at :)

So to conclude:
 - yes it's deprecated for end users: you *may* move/remove it now if
that helps with your goals
 - it's not necessarily going away as long as it's a useful
abstraction for our own internal purposes

Personally I'd not fully remove it: an IndexShardingStrategy interface
is a fitting name for the concept, I'd avoid in future to have this
imply anything about its details such as dynamic vs static sharding
though. It is currently bound to the "static sharding" concept from
historical reasons.

--Sanne

>
>>
>> The new contract just enables us to add new shards dynamically, but
>> you can see this also as a removal of the previous limitation. The
>> high level concept can keep the generic name "index sharding strategy"
>> as it's still fitting.
>>
>> You can see this as a plan to not have users to choose between two
>> ways to express the same thing.
>>
>> Thanks,
>> Sanne
>>
>> >
>> > Thanks,
>> >
>> > --Gunnar
>> > _______________________________________________
>> > hibernate-dev mailing list
>> > hibernate-dev at lists.jboss.org
>> > https://lists.jboss.org/mailman/listinfo/hibernate-dev
>
>