* print tracebacks and line numbers for Lua runtime errors
* revert format changes
* update changelog with lua traceback, #6564
* revert using protected_function for old GetStringListFromFunction and source_function #6564
* add unit test for line numbers in tracebacks, #6564
* apply clang-format (#6564)
* remove unused test helper function, #6564
* suppress leaksanitizer warnings in extract-tests, #6564
When the extractor encounters a lua runtime error, some osmium objects are not freed. In production this doesn't matter because these errors bring down OSRM. In the tests we catch them to ensure they occur, and the leaksanitizer flags them.
This change takes the existing typedefs for weight, duration and
distance, and makes them proper types, using the existing Alias
functionality.
Primarily this is to prevent bugs where the metrics are switched,
but it also adds additional documentation. For example, it now
makes it clear (despite the naming of variables) that most of the
trip algorithm is running on the duration metric.
I've not made any changes to the casts performed between metrics
and numeric types, they now just more explicit.
Weight and duration penalties are flipped in the lambda function
that applies penalties from traffic signals.
Duration is in deciseconds, whilst weight is multipled by
10^weight_precision, with weight_precision being 1 by default.
Therefore, for default routability profile, the penalties end up
being the same, hence why no tests picked this up.
If distance weight is used however, it will incorrectly apply an
additional penalty to the weight, and not add the traffic signal
delay to the duration in the routing graph.
To confuse things further, in some API responses the values are
correct because they use geometry data instead, but it's still
possible that a sub-optimal route was selected.
However, given the distance weight is in meters, and the additional
penalty per traffic light would be 20, it's unlikely this would
have changed the routing results.
In any case, we correct the function to apply the arguments correctly.
In case we're not able to get access to the unscoped credentials.
Scoped packages are also the recommended approach for projects
managed by a team of developers.
Badly constructed OSM intersections can create OSRM intersection
views that have no valid turns.
The guidance code for segregated intersections tries to look
ahead to the second intersection to ensure lanes are announced
intuitively.
Currently, OSRM assumes there are always turns at the second
intersection that we should consider. This commit corrects
this assumption so that it can now handle badly constructed
OSM intersections with no turns.
Due to some rather complex logic that tries to calculate intersection
angles by looking further up the road, it's possible to return
an intersection view that is missing a u-turn - something which
is assumed to exist in later guidance calculations.
We apply a fix here by ensuring the u-turn is always included in
the returned view.
Currently OSRM parses traffic signal nodes without consideration
for the direction in which the signal applies. This can lead
to duplicated routing penalties, especially when a forward and backward
signal are in close proximity on a way.
This commit adds support for directed signals to the extraction and
graph creation. Signal penalties are only applied in the direction
specified by the OSM tag.
We add the assignment of traffic directions to the lua scripts,
maintaining backwards compatibility with the existing boolean
traffic states.
As part of the changes to the internal structures used for tracking
traffic signals during extraction, we stop serialising/deserialising
signals to the `.osrm` file. The traffic signals are only used by
`osrm-extract` so whilst this is a data format change, it will not
break any existing user processes.
Currently there is an edge-case in the turn restriction implementation,
such that routes can not be found if the target input location snaps
to a way used in a (multi) via restriction.
With the addition of snapping input locations to multiple ways, we
can now also snap to the "duplicate" edges created for the restriction graph,
thereby fixing the problem.
This is achieved by adding the duplicate restriction edges to the
geospatial search RTree.
This does open up the possibility of multiple paths representing exactly
the same route - one using the original edge as a source, the other
using the duplicate restriction graph edge as source. This is fine,
as both edges are represented by the same geometry, so will generate
the same result.
This PR improves routing results by adding support for snapping to
multiple ways at input locations.
This means all edges at the snapped location can act as source/target
candidates for routing search, ensuring we always find the best route,
and not the one dependent on the edge selected.
There is a bug in the deflate compression. Therefore, we do not want
to select this in the default choice for HTTP response compression.
Instead we revert back to the previous precedence, selecting gzip as
the priority.
This change unblocks the osrm-extract debug build, which is
currently failing on a maneuver override assertion.
The processing of maneuver overrides currently has three issues
- It assumes the via node(s) can't be compressed (the failing assertion)
- It can't handle via-paths containing incompressible nodes
- It doesn't interop with turn restriction on the same path
Turn restrictions and maneuver overrides both use the same
from-via-to path representation.
Therefore, we can fix these issues by consolidating their
structures and reusing the path representation for
turn restrictions, which already is robust to the above
issues.
This also simplifies some of the codebase by removing maneuver
override specific path processing.
There are ~100 maneuver overrides in the OSM database, so the
impact on processing and routing will be minimal.
Currently route results are annotated with additional path information,
such as geometries, turn-by-turn steps and other metadata.
These annotations are generated if they are not requested or returned
in the response.
Datasets needed to generate these annotations are loaded and available
to the OSRM process even when unused.
This commit is a first step towards making the loading of these datasets
optional. We refactor the code so that route annotations are only
generated if explicitly requested and needed in the response.
Specifically, we change the following annotations to be lazily generated:
- Turn-by-turn steps
- Route Overview geometry
- Route segment metadata
For example. a /route/v1 request with
steps=false&overview=false&annotations=false
would no longer call the following data facade methods:
- GetOSMNodeIDOfNode
- GetTurnInstructionForEdgeID
- GetNameIndex
- GetNameForID
- GetRefForID
- GetTurnInstructionForEdgeID
- GetClassData
- IsLeftHandDriving
- GetTravelMode
- IsSegregated
- PreTurnBearing
- PostTurnBearing
- HasLaneData
- GetLaneData
- GetEntryClass
Requests that include segment metadata and/or overview geometry
but not turn-by-turn instructions will also benefit from this,
although there is some interdependency with the step instructions
- a call to GetTurnInstructionForEdgeID is still required.
Requests for OSM annotations will understandably still need to
call GetOSMNodeIDOfNode.
Making these changes unlocks the optional loading of data contained in
the following OSRM files:
- osrm.names
- osrm.icd
- osrm.nbg_nodes (partial)
- osrm.ebg_nodes (partial)
- osrm.edges
The internal representation of turn restrictions expects only one
`from` way and only one `to` way.
`no_entry` and `no_exit` turn restrictions can have multiple `from` and
`to` ways respectively. This means they are not fully supported by
OSRM's restriction parser.
We complete support for these turn restriction types by parsing all
ways and converting a valid restriction with multiple `from`/`to` members
into multiple internal restrictions.
Currently /trip supports finding round-trip routes where only the
start or end location is fixed. This PR extends this feature to
non-round-trip requests.
We do this by a new table manipulation that simulates non-round-trip
fixed endpoint requests as a round-trip request.
The data facade interface contains numerous methods for looking up
datapoints by identifiers.
Many of the parameters use the NodeID or EdgeID types. However, these two
identifier types are used for representing three different contexts:
1. Node-based graph edges and nodes
2. Edge-based graph edges and nodes
3. Packed geometries
Consider the use of identifier parameters in these examples:
---
GetWeightPenaltyForEdgeID(const EdgeID id) <- edge-based edge
GetUncompressedForwardWeights(const EdgeID id) <- packed geometry
IsLeftHandDriving(const NodeID id) <- edge-based node
GetBearingClass(const NodeID node) <- node-based node
---
This mixing of contexts within the same interface makes it
difficult to understand the relationships and dependencies between
the OSRM datasets.
For 1. and 2. we continue to use the NodeID and EdgeID types, but
change the interface parameter names to identify them as
edge-based or node-based graph properties.
For 3. we define a new type definition, PackedGeometryID.
These changes are to aid with readability. A next step would be
to strongly type these definitions, leveraging the Alias template
already used for OSM identifiers.
It is quite difficult to maintain current Windows CI, because it uses pre-compiled dependencies stored on wolt.com server and there is low chance to find someone who is able to update it.
* Remove highway=bicycle safety bonus
This removes a bonus for a road class that doesn't exist (cycleway would
be correct), but the code was buggy since safety_bonus is nil and caused
an arithmetic exception. We only caught this 4 (!) years later since
recently a way was tagged with highway=bicycle and made it in our OSM
data leading to preprocessing failures.
NodeJS API docs are out of sync with NodeJS C++ wrapper documentation
due to the doc build breaking back in #4043.
The commit fixes the Node dev dependencies such that `npm run docs`
works again and re-enables the check in CI.
A recent change to the Ubuntu Focal CI worker - either a CMake upgrade,
or default installation of the libexpat-dev library - leads to the
32-bit expat library to not be found by CMake.
FindPackage(EXPAT) finds the library via pkg-config, so the fix is
to explicitly include the i386 pkg-config directory in the
PKG_CONFIG_PATH environment variable list.
During the CI migration to Github Actions, a bug was introduced
such that Mason CMake is not being added to the PATH environment
variable correctly, so it defaults to the CMake installed in the
OS environment.
Simple fixing the typo fails as the Mason CMake version requires
libssl 1.0 as a dependency, whilst the Ubuntu Focal runners are
on the newer libssl 1.1. Therefore, we also bump the Mason
CMake version to 3.21.2.
* Add missing profile name to library extract test.
* Support both tzid and TZID properties on timezone geometry. Improve validation of timezone polygons.
* Missing tzid property wasn't a geojson validation issue, shouldn't have been tested there.
* Use filesystem glob to loop over all test executables so we don't miss any in the future.
Co-authored-by: Michael Bell <michael@mjjbell.com>
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.
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.
* Fix test cache to consider MLD executable changes
Currently the test cache is not invalidated if changes to
osrm-partition or osrm-customize executables. This is problematic
when these executables make breaking changes to the data format.
We fix this by including both in the list of executables that
can invalidate the test cache.
* Executables use hyphens
Each MLD cell has source and destination nodes.
MLD is keeping a |source| x |destination| sized table
for various metrics (distances, durations, etc) from each
source to all destinations in a cell.
It stores all of the values for a metric in one large array, with
an offset for each cell to find its values. The offset is currently
limited to 32 bit values, which overflows on very large graphs
(e.g. Planet OSM).
We fix this by changing the offsets to be uint64_t types.
The generation of level masks for compactly storing partition cells
supports sizes that can be stored in 64 bits.
The current implementation fails if the total bit sum is 64 bits
exactly. A bit shift mechanism is used that is undefined when the
shift size is equal to the bit size of the underlying type. This
generates an incorrect mask value.
We fix this by adding a special case for a 64 bit offset. Given this
code is called at most |level| times, there will be no effect on
performance. We also update the assertions to reflect 64 bit masks
are now supported.
osrm-routed does not immediately clean up a keep-alive connection
when the client closes it. Instead it waits for five seconds
of inactivity before removing.
Given a setup with low file limits and clients opening and
closing a lot of keep-alive connections, it's possible for
osrm-routed to run out of file descriptors whilst it waits for
the clean-up to trigger.
Furthermore, this causes the connection acceptor loop to exit.
Even after the old connections are cleaned up, new ones
will not be created. Any new requests will block until the
server is restarted.
This commit improves the situation by:
- Immediately closing connections on error. This includes EOF errors
indicating that the client has closed the connection. This releases
resources early (including the open file) and doesn't wait for the
timer.
- Log when the acceptor loop exits. Whilst this means the behaviour
can still occur for reasons other than too many open files,
we will at least have visibility of the cause and can investigate further.
In newer versions of cmake, FindBoost uses Imported Targets for library
component variables, rather than file paths to the Boost libraries.
cmake uses these targets when linking (e.g. target_link_library) and
knows how to correctly substitute the values. However, the OSRM
pkg-config file that we generate doesn't do this, and ends up writing
the actual target symbols, hence the errors trying to link Boost::<component>.
To fix this for newer cmake versions, we create an intermediate configure step
that references the linker files for the imported targets. This is followed
by a generate step that performs the correct substitution.
See this thread for more details: https://cmake.org/pipermail/cmake/2018-December/068812.html
This is backwards compatible to the existing min cmake version (3.1).
However, building using cmake 3.1 fails with a package.json parsing error,
so this commit also bumps the min version to 3.2.
Duplicate restriction nodes in the edge-based-graph are currently
not in included in a mapping (.osrm.cnbg_to_ebg) from
node-based-graph edges to edge-based-graph nodes.
This mapping is used by the MLD partitioner to assign EBG nodes
to partitions.
The omission from the mapping means all restriction nodes are
included in a special 'invalid' partition. This special partition
will break the geolocation properties of the multi-level hierarchy.
The partition and its super levels will have a large number of
border nodes and very few internal paths between them.
Given the partitioner is the only consumer of the mapping, we fix
the issue by including the duplicate restriction nodes in the mapping,
so that they are correctly assigned to a partition.
This has measurable improvement on MLD routing.
For a country-sized routing network, the fix reduces routing and table
request computation time by ~2% and ~6% respectively.
