Skip to content

Add regression tests for zero-speed segment updates #7341

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.

Sign up for GitHub

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

Open
afarber wants to merge 5 commits into
base: master
Choose a base branch
from
+150 −2
Open

Add regression tests for zero-speed segment updates #7341

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e5185d4
Fix data race in turn penalty updates during customization
afarber Jan 23, 2026
db74c6f
Adapt the test: route on same way with later closed segments
afarber Jan 24, 2026
748ec17
Add entry log
afarber Jan 24, 2026
21db96e
Add turn_id uniqueness test
afarber Jan 29, 2026
fb424df
Remove unnecessary mutex (turn_id is unique per edge)
afarber Jan 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
- CHANGED: Upgrade Cucumber-js to v12 [#7221](https://github.com/Project-OSRM/osrm-backend/pull/7221)
- CHANGED: Add libcap-setcap to alpine dockerfile [#7241](https://github.com/Project-OSRM/osrm-backend/issues/7241)
- FIXED: Minor misspellings found in source code, comments and documents [#7215](https://github.com/Project-OSRM/osrm-backend/pull/7215)
- ADDED: Regression tests for zero-speed segment updates and unit test for turn_id uniqueness [#7332](https://github.com/Project-OSRM/osrm-backend/issues/7332)
- Profiles:
- FIXED: Bicycle and foot profiles now don't route on motor roads [#6697](https://github.com/Project-OSRM/osrm-backend/pull/6697)
- FIXED: Use `cycleway:both` if available. [#6179](https://github.com/Project-OSRM/osrm-backend/issues/6179)
Expand Down
71 changes: 71 additions & 0 deletions features/testbot/zero-speed-updates.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,3 +189,74 @@ Feature: Check zero speed updates
| waypoints | trips | code |
| a,b,c,d | | NoTrips |
| d,b,c,a | | NoTrips |


Scenario: Closing intersection should not block turns at earlier intersections
Given the node map
"""
x
|
a - 1 - b - 2 - c - 3 - d
|
y
"""

And the ways
| nodes |
| abcd |
| xb |
| cy |
And the contract extra arguments "--segment-speed-file {speeds_file}"
And the customize extra arguments "--segment-speed-file {speeds_file}"
# Node IDs (top-to-bottom, left-to-right): x=1, a=2, b=3, c=4, d=5, y=6
# Close ALL segments connecting to node c (4):
# - b-c (3,4) and c-b (4,3)
# - c-d (4,5) and d-c (5,4)
# - c-y (4,6) and y-c (6,4)
And the speed file
"""
3,4,0
4,3,0
4,5,0
5,4,0
4,6,0
6,4,0
"""

# Route from position 1 (on open segment a-b) to x should work
# because turn at b is not affected by closure at c
When I route I should get
| from | to | code |
| 1 | x | Ok |
| a | x | Ok |


Scenario: Closing segment should not cascade to parallel routes
Given the node map
"""
a - 1 - b - 2 - c
| |
d - 3 - e - 4 - f
"""

And the ways
| nodes |
| abc |
| def |
| ad |
| cf |
And the contract extra arguments "--segment-speed-file {speeds_file}"
And the customize extra arguments "--segment-speed-file {speeds_file}"
# Node IDs (top-to-bottom, left-to-right): a=1, b=2, c=3, d=4, e=5, f=6
# Close segment b-c (2,3)
And the speed file
"""
2,3,0
3,2,0
"""

# Route on def should work, route a->f uses alternative via def
When I route I should get
| from | to | code |
| 3 | 4 | Ok |
| a | f | Ok |
5 changes: 3 additions & 2 deletions src/updater/updater.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -779,8 +779,9 @@ Updater::LoadAndUpdateEdgeExpandedGraph(std::vector<extractor::EdgeBasedEdge> &e
if (turn_weight_penalty < TurnPenalty{0})
{
util::Log(logWARNING)
<< "turn penalty " << turn_weight_penalty
<< " is too negative: clamping turn weight to " << weight_min_value;
<< "turn penalty " << turn_weight_penalty << " for turn_id "
<< edge.data.turn_id << " (edge " << edge.source << " -> " << edge.target
<< ") is too negative: clamping turn weight to " << weight_min_value;
turn_weight_penalty = alias_cast<TurnPenalty>(weight_min_value - new_weight);
turn_weight_penalties[edge.data.turn_id] = turn_weight_penalty;
}
Expand Down
75 changes: 75 additions & 0 deletions unit_tests/updater/turn_id_uniqueness.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
#include "extractor/edge_based_edge.hpp"

#include <boost/test/unit_test.hpp>

#include <algorithm>
#include <set>
#include <vector>

BOOST_AUTO_TEST_SUITE(turn_id_uniqueness)

using namespace osrm;

// This test documents and verifies the invariant that turn_id is unique per edge.
//
// During extraction, EdgeBasedGraphFactory renumbers all edges so that each edge's
// turn_id equals its index in the edge list (turn_id = 0, 1, 2, ..., N-1).
//
// The parallel update loop in Updater::LoadAndUpdateEdgeExpandedGraph relies on this
// property: since each edge has a unique turn_id, parallel threads write to different
// slots in turn_weight_penalties[] without racing.

BOOST_AUTO_TEST_CASE(turn_id_assignment_simulates_extraction)
{
// Simulate the turn_id renumbering done by EdgeBasedGraphFactory:
// each edge's turn_id is set to its index in the edge list
std::vector<extractor::EdgeBasedEdge> edges(1000);

// Assign turn_id = index (mimicking the extractor renumbering)
for (std::size_t i = 0; i < edges.size(); ++i)
{
edges[i].data.turn_id = static_cast<NodeID>(i);
}

// Verify all turn_ids are unique
std::set<NodeID> turn_ids;
for (const auto &edge : edges)
{
auto [it, inserted] = turn_ids.insert(edge.data.turn_id);
BOOST_CHECK_MESSAGE(inserted,
"turn_id " << edge.data.turn_id
<< " is not unique - parallel updates would race");
}

BOOST_CHECK_EQUAL(turn_ids.size(), edges.size());
}

BOOST_AUTO_TEST_CASE(parallel_update_safety_with_unique_turn_ids)
{
// This test verifies that with unique turn_ids, parallel writes to different
// array indices are safe (no synchronization needed).
const std::size_t num_edges = 10000;
std::vector<int> penalties(num_edges, 0);
std::vector<extractor::EdgeBasedEdge> edges(num_edges);

// Assign unique turn_ids
for (std::size_t i = 0; i < edges.size(); ++i)
{
edges[i].data.turn_id = static_cast<NodeID>(i);
}

// Simulate parallel updates (sequentially here, but the point is each write
// goes to a different index)
for (const auto &edge : edges)
{
penalties[edge.data.turn_id] = edge.data.turn_id + 100;
}

// Verify each slot was written correctly
for (std::size_t i = 0; i < penalties.size(); ++i)
{
BOOST_CHECK_EQUAL(penalties[i], static_cast<int>(i + 100));
}
}

BOOST_AUTO_TEST_SUITE_END()
Loading