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

Don't delete shared memory segments if we're about to use them #1892

Merged
merged 3 commits into from
Jan 19, 2016

Conversation

danpat
Copy link
Member

@danpat danpat commented Jan 14, 2016

If osrm-datastore is run an even number of times, it will set the layout back to the currently used one. This code changes the SharedDataFacade to check if the new layout is still referring to the same one. The re-initialization is only done if the timestamp has also changed.

This is a partial fix to #1888

It fixes the "simple reproduction recipe", but I can still cause errors if I run osrm-datastore and curl in tight loops. However, running osrm-datastore in a tight loop seems like a really oddball case.

@danpat danpat changed the title Don't delete shared memory segments if we're about to use them [NOT READY] Don't delete shared memory segments if we're about to use them Jan 14, 2016
@danpat
Copy link
Member Author

danpat commented Jan 14, 2016

Hmm, this doesn't seem to fix all cases. If I put osrm-routed under heavy load, then running osrm-datastore can still cause osrm-routed to hang (with high CPU usage).

@danpat
Copy link
Member Author

danpat commented Jan 15, 2016

Additional changes clear up a lot of the problems. However, if I run osrm-datastore very rapidly, I still seem to be able to trigger exceptions in osrm-routed if it's also under heavy load. However, these instances are massively decreased compared to before, so this fix is useful.

This adds slight locking overhead to queries when using the SharedDataFacade. Unfortunately, it's unavoidable overhead if we want correct behaviour, unless we want to investigate writing some kind of lock-free implementation, which is major work.

@TheMarex TheMarex mentioned this pull request Jan 15, 2016
8 tasks
@TheMarex TheMarex changed the title [NOT READY] Don't delete shared memory segments if we're about to use them Don't delete shared memory segments if we're about to use them Jan 19, 2016
danpat and others added 3 commits January 19, 2016 17:44
Without this, it's possible for CheckAndReloadFacade to start working
while a query is still in progress, leading to undefined behaviour.
@TheMarex TheMarex force-pushed the fix_datafacade_loadorder branch from c9d9df5 to e21eaa4 Compare January 19, 2016 16:44
@TheMarex TheMarex merged commit e21eaa4 into develop Jan 19, 2016
@TheMarex TheMarex deleted the fix_datafacade_loadorder branch January 19, 2016 20:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants