fixed to exit AsyncSolrIndexer thread when ticket is updated by trac-admin command #27

Merged
merged 2 commits into from Mar 16, 2014

Conversation

Projects
None yet
2 participants
Contributor

t2y commented Feb 27, 2014

I found a critical bug when AsyncSolrIndexer is used with trac-admin command, and then this patch is quick hack to fix and works for our environment.

Essentially, Trac instance should have the state about trac-admin, however I'm not sure how to get it. This patch inspects the frame object on stack and find execute_command function calling from AdminCommandManager.

Indexing process for fulltext search is called by some trac-admin command. For example, one of them, changeset sub command with commit hook.

$ trac-admin TRAC_ENV changeset added REPOSITORY CHANGESET

After added the changeset, Trac try to update ticket's comment by commit_updater and AsyncSolrIndexer is instantiated to call the upsert method. Then, AsyncSolrIndexer blocks to get items from queue (see below) . As the result, trac-admin command never exited and its process has remained forever.

https://github.com/dnephin/TracAdvancedSearchPlugin/blob/master/tracadvsearch/backend.py#L137

Contributor

t2y commented Mar 15, 2014

ping?

Owner

dnephin commented Mar 15, 2014

Interesting, I guess the trac-admin command does not expect threads to be alive.

I think there are some easier ways to solve this. You could start the thread withdaemon=False which I think would allow the main thread to terminate.

Alternatively I think you could figure out how it was run by using sys.argv[0] instead of inspecting the frames

Contributor

t2y commented Mar 16, 2014

I tried to set daemon False, but still trac-admin never exited.

threading.Thread.__init__(self)
self.setDaemon(False)
Contributor

t2y commented Mar 16, 2014

Alternatively I think you could figure out how it was run by using sys.argv[0] instead of inspecting the frames

Oh, yes! It's better than inspecting the frames. I will fix my patch.

Contributor

t2y commented Mar 16, 2014

Thank you for good advice. I revised my patch and it works for our environment.

dnephin added a commit that referenced this pull request Mar 16, 2014

Merge pull request #27 from t2y/master
fixed to exit AsyncSolrIndexer thread when ticket is updated by trac-admin command

@dnephin dnephin merged commit a7fc0f8 into dnephin:master Mar 16, 2014

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