Invalid NodeIDs in Route Response

[wangyoucao577]

With annotations=true parameter, we can get full nodes of each leg in route response. But some of the node IDs are invalid.

Here's a example:

• Request against my deployment: GET /route/v1/driving/-122.006349,37.364336;-121.875654,37.313767?alternatives=3&annotations=true
• Got some nodeIDs in response/routes/legs/annotations/nodes:49873171102,237301600001101,49939882102,49939887102,1008867724102,1008867725102,955126824102,9280805980001100,49939901102

The ID 9280805980001100 is incorrect. The correct value is 9280805980001101, which is correct in OSRM compiled data:

$ /osrm-build/osrm-files-extractor -alsologtostderr -f map.osrm.nbg_nodes -summary 1000000000 2>&1 | grep 92808059800011
I0410 06:24:26.269722      32 contents.go:59]     osm_node_ids[220188955] 9280805980001101        

It might be something wrong in unpack step, any may be caused by some IDs overflow: the problem seems only occurred on extermly long IDs.
Will investigate further.

[wangyoucao577]

I found that the issue is caused by JSON values storing instead of unpacking.

Generated routes will be converted to JSON before output. In below codes, every node_id will be stored into util::json::Array nodes;.

osrm-backend/include/engine/api/route_api.hpp

Lines 324 to 333 in af51dcc

	if (requested_annotations & RouteParameters::AnnotationsType::Nodes)
	{
	util::json::Array nodes;
	nodes.values.reserve(leg_geometry.osm_node_ids.size());
	for (const auto node_id : leg_geometry.osm_node_ids)
	{
	nodes.values.push_back(static_cast<std::uint64_t>(node_id));
	}
	annotation.values["nodes"] = std::move(nodes);
	}

The util::json::Array is a struct containes std::vector<Value> values,

osrm-backend/include/util/json_container.hpp

Lines 145 to 148 in af51dcc

	struct Array
	{
	std::vector<Value> values;
	};

and the Value defined by

osrm-backend/include/util/json_container.hpp

Lines 122 to 128 in af51dcc

	using Value = mapbox::util::variant<String,
	Number,
	mapbox::util::recursive_wrapper<Object>,
	mapbox::util::recursive_wrapper<Array>,
	True,
	False,
	Null>;

Since the Number is the only type that Value matchs with std::uint64_t, the nodes.values.push_back(static_cast<std::uint64_t>(node_id)); will result to convert the node_id and stores into the Number.
However, the Number defines as double which may insufficient for extermely long std::uint64_t.

osrm-backend/include/util/json_container.hpp

Lines 89 to 94 in af51dcc

	struct Number
	{
	Number() = default;
	Number(double value_) : value{value_} {}
	double value;
	};

In this case, the nodeID 9280805980001101 will be cut when convert it to double. Here's some test code:

// Example program
#include <iostream>
#include <string>
#include <cstdint>
#include <iomanip>

int main()
{
  std::uint64_t id = 9280805980001101;
  double d_id = static_cast<double>(id);
  std::uint64_t convert_back_id = static_cast<std::uint64_t>(d_id);
  
  std::cout << id << std::endl;
  std::cout << std::fixed << std::setprecision(6) << d_id << std::endl;
  std::cout << convert_back_id << std::endl;
}

// output: 
9280805980001101
9280805980001100.000000
9280805980001100

[wangyoucao577]

Have filed issue Project-OSRM#5716 for more discussion.

[wangyoucao577]

A good solution to solve the problem maybe implement various number types in the JSON util, not only one more uint64_t number, but also others such as int64_t, int32, uint32, etc. I think it needs more discussion on how to implement it in current situation.

Before get a reasonable conclusion, I'd like to go with a simple improvement first: return string instead of Number for nodes in annotations.
Most of time we'll only need the nodes as index instead of math calculation, so string should be enough for the usage. But it may still have some downgrades:

• May cause response size increasing since string may possible to take more bytes.
• It will also affect receive side when parsing.

Anyway, let's have a try to see whether it works or not.

[wangyoucao577]

I tried a long route(about 4700km, from us west to us east) to see whether the response size acceptable, here's some test result:

-rw-r--r-- 1 root root  318 Apr 17 03:09 4700km-string.header.log
-rw-r--r-- 1 root root 9.3M Apr 17 03:09 4700km-string.json
-rw-r--r-- 1 root root 2.9M Apr 17 03:09 4700km-string.json.gz
-rw-r--r-- 1 root root  318 Apr 17 01:36 4700km.header.log
-rw-r--r-- 1 root root 9.1M Apr 17 01:36 4700km.json
-rw-r--r-- 1 root root 3.0M Apr 17 01:36 4700km.json.gz

The string response only increase about 200KB, and actually it even reduces about 100KB if enable "Accept-Encoding: gzip". It seems not a problem.

One more problem needs highlight is that nearest also returns nodes. Return string instead of Number may not make sense for a real functional service. So I decided to leave it for now. Once we got final solution, both of them should be solved by the right way.