On Wed 2013-05-29 17:39, Sanne Grinovero wrote: > We're starting a series of refactorings in Hibernate Search to improve > how we handle the entity mapping to the index; to summarize goals: > > 1# Expose the Metadata as API > > We need to expose it because: > a - OGM needs to be able to read this metadata to produce appropriate queries > b - Advanced users have expressed the need to for things like listing > all indexed entities to integrate external tools, code generation, > etc.. > c - All users (advanced and not) have interest in -at least- logging > the field structure to help creating Queries; today people need a > debugger or Luke. > > Personally I think we end up needing this just as an SPI: that might > be good for cases {a,b}, and I have an alternative proposal for {c} > described below. > However we expose it, I think we agree this should be a read-only > structure built as a second phase after the model is consumed from > (annotations / programmatic API / jandex / auto-generated by OGM). It > would also be good to keep it "minimal" in terms of memory cost, so to > either: > - drop references to the source structure > - not holding on it at all, building the Metadata on demand (!) > (Assuming we can build it from a more obscure internal representation > I'll describe next). To me since we have an internal representation for DocumentBuilder to do its runtime job, the user visible model could end up being a flyweight object in front of it exposing thigns the way we want. Whether you create this flyweight structure each time is another question. > > Whatever the final implementation will actually do to store this > metadata, for now the priority is to define the contract for the sake > of OGM so I'm not too concerned on the two phase buildup and how > references are handled internally - but let's discuss the options > already. > > 2# Better fit Lucene 4 / High performance > > There are some small performance oriented optimizations that we could > already do with Lucene 3, but where unlikely to be worth the effort; > for example reusing Field instances and pre-intern all field names. > These considerations however are practically mandatory with Lucene 4, as: > - the cost of *not* doing as Lucene wants is higher (runtime field > creation is more expensive now) > - the performance benefit of following the Lucene expectations are > significantly higher (takes advantage of several new features) > - code is much more complex if we don't do it I am not sure how that requirement fits witht he solution you describe later. > > 3# MutableSearchFactory > > Let's not forget we also have a MutableSearchFactory to maintain: new > entities could be added at any time so if we drop the original > metadata we need to be able to build a new (read-only) one from the > current state. > > 4# Finally some cleanups in AbstractDocumentBuilder > > This class served us well, but has grown too much over time. > > Things we wanted but where too hard to do so far: > - Separate annotation reading from Document building. Separate > validity checks too. > - It checks for JPA @Id using reflection as it might not be available > -> pluggable? We know only one use case for this pluggable mechanism, do we really need it? > - LuceneOptionsImpl are built at runtime each time we need one -> > reuse them, coupling them to their field > Do you think it would yield performance improvement? It does not look like an expensive object to create compared to keeping a reference around. What's your reasoning? > DocumentBuilderIndexedEntity specific: > - A ConversionContext tracks progress on each field by push/pop a > navigation stack to eventually thrown an exception with the correct > description. If instead we used a recursive function, there would be > no need to track anything. I'm not entirely following how the recursive method could help you to track the context. Upon failure, You would catch inner call exceptions and add your context information each time and rethrow? > - We had issues with "forgetting" to initialize a collection before > trying to index it (HSEARCH-1245, HSEARCH-1240, ..) > - We need a reliable way to track which field names are created, and > from which bridge they are originating (including custom bridges: > HSEARCH-904) > - If we could know in advance which properties of the entities need > to be initialized for a complete Document to be created we could > generate more efficient queries at entity initialization time, or at > MassIndexing select time. I think users really would expect such a > clever integration with ORM (HSEARCH-1235) > > > == Solution ? == > > Now let's assume that we can build this as a recursive structure which > accepts a generic visitor. > One could "visit" the structure with a static collector to: > - discover which fields are written - and at the same time collect > information about specific options used on them > -> query validation > -> logging the mapping > -> connect to some tooling > - split the needed properties graph into optimised loading SQL or > auto-generated fetch profiles; ideally taking into account 2nd level > cache options from ORM (which means this visitor resides in the > hibernate-search-orm module, not engine! so note the dependency > inversion). > - visit it with a non-static collector to initialize all needed > properties of an input Entity > - visit it to build a Document of an initialized input Entity Does that mean the entity graph (data) is really traversed twice, once to init the boundaries and a second time to build the document? > - visit it to build something which gets feeded into a non-Lucene > output !! (ElasticSearch or Solr client value objects: HSEARCH-1188) This one is a really interesting idea. > .. define the Analyzer mapping, generate the dynamic boosting > values, etc.. each one could be a separate, clean, concern. > > This would also make it easier to implement a whole crop of feature > requests we have about improving the @IndexedEmbedded(includePaths) > feature, and the ones I like most: > > # easy tool integration for inspection > # better testability of how we create this metadata > # could make a "visualizing" visitor to actually show how a test > entity is transformed and make it easier to understand why it's > matching a query (or not). > > Quite related, what does everybody think of this : > https://hibernate.atlassian.net/browse/HSEARCH-438 Support runtime > polymorphism on associations (instead of defining the indexed > properties based on the returned type) > ? > Personally I think the we should support that, but it's a significant > change. I'm bringing that up again as I suspect it would affect the > design of the changes proposed above. > > > This might sound a big change; in fact I agree it's a significant > style change but it is rewriting what is defined today in just 3 > classes; no doubt we'll get more than a dozen ouf of it, but I think > it would be better to handle in the long run, more flexible and > potentially more efficient too. What's your argument for efficiency? > Do we all agree on this? In practical terms we'd also need to define > how far Hardy wants to go with this, if he wants to deal only with the > Metadata API/SPI aspect and then I could apply the rest, or if he > wants to try doing it all in one go. I don't think we can start > working in parallel on this ;-) I have always been a skeptic of the visitor pattern. I know you have been drinking it over and over for the parser but to me it's a pattern that: - makes the big picture harder to grasp - extremely difficult to debug if some generic behavior is different that what you expect - tend to create visitors with state (machines) because contextual information ends up being needed anyways In our case, there might be correlation between booting and the specific structure, or analyzer and bridges (making that up). AFAIU from the visitor pattern, you need to merge those correlated works in a single visitor or store intermediate state somewhere in your "output" structure. I like the idea of a visitor to make non Lucene backend easier but the fact that I don't grok code using the visitor pattern is of concern to me. I can be convinced I imagine but I'll need arguments.

_______________________________________________ hibernate-dev mailing list hibernate-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev

On 30 Jan 2013, at 2:00 PM, Gunnar Morling <gunnar(a)hibernate.org&gt; wrote: > I'm wondering how many node types there will be and how deep the hierarchy > actually is. If it is basically only something like BeanDescriptor and > PropertyDescriptor, is it really worth a visitor pattern? Right, my thinking as well. > On a more general note, would it make sense to have a joint meta model for > the different projects such as OGM and Search? No idea what model OGM needs, but I have a hard time seeing project specific meta data being shared. The information it provides it per definition very specific to the given project.

This might help avoiding to write and maintain redundant code (e.g. merging > meta data from different sources) and also might be beneficial for the sake > of memory. Such a common model might have generic node types, to which the > different clients could attach specific payload. That sounds a little over engineered to me. Maybe if I would see some prototype code. But I am skeptical about this. > Note that I'm not sure whether that's a sound idea, just throwing it out :) Sure. np --Hardy

>> Things we wanted but where too hard to do so far: >> - Separate annotation reading from Document building. Separate >> validity checks too. >> - It checks for JPA @Id using reflection as it might not be available >> -> pluggable? > > We know only one use case for this pluggable mechanism, do we really > need it? Which is?

I'm sorry I now realize I made it hard to understand as I didn't actually explain how I mean this to be recursive. First, note that another frequently requested feature is to be able to add some fields to a Document *in addition* to what we would normally do. Today this flexibility is granted by defining a class level bridge, but by doing so this disables processing of @Field annotations, so people can't decorate but have to hardcode the full transformation in their bridge.

To solve that - and the design mentioned above - I was thinking of compositing bridges recursively to match the metadata.

To express that in pseudo-functional code, an entity: @ClassBridge(impl=CustomAnimals.class) @Indexed class Animal { @Id long id; @IndexedEmbedded Color skinColor; @Field name; } would generate a reusable transformation function which gets associated to the Animal.class class ObjectToDocument { Document transform(Entity e); } [not exactly like that, bear with me a moment]

I guess by now you start seeing the problem of defining the exact signature of such composite transformation blocks: I mentioned Visitor in my first email as I think it could help, but it doesn't have to be strictly a Visitor.

The problem is that we will want to navigate the internal metadata for different purposes, as I had outlined in the next paragraph; generally a Visitor allows to decouple the metadata graph from the purpose, while also preserving a good level of typesafety and performance: let's not forget this is one of the hottest areas of the Search codebase (CPU wise), and at the same time the place where we trigger the more important optimisations, like opportunities to skip network operations or disk IO.

>> - We need a reliable way to track which field names are created, and >> from which bridge they are originating (including custom bridges: >> HSEARCH-904)

>> - If we could know in advance which properties of the entities need >> to be initialized for a complete Document to be created we could >> generate more efficient queries at entity initialization time, or at >> MassIndexing select time. I think users really would expect such a >> clever integration with ORM (HSEARCH-1235)

Seems in the list above I forgot my favorite one: dump the metadata as simple text on bootup; this should greatly simplify query writing, and doesn't need to open the index with Luke to figure out the field names / options.

Gee, that's an email ;-) Before getting too much into it I think it would be useful to talk about what I am actually doing. I am trying to expose a meta data API for Search which allows users to determine which entities are indexed and which fields are available for each entity. I am trying to do a similar approach to Bean Validation where all metadata is exposed via descriptors. The entry point into the API is the SearchFactory. I am basically thinking about something like this (feedback welcome): /** * Top level descriptor of the metadata API. Giving access to the indexing information for a single entity. * * @author Hardy Ferentschik */ public interface IndexedEntityDescriptor {

/** * @return Returns {@code true} if the entity for this descriptor is indexed, {@code false} otherwise */ boolean isIndexed();

/** * @return Returns the class boost value, 1 being the default. */ float getClassBoost(); /** * @return Returns the names of the indexes instances of the entity are indexed into. Generally this will * be just one index, however, when sharding is applied multiple indexes per entity can be used. */ Set<String> getIndexNames();

/** * @return Returns a set of {@code FieldDescriptor}s for the indexed fields of the entity. */ // TODO does this include the id field descriptor or should that be a separate descriptor?

// TODO should OBJECT_CLASS be considered? Set<FieldDescriptor> getIndexedFields();

} /** * Metadata related to a single indexed field. * * @author Hardy Ferentschik */ public interface FieldDescriptor { /** * Returns the Lucene {@code Document} field name for this indexed property. * * @return Returns the field name for this index property */ String getFieldName();

/** * @return Returns an {@code Analyze} enum instance defining the type of analyzing applied to * this field. */ Analyze getAnalyzeType(); /** * @return Returns a {@code Store} enum instance defining whether the index value is stored in the index itself. */ Store getStoreType(); /** * @return Returns a {@code TermVector} enum instance defining whether and how term vectors are stored for this * field */ TermVector getTermVectorType(); /** * @return Returns a {@code Norms} enum instance defining whether and how norms are stored for this * field */ Norms getNormType(); /** * @return Returns the boost value for this field. 1 being the default value. */ float getBoost(); /** * @return Returns the string used to index {@code null} values. {@code null} in case null values are not indexed. */ String nullIndexedAs(); /** * @return Returns the field bridge instance used to convert the property value into a string based field value */ FieldBridge getFieldBridge(); /** * @return Returns the analyzer used for this field, {@code null} if the field is not analyzed */ Analyzer getAnalyzer(); } On top of this I am planning to add (addressing HSEARCH-903): public interface FieldNameReportingBridge { Iterable<String> getGeneratedFieldNames(String baseFieldName); }

The latter I need to allow custom bridges to report which fields they add. Most of the information I need to implement all this is in AbstractDocumentBuilder.PropertiesMetadata. The plan so far was to extract the information from there and while working in this making PropertiesMetadata a proper object (instead of the parallel arrays thingy).

Maybe some other minor refactorings along the way. I was not going to touch the processing of annotations for now. As discussed that, there we would need yet another level of abstraction (similar to EntitySource in ORM or BeanConfiguration in HV). Something which can be populated by either annotation processing (be it Jandex or reflection) or by the the programmatic API. Different story though. For what I can tell I don't need a Visitor pattern for what I have planned to do so far. If you think I am on the wrong track let me know and let me see the light. One thing I was wondering about after your email, however, was whether the API needs to provide information which field/getter/class is responsible for creating a given Lucene Document Field. Do we have a use case for that? On 29 Jan 2013, at 6:39 PM, Sanne Grinovero <sanne(a)hibernate.org&gt; wrote: > We're starting a series of refactorings in Hibernate Search to improve > how we handle the entity mapping to the index; to summarize goals: > > 1# Expose the Metadata as API > > We need to expose it because: > a - OGM needs to be able to read this metadata to produce appropriate queries @gunnar, does the API above address your needs?

> Personally I think we end up needing this just as an SPI: that might > be good for cases {a,b}, and I have an alternative proposal for {c} > described below. -1 why SPI. I think this is a very general purpose API useful for any users. For example, you could image to build auto field suggesting query field which makes suggestions on which fields you can search on (a little like the Jira queries). In this case you could get the available fields via this API. Just to mention one use case. > However we expose it, I think we agree this should be a read-only > structure built as a second phase after the model is consumed from > (annotations / programmatic API / jandex / auto-generated by OGM). +1 > It > would also be good to keep it "minimal" in terms of memory cost, so to > either: > - drop references to the source structure > - not holding on it at all, building the Metadata on demand (!) > (Assuming we can build it from a more obscure internal representation > I'll describe next). Given that I am going to build it from required runtime information it could for sure be lazily loaded. However, right now I think I will just go for the straight forward approach. > 3# MutableSearchFactory > > Let's not forget we also have a MutableSearchFactory to maintain: new > entities could be added at any time so if we drop the original > metadata we need to be able to build a new (read-only) one from the > current state. Good point > Things we wanted but where too hard to do so far: > - Separate annotation reading from Document building. Separate > validity checks too. +1 See above. I want to address this in another issue. We will need another intermediate model for that. With this in place we can remove commons-annotaiotns and easily consume a Jandex index as well > - It checks for JPA @Id using reflection as it might not be available > -> pluggable? Not sure what you mean here. That's just a very specific JPA/ORM based use case. > - LuceneOptionsImpl are built at runtime each time we need one -> > reuse them, coupling them to their field +1 > - We need a reliable way to track which field names are created, and > from which bridge they are originating (including custom bridges: > HSEARCH-904) See above and the FieldNameReportingBridge I am suggesting > == Solution ? == > > Now let's assume that we can build this as a recursive structure which > accepts a generic visitor. … that's where you loose me. I think I am a little like Emmanuel here. Where does a Visitor pattern help here? --Hardy _______________________________________________ hibernate-dev mailing list hibernate-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev

> Set<String> getIndexNames(); > > Would something like Set<IndexDescriptor> getIndexes() make sense? That's an interesting thought. In fact I was wondering whether more index specific information is needed. On the other hand, I was thinking thinking that as long as you have the index name you can always get more information via accessing the IndexManager directly - SearchFactoryImplementor.getAllIndexesManager.getIndexManager(String indexName)

> Forgot to mention that I really dislike getAllIndexesManager(). I would love to rename it to getIndexManagers() or getIndexManagerHolder(). > Might be something for Search 5!? I'd say deprecate the existing one and create getIndexManagerHolder() right now. No need to wait for 5.