[hibernate-search] Implement indexing and search #6283

matthias-ronge · 2024-10-29T16:48:00Z

Issue #5760 2c), 2d), 2e) and 2f)

Follow-up pull request to #6218 (immediate diff)

The filters work. Metadata is indexed and searchable. The metadata search syntax works as documented in the wiki.

BartChris · 2024-10-29T18:04:19Z

Great work! Can you elaborate (short high level overview) how the metadata is indexed now in general? (For reference: #4266)

henning-gerhardt · 2024-10-30T07:55:50Z

Kitodo/src/main/java/org/kitodo/production/filters/FilterMenu.java

+        Map<String, String> parameters = FacesContext.getCurrentInstance().getExternalContext()
+                .getRequestParameterMap();
+        if (parameters.containsKey("input") && StringUtils.isBlank(filterInEditMode)) {
+            filterInEditMode = parameters.get("input");


You are taking over any argument from the outside without validating that the given value is correct or at least is in range of the expectations?

Yes, the user can enter any string into the search slot. The input is not “checked”. But of course, if the user has entered nonsense, the search will not find any results.

Thank you for this clarification but I expected something different as I read the method, method content and the used written class variable filterInEditMode. Handling of this class variable filterInEditMode is strange in this class but this is a different story.

matthias-ronge · 2024-10-30T09:24:35Z

Can you elaborate (short high level overview) how the metadata is indexed now in general?

The metadata is stored in a text field that is indexed. The search is later carried out on the text field.

"Pseudo words" are used to search on certain metadata fields, i.e. if the word Berlin is in the TitleDocMain field in the metadata, a pseudo word "titledocmainqberlin" is also indexed. If a user later enters "TitleDocMain:Berlin" in the search field, this is converted into the pseudo word and searched for.

Pseudo words are generated for metadata keys, translated metadata keys according to the rule set, and domains according to the ruleset. The user can therefore also search for "Haupttitel:Berlin" (if the translation in the ruleset is this).

BartChris · 2024-10-30T16:35:15Z

Pseudo words are generated for metadata keys, translated metadata keys according to the rule set, and domains according to the ruleset. The user can therefore also search for "Haupttitel:Berlin" (if the translation in the ruleset is this).

Thanks a lot, this is helpful! Do i understand you correctly that you make the fields searchable by their german and english label as well? So for example "Dokumenttyp:Monograph" as a possible search with the german label?

<key id="docType" use="docType">
           <label>Document Type</label>
           <label lang="de">Dokumenttyp</label>
</key>

And those combined terms (LABEL+VALUE) are all stored in the index? What happens if somebody changes the german label of a metadata key in the ruleset?

BartChris · 2024-10-30T17:00:43Z

Kitodo-DataManagement/src/main/java/org/kitodo/data/database/beans/Keyworder.java

+     * Creates the keywords for searching in correction messages. This is a
+     * double-truncated search, i.e. any substring.
+     * 
+     * @param comments
+     *            the comments of a process
+     * @return keywords
+     */
+    private static final Set<String> initCommentKeywords(List<Comment> comments) {
+        Set<String> tokens = new HashSet<>();
+        for (Comment comment : comments) {
+            String message = comment.getMessage();
+            if (StringUtils.isNotBlank(message)) {
+                for (String splitWord : splitValues(message)) {
+                    String word = normalize(splitWord);
+                    for (int i = 0; i < word.length(); i++) {
+                        for (int j = i + 1; j <= word.length(); j++) {
+                            tokens.add(word.substring(i, j));
+                        }
+                    }
+                }
+            }
+        }
+        return tokens;
+    }


Can you explain the reasoning here? Do i read this correctly and you are indexing every possible substring of all the words in a comment? Is this really necessary? Normal indexing should make it possible to search by word. Is this not enough?

The wiki page requires:

Eine Suche nach Meldung findet alle Vorgänge, die einen Kommentar mit einer Nachricht wie Das ist eine Korrekturmeldung! haben.

To achieve this, we need both-side subword search. I only follow the expected behaviour here. I would prefer a simple-word search, too, but this is what the requirement actually looks like. Maybe we can discuss this, if it is really needed, as it inflates the index. I don’t know a clever way of splitting a German compound-word to its single words, to index just these. (This is possible, but it would be a sophisticated task of computational linguistics, typically involving a dictionary file, too, that I don’t have.)

matthias-ronge · 2024-10-31T09:54:17Z

Do i understand you correctly that you make the fields searchable by their german and english label as well? So for example "Dokumenttyp:Monograph" as a possible search with the german label?

Yes.

What happens if somebody changes the german label of a metadata key in the ruleset?

The search will only find the process with the old label, until the process is saved again, or all processes are re-indexed. Since the index is now only used for filters, re-indexing (or a missing index) won’t “break” the application any more, as it did in the past, it should be possible to run re-indexing in an environment where people work on without breaking something (but it will still make it slow meanwhile!)

BartChris · 2024-10-31T10:32:15Z

What happens if somebody changes the german label of a metadata key in the ruleset?

The search will only find the process with the old label, until the process is saved again, or all processes are re-indexed. Since the index is now only used for filters, re-indexing (or a missing index) won’t “break” the application any more, as it did in the past, it should be possible to run re-indexing in an environment where people work on without breaking something (but it will still make it slow meanwhile!)

I understand. I fear a situation, where correcting a typo in a label would require a reindex of everything, otherwise old data is only found with the misspelled label. I do not want to increase your workload too much, but would it be the option to allow the label-based-filters in the UI, but only index and search under the common key in the index? By e.g. having a mapping function, which maps the labels to the actual key and searches for that.

BartChris · 2024-10-31T11:15:54Z

Since the index is now only used for filters, re-indexing (or a missing index) won’t “break” the application any more, as it did in the past, it should be possible to run re-indexing in an environment where people work on without breaking something (but it will still make it slow meanwhile!)

A more general remark here. I have not really tested anything so far, but it appears to me that almost everything gets indexed. My assumption would be that functionality like displaying list of processes (with all the necessary icons, which require lookups) is faster from the index since we can store data there in denormalized form. It is nice, that we can run Kitodo without a running index for certain functionality but my intuition would be to always use the index if that gives us an edge over database-based retrieval. Maybe i am overlooking something, but it might be worth considering to rely more on the index to have better performance when displaying 100 processes at once, which is very slow in Kitodo 3.7. (which probably uses a mixture of index and database-based retrieval)

This is not necessarily adressed at the current PR, but more a general remark which can be considered.

matthias-ronge · 2024-10-31T13:53:48Z

I fear a situation, where correcting a typo in a label would require a reindex of everything, otherwise old data is only found with the misspelled label.

In my experience, rulesets are rarely changed at all during a running project, except perhaps the addition of a field; and I know librarians are very good at not making typos.

would it be the option to allow the label-based-filters in the UI, but only index and search under the common key in the index? By e.g. having a mapping function, which maps the labels to the actual key and searches for that.

The metadata is also indexed under the key string defined in the rule set, so you can always use the key name for the search.
Yes, of course, that would be possible, but I think it goes against the purpose of using search engines. It would be possible to implement this, but it would significantly slow down search queries because all relevant rulesets would have to be determined each time and checked to see whether a field name needs to be translated. I don't think that would be useful.

Perhaps it would be interesting to discuss this case among users. Such a change can be introduced later.

matthias-ronge · 2024-10-31T14:13:01Z

it appears to me that almost everything gets indexed.

Yes, a lot of indexing is still going on at the moment and I don't think that's necessary. But the task here was not to make more than necessary changes, but to keep the application as most as it was before. But I think that in a separate development, it will be possible to clean this up so that in the end only processes and tasks are indexed, as only these contain metadata. The other objects are never searched, so they don’t do something sensible in the index by now.

My assumption would be that functionality like displaying list of processes […] is faster from the index since we can store data there in denormalized form. […] Maybe i am overlooking something, but it might be worth considering to rely more on the index to have better performance when displaying 100 processes at once, which is very slow in Kitodo 3.7. (which probably uses a mixture of index and database-based retrieval)

No, it's exactly the other way round: In Production 3.7, the display is taken from the index alone, as you describe here as a wish. And it is slow. In this version, this behavior was exactly inverted and now everything except the keyword search is taken from the database. On my developer laptop, the application has become significantly faster as a result.

Don't ask me why it behaves like this. This is another example of how performance is less about well-intentioned programming than about actual measurements. I suspect that both database queries and restoring Java in-memory objects from database rows are so much more sophisticated, having been researched for several decades longer, that it beats out indexing.

henning-gerhardt

It is a code only reading review. I'm not sure if the word splitting for any kind of indexing type and there content is really needed to be done on our side - this should be done on an internal way in ES/OS itself.

Kitodo-DataManagement/src/main/java/org/kitodo/data/database/beans/IndexingKeyworder.java

Kitodo-DataManagement/src/main/java/org/kitodo/data/database/persistence/BaseDAO.java

Kitodo-DataManagement/src/test/resources/log4j2.xml

Kitodo/src/test/java/org/kitodo/production/services/command/CommandServiceTest.java

Kitodo-DataManagement/src/main/java/org/kitodo/data/database/beans/IndexingKeyworder.java

solth · 2024-12-02T11:32:04Z

~~I get an exception when navigating to the processes list (after rebuilding the index via the updated "indexing" page):~~

Edit: I tested on an outdated version of this branch. With the latest commits the exception on the process list page disappears.

Instead, I now get an exception when trying to edit a project, though:

BartChris · 2024-12-12T19:40:14Z

All the filters i enter are always duplicated in the filter menu, which also duplicates the conditions in the database query.

The SQL query generated has two IN-conditions with the same process Ids.

BartChris · 2024-12-12T19:44:30Z

Kitodo/src/main/java/org/kitodo/production/services/data/ProcessService.java

@@ -470,6 +472,7 @@ public List<Process> loadData(int offset, int limit, String sortField, SortOrder
                .collect(Collectors.toList());
        query.restrictToProjects(projectIDs);
        query.defineSorting(SORT_FIELD_MAPPING.get(sortField), sortOrder);
+        query.performIndexSearches();


Just an observation: The strategy here is to retrieve the process IDs for which a specific condition e.g. stepopen:Archivierung is met from the Search Index. Those IDs are used to extend the sql query with a IN-statement containing all the IDs retrieved from the search index. This can lead to REALLY large queries when you have 10.000 processes in your database and 9000 of them are in the state "Strukturierung". If you search for that exact stage the SQL IN clause will contain 9000 process IDs. With more processes in Kitodo the queries can become really large, maybe exhausting MySQL here.

Just an observation: The strategy here is to retrieve the process IDs for which a specific condition e.g. stepopen:Archivierung is met from the Search Index. Those IDs are used to extend the sql query with a IN-statement containing all the IDs retrieved from the search index

This was necessary in the past or with ES / OS as ES / OS can only retrieve JSON based data which are no objects and are not so good accessible. For this reason the ES / OS result was given to Hibernate to retrieve the objects from the database which could contain more information and accessing them was a lot easier. As the used ES / OS API could never result more than 10.000 hits there was a limit to the Hibernate query itself.

This is not needed anymore and should be removed. As after the query is send to Hibernate / Hibernate-Search the results are objects which are easy accessible and hold all information which are needed.

We have a situation, where some information (metadata, but also filter settings like "stepopen") can only be queried using the index. So we need the index to answer the query and therefor we have to query the index somewhere in the process. In my review i will try to concentrate on functionality so that we can focus on performance later, but the general remark i have is this:

We have seen in different places (e.g. #4378) that raw SQL / dedicated index queries often outperforms Hibernate standard queries, which do sometimes hundred of subsequent queries to fetch data which could be fetched in one go. (If Hibernate supports the optimized SQL query)

The process list search is one of the most used features and it should be optimized as much as possible (within the constraints of Hibernate) in my oppinion. When scrolling through the process list i still see A LOT of SQL queries. And queries like here

kitodo-production/Kitodo/src/main/webapp/WEB-INF/templates/includes/processes/processActionsColumn.xhtml

Line 35 in b6fb9ba

styleClass="#{ProcessForm.createProcessAsChildPossible(process) ? '' : 'ui-state-disabled'}"

are executed for every single process leading to the classical n+1 query.

I am wondering wether we can not construct something like a ProcessListDTO Object (or ProcessListObject, this is not about the DTO concept), which contains only the information needed to construct the list. (and the statistics). The objects in the list could already have the necessary properties like hasChildren precomputed, so there is no reason to do additional database queries to calculate them hundred of times.

And since we are already using the index, why not store the necessary properties in the index? We do not have to store all projects associated with a process in the index, but we should store the information which is needed for display like hasChildren. If we have this, we ideally only need one query insted of hundreds to get all the data for the list and have good performance. This will also make issues like #6348 less annoying, although we can do something on usability there, too.

The necessary features of Hibernate Search have already been referenced in other places

https://docs.jboss.org/hibernate/search/7.1/reference/en-US/html_single/#projections
https://docs.jboss.org/hibernate/search/7.1/reference/en-US/html_single/index.html#search-mapping-associated
https://docs.jboss.org/hibernate/search/7.1/reference/en-US/html_single/index.html#mapping-reindexing-derivedfrom

I think it is fantastic that we now reduce indexing time by only index the processes and using the Hibernate abstractions simplifies a lot of logic. But when necessary for performance it would be great if we could use dedicated custom logic to achieve the perfomance in places where it is mostly needed.

The basic design decision was that all data is loaded from the database, and the index is only used when necessary. This is the right decision for a business system with live data and a few users. For a system with thousands of users and less time-critical (for example a travel portal where it doesn't matter if newly added offers only appear after a few minutes), a completely different design would make sense.

Secondly, with this design we wanted to ensure that the application always works, even without an available index. There is no downtime in productivity due to a broken index. The only exception is that you cannot create child processes under existing parent processes, and most search queries will remain empty.

I would like to assure you at this point that I have thought this through very carefully and made this decision to the best of my knowledge and belief.

Technically, only the IDs are queried here, the objects are not loaded. I was very disappointed to find that such functionality is not available in Hibernate Search in the Hibernate Query Language. I had expected that it would be possible to search on indexed fields using a specific HQL keyword or syntax, but that is not the case, I have not found anything like that. So here the IDs of the hits from the index query are passed in HQL; yes, this can possibly cause queries that are several KB in size in some cases, but should not be a problem on a server with GBs of memory. Comparing integers with IDs for the database engine should not be a big deal either.

The many queries when building the process list page are a design flaw. The form classes that underpin the JSP pages should actually be initialized when the page is called up or when a user action is taken. The fact that a getter from the JSP triggers a network action is not the right solution. This is certainly something that we have already inherited from the legacy code and that we should clean up at some point. I have often thought about this. But it should be seen as a separate cause-effect field and not part of this development.

BartChris · 2024-12-17T17:28:22Z

Requirement checklist; as discussed in the Community Board:

Blue search box in the top left jumps to the normal process list and not a special list with less selectable actions

Works in general. Some problems

a) Searching in the top left box does not reset the process list filter, but appends. A search should reset the current search.

Searching for processes

a) Searching for process:11 throws an exception. It seems to happen when i just type process:11 and press enter, without selecting process from the dropdown.

The same thing sometimes happens when searching for 11 in the top left search bar

After that it is not really possible to return to the process list without logging out.

b) The requirements outlined above seem to be working

character string separation at non-word boundaries, where a word consists of letters and numbers
every separated group can be searched using right truncation, only groups with at least three characters need to be indexed AND
every separated group can be searched using left truncation, only groups with at least three characters need to be indexed
but not both in combination

BartChris · 2024-12-17T17:32:15Z

~~I get an exception when navigating to the processes list (after rebuilding the index via the updated "indexing" page):~~

Edit: I tested on an outdated version of this branch. With the latest commits the exception on the process list page disappears.

Instead, I now get an exception when trying to edit a project, though:

I get the same exception when trying to add a new process through the project list.

henning-gerhardt reviewed Oct 30, 2024

View reviewed changes

matthias-ronge force-pushed the 5760_2c branch 2 times, most recently from c052fc4 to 5bee9cb Compare October 30, 2024 11:30

BartChris reviewed Oct 30, 2024

View reviewed changes

matthias-ronge marked this pull request as ready for review October 31, 2024 13:53

solth mentioned this pull request Nov 26, 2024

[hibernate-search] Introduce Hibernate Search framework and implement indexing page #6218

Merged

solth requested review from BartChris and henning-gerhardt December 2, 2024 09:40

henning-gerhardt suggested changes Dec 2, 2024

View reviewed changes

matthias-ronge added 12 commits December 4, 2024 10:39

Implement search

421fb5d

Add search fields to Process and Task beans

9de7742

Fix indexing issues

31f8343

Get search to work, use IndexingService, some clean-up

a8db792

Fix checkstyle (Kitodo - Data Management)

18e3462

Fix checkstyle (Kitodo - Core)

dcfa749

Re-enable the global search slot

f854533

Fix some more things

e5b9bf5

Reduce log output

be9829a

Do not crash tests with orphaned task objects

9115abd

Betterr logging

d364da6

Fix tasks indexing

0f02806

matthias-ronge added 10 commits December 4, 2024 10:39

Remove what isn't absolutely necessary

02c8f28

Prevent loading tasks in wrong Hibernate session for indexing

a3b6016

Remove what isn't absolutely necessary

14ee114

Make indexing much faster avoiding to reload related objects each time

663b222

Adapt to the guidelines of the community board

8a2087d

Fix checkstyle

1176bdf

Adds javadoc

cf151ab

Fix processes list

eb8f044

Do not sort query for any object

01c169e

Simplify creation of debug output

b64a28a

matthias-ronge force-pushed the 5760_2c branch from 96af397 to b64a28a Compare December 4, 2024 09:40

matthias-ronge added 6 commits December 4, 2024 10:41

Remove unused import

acb891c

Add comment to number of elements logged

2f5979e

Undo accidental commit of messages file

e76f6e0

Fix NonUniqueObjectException

b6fb9ba

Fix annotation deleted in error

cd0791a

Simplify and rename class for process keywords

d9cfbf1

BartChris reviewed Dec 12, 2024

View reviewed changes

matthias-ronge added 5 commits February 3, 2025 16:38

Load parent with process

552bee7

Load process with comment

a0f847e

Fix different implementation of hashCode() to equals() in Template

9d48bf0

Load process with task

f6980d1

Load task with comment

c8a653b

matthias-ronge force-pushed the 5760_2c branch from b0bb515 to c8a653b Compare February 3, 2025 16:40

matthias-ronge added 3 commits February 3, 2025 17:50

Load ruleset with process

ecd075b

Fix NPE

c267d59

Load docket with process

ae572b3

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[hibernate-search] Implement indexing and search #6283

[hibernate-search] Implement indexing and search #6283

matthias-ronge commented Oct 29, 2024 •

edited

Loading

BartChris commented Oct 29, 2024 •

edited

Loading

henning-gerhardt Oct 30, 2024

matthias-ronge Oct 30, 2024

henning-gerhardt Oct 30, 2024

matthias-ronge commented Oct 30, 2024

BartChris commented Oct 30, 2024 •

edited

Loading

BartChris Oct 30, 2024 •

edited

Loading

matthias-ronge Oct 31, 2024

matthias-ronge commented Oct 31, 2024

BartChris commented Oct 31, 2024 •

edited

Loading

BartChris commented Oct 31, 2024 •

edited

Loading

matthias-ronge commented Oct 31, 2024

matthias-ronge commented Oct 31, 2024

henning-gerhardt left a comment

solth commented Dec 2, 2024 •

edited

Loading

BartChris commented Dec 12, 2024

BartChris Dec 12, 2024 •

edited

Loading

henning-gerhardt Dec 13, 2024

BartChris Dec 13, 2024 •

edited

Loading

matthias-ronge Feb 4, 2025 •

edited

Loading

BartChris commented Dec 17, 2024 •

edited

Loading

BartChris commented Dec 17, 2024 •

edited

Loading

[hibernate-search] Implement indexing and search #6283

Are you sure you want to change the base?

[hibernate-search] Implement indexing and search #6283

Conversation

matthias-ronge commented Oct 29, 2024 • edited Loading

BartChris commented Oct 29, 2024 • edited Loading

henning-gerhardt Oct 30, 2024

Choose a reason for hiding this comment

matthias-ronge Oct 30, 2024

Choose a reason for hiding this comment

henning-gerhardt Oct 30, 2024

Choose a reason for hiding this comment

matthias-ronge commented Oct 30, 2024

BartChris commented Oct 30, 2024 • edited Loading

BartChris Oct 30, 2024 • edited Loading

Choose a reason for hiding this comment

matthias-ronge Oct 31, 2024

Choose a reason for hiding this comment

matthias-ronge commented Oct 31, 2024

BartChris commented Oct 31, 2024 • edited Loading

BartChris commented Oct 31, 2024 • edited Loading

matthias-ronge commented Oct 31, 2024

matthias-ronge commented Oct 31, 2024

henning-gerhardt left a comment

Choose a reason for hiding this comment

solth commented Dec 2, 2024 • edited Loading

BartChris commented Dec 12, 2024

BartChris Dec 12, 2024 • edited Loading

Choose a reason for hiding this comment

henning-gerhardt Dec 13, 2024

Choose a reason for hiding this comment

BartChris Dec 13, 2024 • edited Loading

Choose a reason for hiding this comment

matthias-ronge Feb 4, 2025 • edited Loading

Choose a reason for hiding this comment

BartChris commented Dec 17, 2024 • edited Loading

BartChris commented Dec 17, 2024 • edited Loading

matthias-ronge commented Oct 29, 2024 •

edited

Loading

BartChris commented Oct 29, 2024 •

edited

Loading

BartChris commented Oct 30, 2024 •

edited

Loading

BartChris Oct 30, 2024 •

edited

Loading

BartChris commented Oct 31, 2024 •

edited

Loading

BartChris commented Oct 31, 2024 •

edited

Loading

solth commented Dec 2, 2024 •

edited

Loading

BartChris Dec 12, 2024 •

edited

Loading

BartChris Dec 13, 2024 •

edited

Loading

matthias-ronge Feb 4, 2025 •

edited

Loading

BartChris commented Dec 17, 2024 •

edited

Loading

BartChris commented Dec 17, 2024 •

edited

Loading