Skip to content

Conversation

@davidcaron
Copy link
Contributor

Overview

When I tested sending lots of small requests to pywps:

  • Multiple threads couldn't write to the database at the same time (a global session was shared between threads)
  • Workers would take the same request twice from the stored_requests table
  • Unfinished requests would pile up and being considered as running, even though an error happened

I found that making Pywps completely threadsafe and multi-process safe is challenging. When I started using locks, the forked Pywps workers would copy the opened lock to the forked process, leading to a deadlock.

Another solution would be to spawn processes instead of forking them. But this is Python 3.4+ only.

So I fixed a few things so that race conditions are less likely to happen.

Related Issue / Discussion

Also, I think 24f48dd fixes #450

Additional Information

Contribution Agreement

(as per https://github.com/geopython/pywps/blob/master/CONTRIBUTING.rst#contributions-and-licensing)

  • I'd like to contribute [feature X|bugfix Y|docs|something else] to PyWPS. I confirm that my contributions to PyWPS will be compatible with the PyWPS license guidelines at the time of contribution.
  • I have already previously agreed to the PyWPS Contributions and Licensing Guidelines

@coveralls
Copy link

Coverage Status

Coverage remained the same at ?% when pulling 00273c7 on davidcaron:improve-queue-race-conditions into e980517 on geopython:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage remained the same at ?% when pulling 00273c7 on davidcaron:improve-queue-race-conditions into e980517 on geopython:master.

@cehbrecht cehbrecht added the bug label Mar 6, 2019
@cehbrecht cehbrecht added this to the 4.4.0 milestone Mar 6, 2019
@cehbrecht cehbrecht requested review from jachym and ldesousa March 6, 2019 12:39
@cehbrecht
Copy link
Collaborator

@davidcaron is there a difference if we use sqlite or postgres? Would that influence the current PR?

@huard
Copy link
Collaborator

huard commented Mar 6, 2019

I can't review this PR but these issues have caused us some trouble lately. We'd be really glad if this could be fixed.

@davidcaron
Copy link
Contributor Author

@cehbrecht I tested both postgres and sqlite, and I found that postgres handles concurrent connections better than sqlite... which isn't surprising, but sqlite should at least do the job.

This PR improves connections using both sqlite and postgres, but in my tests, I found that sqlite had more problems than postgres, and I couldn't pinpoint what caused sqlite to crash. I got file corruption error messages from sqlite.

Keep in mind that I was sending short bursts of 4-5 requests at the same time, and I never got any issue when the request has time to start processing before receiving another one.

Copy link
Collaborator

@cehbrecht cehbrecht left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

double checked locally with Emu WPS. Code looks fine.

@cehbrecht
Copy link
Collaborator

@davidcaron @jachym I don't have much insight in the database handling but since it improves the situation I will merge it.

@cehbrecht cehbrecht merged commit 034b185 into geopython:master Mar 7, 2019
@cehbrecht
Copy link
Collaborator

@davidcaron done. Thanks :)

@davidcaron davidcaron deleted the improve-queue-race-conditions branch March 8, 2019 18:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Response document of a stored job is not updated

4 participants