Problem with iteration over queue in .indexing.IndexQueue.process

[jensens]

I wrote an multilingual fallback index for Plone plone.app.multilingualindexes. In order to get all items up to date, I need to cross-index objects. On unindex_object I need to reindex the same index of all objects in the same translation group. When doing so, for some reason (I did not figured out why) the for loop is reset:

Products.CMFCore/Products/CMFCore/indexing.py

Line 218 in 5bfac55

for op, obj, attributes, metadata in self.queue:

I have two items in the queue and the first item is processed twice (and then unindex fails).
I took the debugger and stepped through to verify this.

No idea how. I guess the iterator of self.queue was reset while unindex runs.

I then tried to work around the iterator reset - which results in this code:

    while self.queue:
        op, obj, attributes, metadata = self.queue.pop(0)

see https://github.com/plone/plone.app.multilingualindexes/blob/e89cc65db76108d7050ef73b6171987fd2ba0d41/src/plone/app/multilingualindexes/patches.py#L18-L19

And now all works fine.

I would really like to provide a pull request, but I first I want to understand what happens here. Also, I have no good idea how to write a test for this effect.

Is anyone here with more insights/ ideas?

[d-maurer]

Jens W. Klein wrote at 2019-7-12 10:11 -0700:

I wrote an multilingual fallback index for Plone [plone.app.multilingualindexes](https://github.com/plone/plone.app.multilingualindexes). In order to get all items up to date, I need to cross-index objects. On `unindex_object` I need to reindex the same index of all objects in the same translation group. When doing so, for some reason (I did not figured out why) the for loop is reset: https://github.com/zopefoundation/Products.CMFCore/blob/5bfac550a557efc10f7d335d3212ad2c82b66435/Products/CMFCore/indexing.py#L218 I have two items in the queue and the first item is processed twice (and then unindex fails). I took the debugger and stepped through to verify this. ...

The indexing queues are thread local. Thus, any side effects must come from the same thread. The queue is changed (in a way that might explain your observation) by `IndexQueue.optimize` which is internally called by `IndexQueue.process`. As a conseqeunce, `process` must not be called recursively. `process` is called at the beginning of the transaction (in a "BeforeCommit" hook) and via `processQueue` (whenever you perform a search). `processQueue` has logic which prevents its recursive call. **HOWEVER** this logic does not protect the call of `process` by the transaction machinery. Thus, if you perform a catalog search (or otherwise call `processQueue`) in your index updates, then the queue can be changed in an uncontrolled (and unexpected) way (which might explain your observation). Note: the `processQueue` call at the beginning of catalog searches is there to ensure that the catalog is up to date (such that the search does not miss modifications by the current transaction). If an index update performs a catalog search, then `IndexQueue.process` is called recursively. Special measures would be necessary to handle such a situation in a safe way. I would implement a recursive call detection in `IndexQueue.process` to check whether `process` is called recursively in your case.

[jensens]

Thus, if you perform a catalog search

Probably that is the problem. In order to get the other translations I use the Translation Manager which queries the catalog.

I would implement a recursive call detection in IndexQueue.process
to check whether process is called recursively in your case

Idea: Set a flag on the queue if a "process" is at work, unset if done.
If flag is set, skip subsequent processing.
Opinions?

[d-maurer]

Jens W. Klein wrote at 2019-7-13 01:32 -0700:

> Thus, if you perform a catalog search Probably that is the problem. In order to get the other translations I use the Translation Manager which queries the catalog. > I would implement a recursive call detection in `IndexQueue.process` to check whether `process` is called recursively in your case Idea: Set a flag on the queue if a "process" is at work, unset if done. If flag is set, skip subsequent processing. Opinions?

Okay, if the translation manager's catalog search is allowed to miss some catalog modifications by the current transaction. Otherwise, if would be better if the recursive `process` call would do the whole remaining work -- something your `while` loop would ensure.