<html>
<head>
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<div class="moz-cite-prefix">Hi Sanne, <br>
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.<br>
<br>
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 <i>Object
QueryInterceptor.extractValue(Object wrappedValue)</i> 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.<br>
<br>
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?<br>
<br>
Thanks,<br>
Adrian<br>
<br>
On 09/04/2013 02:22 PM, Sanne Grinovero wrote:<br>
</div>
<blockquote
cite="mid:CAFm4XO1yBgoHA0rj=A-ky5LxrVN2Cd23UvzgGYAf6tPPNcNUrg@mail.gmail.com"
type="cite">
<pre wrap="">On 4 September 2013 12:01, Adrian Nistor <a class="moz-txt-link-rfc2396E" href="mailto:anistor@redhat.com"><anistor@redhat.com></a> wrote:
</pre>
<blockquote type="cite">
<pre wrap="">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?
</pre>
</blockquote>
<pre wrap="">
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:
- <a class="moz-txt-link-freetext" href="https://issues.jboss.org/browse/ISPN-951">https://issues.jboss.org/browse/ISPN-951</a>
- <a class="moz-txt-link-freetext" href="https://issues.jboss.org/browse/ISPN-2610">https://issues.jboss.org/browse/ISPN-2610</a>
</pre>
<blockquote type="cite">
<pre wrap="">
Thanks!
[1] <a class="moz-txt-link-freetext" href="https://github.com/infinispan/infinispan/pull/2022">https://github.com/infinispan/infinispan/pull/2022</a>
_______________________________________________
infinispan-dev mailing list
<a class="moz-txt-link-abbreviated" href="mailto:infinispan-dev@lists.jboss.org">infinispan-dev@lists.jboss.org</a>
<a class="moz-txt-link-freetext" href="https://lists.jboss.org/mailman/listinfo/infinispan-dev">https://lists.jboss.org/mailman/listinfo/infinispan-dev</a>
</pre>
</blockquote>
<pre wrap="">_______________________________________________
infinispan-dev mailing list
<a class="moz-txt-link-abbreviated" href="mailto:infinispan-dev@lists.jboss.org">infinispan-dev@lists.jboss.org</a>
<a class="moz-txt-link-freetext" href="https://lists.jboss.org/mailman/listinfo/infinispan-dev">https://lists.jboss.org/mailman/listinfo/infinispan-dev</a>
</pre>
</blockquote>
<br>
</body>
</html>