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

Use global mutex instead file lock to fix issues with threaded mpm's #1224

Closed
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@mturk

mturk commented Oct 6, 2016

Patch fixes persistent storage data corruption issues with threaded mpm's for Apache Httpd. Reason for that is because the persistent storage locking is based on SDBM file locking mechanism which works only across process boundaries thus making threaded mpm's like worker or event unsafe.
Instead file lock, the patch uses global mutex which is both process and thread safe.

We have observed this issue with
https://issues.jboss.org/browse/JWS-489

@bostrt bostrt referenced this pull request Feb 11, 2017

Closed

GC frequency option #1298

@dkopecek

This comment has been minimized.

dkopecek commented Apr 20, 2017

Any plans to merge or close this PR?

@zimmerle

This comment has been minimized.

Member

zimmerle commented Apr 20, 2017

Hi @dkopecek,

Are you using this pull request? did you notice any performance impact in terms of throughput and latency?

@mturk: I am afraid that this global mutex will reduce a lot the performance, what is your thinking about it?

@zimmerle zimmerle self-assigned this May 8, 2017

@zimmerle zimmerle self-requested a review May 8, 2017

@zimmerle

This comment has been minimized.

Member

zimmerle commented May 19, 2017

Just to confirm my suspicions, I have made a set of tests with and without this patch and the results are demonstrated bellow.

Results with the patch

with

Results without the patch

without

My suspicions is that the apr lock is way slower than the file lock used by the sdbm, therefore we have slower numbers in terms of request per second. The huge difference between the two locks is conceptual: SDBM lock fails while storing value in a locked collection while the apr's one [this patch] hold until the critical section is free of use to perform its operations.

In fact, this patch mitigate the problem, however at a cost of performance reduction.

I would suggest the creation of a compilation flag to define whenever a global lock should be used or not. Having this option will let the user to choose between collection-correctness or performance. In the end of the day the correctness could be handling by a 3rd party process.

@zimmerle

This comment has been minimized.

Member

zimmerle commented May 21, 2017

This pull request was merged and the betterment provided by it will be available upon the utilization of the flag --enable-collection-global-lock during the ./configure.

Thank you @mturk.

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