Upgrade Boost to 1.70, fix inefficient connection handling #6113

mjjbell · 2021-08-29T21:00:29Z

Issue

A request to osrm-routed can be assigned to a thread which is currently busy processing another request, even when there
are other threads/cores available. This unnecessarily delays the response, and can make requests appear to hang when awaiting CPU intensive requests to finish.

See #6039 for details of how to reproduce the issue.

osrm-routed server implementation is heavily influenced by the HTTP server 3 example in the
Boost.Asio docs. By upgrading to Boost 1.70 and updating the server connections to match the example provided in the 1.70
release, the problem is resolved.

The diff of the changes to the Boost.Asio stack are vast, so it's difficult to identify the exact cause. However
the implementation change is to push the strand of execution into the socket (and timer) objects, which suggests it's
fixing the type of threading issue we are observing.

This PR therefore also bumps the Boost dependency to >= 1.70. Given this was released over two years ago and many people will have already been using this version as a dependency when building OSRM, I see this as low risk.

Tasklist

CHANGELOG.md entry (How to write a changelog entry)
update relevant Wiki pages
add tests (see testing documentation
review
adjust for comments
cherry pick to release branch

Requirements / Relations

Fixes #6039

mjjbell · 2021-08-29T21:01:19Z

src/server/connection.cpp

-Connection::Connection(boost::asio::io_service &io_service, RequestHandler &handler)
-    : strand(io_service), TCP_socket(io_service), timer(io_service), request_handler(handler)
+Connection::Connection(boost::asio::io_context &io_context, RequestHandler &handler)
+    : strand(boost::asio::make_strand(io_context)), TCP_socket(strand), timer(strand),


The important change is here. Instantiating the socket and timer with the strand.

mjjbell · 2021-08-29T21:02:45Z

src/server/connection.cpp

@@ -27,12 +28,11 @@ boost::asio::ip::tcp::socket &Connection::socket() { return TCP_socket; }
 /// Start the first asynchronous operation for the connection.
 void Connection::start()
 {
-    TCP_socket.async_read_some(
-        boost::asio::buffer(incoming_data_buffer),
-        strand.wrap(boost::bind(&Connection::handle_read,


This means we no longer need to explicitly use the strand in our connection event handlers.

mjjbell · 2021-08-29T21:04:40Z

CMakeLists.txt

@@ -36,7 +36,7 @@ list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake")

 if(ENABLE_MASON)
  # versions in use
-  set(MASON_BOOST_VERSION "1.65.1")
+  set(MASON_BOOST_VERSION "1.73.0")


Upgrade Mason Boost to 1.73 as this is the first version >= 1.70 Mason provides that has all the libraries OSRM needs.

TheMarex

Thanks for digging. This code was literally c&p from the boost example mostly for testing and never really ran in our production system (HTTP went via the node bindings), I presume this is why this went unnoticed for so long.

mjjbell · 2021-09-04T13:09:23Z

Needs #6119 for CI to pass.

hoerup · 2021-09-27T19:51:40Z

Why not go with debian-bullseye and boost 1.74 for the docker image ?

mjjbell · 2021-09-28T12:37:29Z

Why not go with debian-bullseye and boost 1.74 for the docker image ?

Hmm, it probably wasn't released when I started working on this. Will update.

hoerup · 2021-09-28T13:16:02Z

If you're updating boost requirement past 1.66 there is also the option of using boost.beast - but that is an entirely different task

A request to osrm-routed can be assigned to a thread which is currently busy processing another request, even when there are other threads/cores available. This unnecessarily delays the response, and can make requests appear to hang when awaiting CPU intensive requests to finish. The issue looks like a bug in Boost.Asio multithreaded networking stack. osrm-routed server implementation is heavily influenced by the HTTP server 3 example in the Boost.Asio docs. By upgrading to Boost 1.70 and updating the server connections to match the example provided in the 1.70 release, the problem is resolved. The diff of the changes to the Boost.Asio stack are vast, so it's difficult to identify the exact cause. However the implementation change is to push the strand of execution into the socket (and timer) objects, which suggests it could fix the type of threading issue we are observing.

mjjbell · 2021-09-30T21:05:46Z

If you're updating boost requirement past 1.66 there is also the option of using boost.beast - but that is an entirely different task

I'll leave that for someone else 🙂

I'm going to include #6119 in this PR to get it over the line.

It upgrades Ubuntu CI to 20.04 and bumps the GCC and Node matrices to something more recent. Hopefully that's not too controversial.

Moves Linux CI builds to the latest Ubuntu LTS. - Bumps the GCC matrix to versions {7,8,9,10,11}, making 9 the default for testing non-standard builds. - Bump Node matrix to {12,14,16,LTS,latest} now that 10 is EOL. - Fixes to CI builds due to library changes on newer distro.

nilsnolde · 2021-10-02T16:42:36Z

I have little experience with cmake, but I could try my luck: is it possible to make 1.70 requirement optional for osrm-routed @mjjbell ? Very soon I’ll try to build some Python bindings for cross platform (and open source them based on pyosrm/cython), but that needs a manylinux compatible system, the freshest available is Debian 9 with boost 1.61. Of course it won’t need osrm-routed but glancing at the cmakelists.txt it’s not (yet) possible to exclude the server right? Would it be accepted to make the server component optional so one can safely downgrade the boost requirement?

mjjbell · 2021-10-02T19:03:07Z

Commented over in #6139, building the server could be optional.

v5.27.0 - Changes from 5.26.0 - API: - ADDED: Add Flatbuffers support to NodeJS bindings. [Project-OSRM#6338](Project-OSRM#6338) - CHANGED: Add `data_version` field to responses of all services. [Project-OSRM#5387](Project-OSRM#5387) - FIXED: Use Boost.Beast to parse HTTP request. [Project-OSRM#6294](Project-OSRM#6294) - FIXED: Fix inefficient osrm-routed connection handling [Project-OSRM#6113](https://gihub.com/Project-OSRM/osrm-backend/pull/6113) - FIXED: Fix HTTP compression precedence [Project-OSRM#6113](Project-OSRM#6113) - NodeJS: - FIXED: Support `skip_waypoints` in Node bindings [Project-OSRM#6060](Project-OSRM#6060) - Misc: - ADDED: conanbuildinfo.json for easy reading of dependencies [Project-OSRM#6388](Project-OSRM#6388) - CHANGED: Improve performance of JSON rendering. Fix undefined behaviour in JSON numbers formatting. [Project-OSRM#6380](Project-OSRM#6380) - ADDED: Add timestamps for logs. [Project-OSRM#6375](Project-OSRM#6375) - CHANGED: Improve performance of map matching via getPathDistance optimization. [Project-OSRM#6378](Project-OSRM#6378) - CHANGED: Optimize RestrictionParser performance. [Project-OSRM#6344](Project-OSRM#6344) - ADDED: Support floats for speed value in traffic updates CSV. [Project-OSRM#6327](Project-OSRM#6327) - CHANGED: Use Lua 5.4 in Docker image. [Project-OSRM#6346](Project-OSRM#6346) - CHANGED: Remove redundant nullptr check. [Project-OSRM#6326](Project-OSRM#6326) - CHANGED: missing files list is included in exception message. [Project-OSRM#5360](Project-OSRM#5360) - CHANGED: Do not use deprecated Callback::Call overload in Node bindings. [Project-OSRM#6318](Project-OSRM#6318) - FIXED: Fix distance calculation consistency. [Project-OSRM#6315](Project-OSRM#6315) - FIXED: Fix performance issue after migration to sol2 3.3.0. [Project-OSRM#6304](Project-OSRM#6304) - CHANGED: Pass osm_node_ids by reference in osrm::updater::Updater class. [Project-OSRM#6298](Project-OSRM#6298) - FIXED: Fix bug with reading Set values from Lua scripts. [Project-OSRM#6285](Project-OSRM#6285) - FIXED: Bug in bicycle profile that caused exceptions if there is a highway=bicycle in the data. [Project-OSRM#6296](Project-OSRM#6296) - FIXED: Internal refactoring of identifier types used in data facade [Project-OSRM#6044](Project-OSRM#6044) - CHANGED: Update docs to reflect recent build and dependency changes [Project-OSRM#6383](Project-OSRM#6383) - Build: - REMOVED: Get rid of Mason. [Project-OSRM#6387](Project-OSRM#6387) - CHANGED: Use clang-format from CI base image. [Project-OSRM#6391](Project-OSRM#6391) - ADDED: Build Node bindings on Windows. [Project-OSRM#6334](Project-OSRM#6334) - ADDED: Configure cross-compilation for Apple Silicon. [Project-OSRM#6360](Project-OSRM#6360) - CHANGED: Use apt-get to install Clang on CI. [Project-OSRM#6345](Project-OSRM#6345) - CHANGED: Fix TBB in case of Conan + NodeJS build. [Project-OSRM#6333](Project-OSRM#6333) - CHANGED: Migrate to modern TBB version. [Project-OSRM#6300](Project-OSRM#6300) - CHANGED: Enable performance-move-const-arg clang-tidy check. [Project-OSRM#6319](Project-OSRM#6319) - CHANGED: Use the latest node on CI. [Project-OSRM#6317](Project-OSRM#6317) - CHANGED: Migrate Windows CI to GitHub Actions. [Project-OSRM#6312](Project-OSRM#6312) - ADDED: Add smoke test for Docker image. [Project-OSRM#6313](Project-OSRM#6313) - CHANGED: Update libosmium to version 2.18.0. [Project-OSRM#6303](Project-OSRM#6303) - CHANGED: Remove EXACT from find_package if using Conan. [Project-OSRM#6299](Project-OSRM#6299) - CHANGED: Configure Undefined Behaviour Sanitizer. [Project-OSRM#6290](Project-OSRM#6290) - CHANGED: Use Conan instead of Mason to install code dependencies. [Project-OSRM#6284](Project-OSRM#6284) - CHANGED: Migrate to C++17. Update sol2 to 3.3.0. [Project-OSRM#6279](Project-OSRM#6279) - CHANGED: Update macOS CI image to macos-11. [Project-OSRM#6286](Project-OSRM#6286) - CHANGED: Enable even more clang-tidy checks. [Project-OSRM#6273](Project-OSRM#6273) - CHANGED: Configure CMake to not build flatbuffers tests and samples. [Project-OSRM#6274](Project-OSRM#6274) - CHANGED: Enable more clang-tidy checks. [Project-OSRM#6270](Project-OSRM#6270) - CHANGED: Configure clang-tidy job on CI. [Project-OSRM#6261](Project-OSRM#6261) - CHANGED: Use Github Actions for building container images [Project-OSRM#6138](Project-OSRM#6138) - CHANGED: Upgrade Boost dependency to 1.70 [Project-OSRM#6113](Project-OSRM#6113) - CHANGED: Upgrade Ubuntu CI builds to 20.04 [Project-OSRM#6119](Project-OSRM#6119) - CHANGED: Make building osrm-routed optional [Project-OSRM#6144](Project-OSRM#6144) - FIXED: Run all unit tests in CI [Project-OSRM#5248](Project-OSRM#5248) - FIXED: Fix installation of Mason CMake and 32 bit CI build [Project-OSRM#6170](Project-OSRM#6170) - FIXED: Fixed Node docs generation check in CI. [Project-OSRM#6058](Project-OSRM#6058) - CHANGED: Docker build, enabled arm64 build layer [Project-OSRM#6172](Project-OSRM#6172) - CHANGED: Docker build, enabled apt-get update/install caching in separate layer for build phase [Project-OSRM#6175](Project-OSRM#6175) - FIXED: Bump CI complete meta job to ubuntu-20.04 [Project-OSRM#6323](Project-OSRM#6323) - CHANGED: Node packages are now scoped by @Project-OSRM [Project-OSRM#6386](Project-OSRM#6386) - Routing: - CHANGED: Lazily generate optional route path data [Project-OSRM#6045](Project-OSRM#6045) - FIXED: Completed support for no_entry and no_exit turn restrictions. [Project-OSRM#5988](Project-OSRM#5988) - ADDED: Add support for non-round-trips with a single fixed endpoint. [Project-OSRM#6050](Project-OSRM#6050) - FIXED: Improvements to maneuver override processing [Project-OSRM#6125](Project-OSRM#6125) - ADDED: Support snapping to multiple ways at an input location. [Project-OSRM#5953](Project-OSRM#5953) - FIXED: Fix snapping target locations to ways used in turn restrictions. [Project-OSRM#6339](Project-OSRM#6339) - ADDED: Support OSM traffic signal directions. [Project-OSRM#6153](Project-OSRM#6153) - FIXED: Ensure u-turn exists in intersection view. [Project-OSRM#6376](Project-OSRM#6376) - FIXED: Gracefully handle no-turn intersections in guidance processing. [Project-OSRM#6382](Project-OSRM#6382) - Profile: - CHANGED: Bicycle surface speeds [Project-OSRM#6212](Project-OSRM#6212) - Tools: - CHANGED: Do not generate intermediate .osrm file in osrm-extract. [Project-OSRM#6354](Project-OSRM#6354)

mjjbell commented Aug 29, 2021

View reviewed changes

mjjbell mentioned this pull request Aug 29, 2021

The router.project-osrm.org is returning a 502 bad gateway once in a while #6102

Closed

TheMarex approved these changes Sep 3, 2021

View reviewed changes

mjjbell force-pushed the mbell/connection_threads branch from 0a4097f to 4a820ce Compare September 4, 2021 00:57

mjjbell force-pushed the mbell/connection_threads branch from 4a820ce to 84ed6f7 Compare September 30, 2021 21:08

mjjbell merged commit eb3bf5c into Project-OSRM:master Sep 30, 2021

This was referenced Sep 30, 2021

Upgrade Ubuntu CI builds to use 20.04 #6119

Closed

Added GCC10 support to CI builds. #5857

Closed

nilsnolde mentioned this pull request Oct 2, 2021

boost 1.70 breaks important linux distros #6139

Closed

This was referenced Oct 3, 2021

OSRM taking long time to fetch the duration matrix #6140

Closed

Make osrm-routed optional #6144

Merged

mjjbell mentioned this pull request Oct 15, 2021

osrm-routed monitoring in the prometheus format #6147

Closed

nilsnolde mentioned this pull request Jan 4, 2022

Build instructions for Ubuntu fail due to outdated libboost #6184

Closed

mjjbell mentioned this pull request Aug 1, 2022

Use Boost.Beast to parse HTTP request #6294

Merged

6 tasks

mjjbell mentioned this pull request Aug 18, 2023

Have you considered upgrading from http1.0 to http1.1？ #6672

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade Boost to 1.70, fix inefficient connection handling #6113

Upgrade Boost to 1.70, fix inefficient connection handling #6113

mjjbell commented Aug 29, 2021 •

edited

Loading

mjjbell Aug 29, 2021

mjjbell Aug 29, 2021

mjjbell Aug 29, 2021

TheMarex left a comment

mjjbell commented Sep 4, 2021

hoerup commented Sep 27, 2021

mjjbell commented Sep 28, 2021

hoerup commented Sep 28, 2021

mjjbell commented Sep 30, 2021

nilsnolde commented Oct 2, 2021

mjjbell commented Oct 2, 2021

Upgrade Boost to 1.70, fix inefficient connection handling #6113

Upgrade Boost to 1.70, fix inefficient connection handling #6113

Conversation

mjjbell commented Aug 29, 2021 • edited Loading

Issue

Tasklist

Requirements / Relations

mjjbell Aug 29, 2021

Choose a reason for hiding this comment

mjjbell Aug 29, 2021

Choose a reason for hiding this comment

mjjbell Aug 29, 2021

Choose a reason for hiding this comment

TheMarex left a comment

Choose a reason for hiding this comment

mjjbell commented Sep 4, 2021

hoerup commented Sep 27, 2021

mjjbell commented Sep 28, 2021

hoerup commented Sep 28, 2021

mjjbell commented Sep 30, 2021

nilsnolde commented Oct 2, 2021

mjjbell commented Oct 2, 2021

mjjbell commented Aug 29, 2021 •

edited

Loading