Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid to call queue.process a second time when indexing #21

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sgeulette
Copy link
Member

Solution for issue #20.
Need ideally a specific test to verify duplication absence.

@jensens
Copy link
Member

jensens commented Sep 18, 2019

How does this relate to the already merged code in Products.CMFCore?
Is there also work needed?
Is this related to the issue here zopefoundation/Products.CMFCore#79 and the fix here zopefoundation/Products.CMFCore#80 ?

@sgeulette
Copy link
Member Author

It is indeed related to zopefoundation/Products.CMFCore#79.
I can try to apply the same fix.

@pgrunewald
Copy link

We have the very same issue (with duplicate rids). Does the fix works fine for you? Or do you happen to integrate the fix of jensen?

@sgeulette
Copy link
Member Author

The Fix works fine in Plone 4. Not yet tested in higher version.

@jensens
Copy link
Member

jensens commented May 4, 2022

I am not working any more on collective.indexing. Feel free to merge this if it solves the problem. If a release is needed best ping Maurits or Timo.

@pgrunewald
Copy link

I'll report back, after having run this branch against our test suites (with several hundreds robot tests) and doing manual tests.

@pgrunewald
Copy link

I'm pleased to report, that the changes of this PR did not fail any of our tests (more than 400 of them). 👍

Still, a dedicated test here would be useful, maybe one that fails and later succeeds with the changes.

@jensens
Copy link
Member

jensens commented May 12, 2022

@pgrunewald do you volunteer to add such a test? Otherwise I would go for a merge.

@pgrunewald
Copy link

Ok, important update: Unfortunately there is an error, when creating two objects at the very same time:

2022-07-14 08:49:05 ERROR Zope.SiteErrorLog 1657781345.950.531340561751 http://localhost:8080/TUD/intern-test/subsection/portal_factory/EventBoard/eventboard.2022-07-14.1701072359/atct_edit
Traceback (innermost last):
Module ZPublisher.Publish, line 146, in publish
Module Zope2.App.startup, line 303, in commit
Module transaction._manager, line 89, in commit
Module transaction._transaction, line 323, in commit
Module transaction._transaction, line 393, in _callBeforeCommitHooks
Module collective.indexing.transactions, line 57, in before_commit
KeyError: <collective.indexing.queue.IndexQueue object at 0x7fd0e728cd70>

/home/paul/webcms/zinstance/src/collective.indexing/src/collective/indexing/transactions.py(57)before_commit()
56 self.queue.clear()
---> 57 processing.remove(self.queue)
58

ipdb> processing
set([])

The first object was successfully added, but it crashes for the the second one. Our own tests run through, because there are never two threads updating the catalog at the same time. This PR therefor needs more inspection and is not sufficient yet.

On the side note: Our own analysis makes me believe that it is more a problem with the objects being indexed rather than the catalog itself.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants