10 p.m.
Hi Sanne,
I'm not out of cool tasks yet, there's still some work needed for remote
query before even reaching the point to start polishing it.
Re QueryInterceptor and RemoteValueWrapperInterceptor, I've also
considered adding the wrapping functionality directly to
QueryInterceptor, well actually to a remote-query specific subclass,
which could also make possible those entity class related
optimizations/simplifications for the remote case you mentioned. If you
look at method /Object QueryInterceptor.extractValue(Object
wrappedValue)/ you'll know what I mean. The QueryInterceptor already has
some particular handling for MarshalledValue which could be expanded to
also provide special treatment for those byte[] of the remote caches so
RemoteValueWrapperInterceptor would disappear.
The only problem that stopped me form merging these two interceptors is
the dependency hell it generates: query module needs to know a bit about
remote query or alternatively remote query should be able to replace the
interceptor installed by query with a specialized one (not sure if it
can be done, but replacing seems pretty silly anyway). Any ideas on how
to specialize an interceptor installed by a different module?
Thanks,
Adrian
On 09/04/2013 02:22 PM, Sanne Grinovero wrote:
On 4 September 2013 12:01, Adrian Nistor <anistor(a)redhat.com>
wrote:
> Hi devs,
>
> Currently remote caches have byte[] values. With the introduction of
> remote query these byte arrays are no longer opaque for search-enabled
> caches, they are Protobuf encoded and their schema is known and we have
> to somehow help hibernate-search extract the indexable data and index
> it. As a side note, the indexing for embedded mode is done by
> QueryInterceptor which relies on hibernate-search annotated cache
> values. This definitely does not work directly for byte[].
>
> One option is to write another interceptor similar to QueryInterceptor
> but capable of making sense of the byte[], but that would duplicate
> functionality (and maintenance) and would have to use Lucene directly
> because in order to use hibernate-search you would need annotations on
> your entities. And if we go this route we might need to also duplicate
> MassIndexer.
>
> A second more preferable option is to wrap the byte[] values into a
> hsearch FieldBridge or ClassBridge annotated entity so we can continue
> to use existing QueryInterceptor. This was implemented in the PR [1],
> the wrapping entity being
> org.infinispan.query.remote.indexing.ProtobufValueWrapper and the place
> where the wrapping happens is a new interceptor,
> org.infinispan.query.remote.indexing.RemoteValueWrapperInterceptor.
> RemoteValueWrapperInterceptor uses a mechanism similar to
> TypeConverterInterceptor (of compat mode), so a common base class was
> extracted for them.
>
> The wrapping approach has received favourable review so far. But I'm
> asking myself (and you) maybe there's an even simpler solution that I've
> overlooked? Or maybe we can do the wrapping somehow without introducing
> a new interceptor?
Current solution seems quite simple and maintainable to me, +1 to keep
it. I'd also advise to stay away from low-level Lucene APIs: they
change significantly and you don't want to rewrite it all for each
update, unless you read the lucene-dev mailing list at breakfast
everyday.
Still, in this first round we favored quick development and there is
room for improvement in several areas; for example, since you
mentioned specifically the QueryInterceptor, I would prefer to see a
specialized edition of it (maybe sharing some code) to skip all the
overhead it has in checking if the indexed type is a well-known
indexed class, this logic is quite tricky and needs some reflection
while it's all unnecessary burden if you are able to register the
one-and-only indexed valueholder.
I'm confident that you'll find more simplifications on this road, just
pointing you to a potential starting point.. also if you're running
out of cool tasks I could point you to some open issues, there are
some other feature requests that I would consider quite important, for
example:
- https://issues.jboss.org/browse/ISPN-951
- https://issues.jboss.org/browse/ISPN-2610
> Thanks!
>
> [1] https://github.com/infinispan/infinispan/pull/2022
> _______________________________________________
> infinispan-dev mailing list
> infinispan-dev(a)lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/infinispan-dev
_______________________________________________
infinispan-dev mailing list
infinispan-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev