Feature/ors http routing backend

[sfendrich]

Issue

#206

Tasks

• review

[jcoupey]

Thanks for the PR. Could you please resolve conflicts? Since you branched out from master, a new parameter for error type has been introduced in the Exception constructor. Adding ERROR::ROUTING should do the trick.

[sfendrich]

Sorry for the merge conflict. I forgot to configure our fork to sync with upstream, so I didn't notice the changes on a pull.

[jcoupey]

To test this locally, I have setup a fresh ORS install checked out at 136e5510 on the release_5.0 branch. I'm then running sudo docker-compose up from the docker folder and getting the usual output ending up with memory usage by profiles.

Running vroom -i test.json -a driving-car:localhost -p driving-car:8080 -r ors on a sample instance result in an error with substr because the response is empty. The ORS logs indicate:

$ sudo tail -f logs/tomcat/localhost_access_log.2019-02-28.txt
172.18.0.1 - - [28/Feb/2019:12:22:31 +0000] "POST /ors/v2/matrix/driving-car HTTP/1.1" 404 5

The query looks fine when outputted from orshttp_wrapper.cpp:

POST /ors/v2/matrix/driving-car HTTP/1.1
Accept: */*
Content-Type: application/json
Content-Length: 99
Host: localhost:8080
Connection: close

{"locations":[[8.694724,49.409358],[8.676624,49.415887],[8.683491,49.407064],[8.684864,49.415664]]}

Am I missing something here?

[sfendrich]

I think you are doing the right thing. I don't get it run on my local machine neither. Maybe we have some issue with the URL paths. I'll let you know as soon as I know more.

[nilsnolde]

If I may chime in:
the URL path needs a /json at the very end. Also, /ors/ is not part of the query URL. Instead it's part of the base URL.

[sfendrich]

@nilsnolde I'm not sure whether this will solve the issue. I can request a matrix on my local instance as well as on our server without the endpoint /json. Also the distinction between base-url and query-url makes no sense on the http-level.

[sfendrich]

Ok, I got it running locally by changing the path from /ors/ to /openrouteservice-5.0/ in orshttp_wrapper.cpp. We should find a more stable solution here.

[nilsnolde]

@sfendrich jep, you're right about /json. But of course it makes a difference if it's part of the query string, since you hard-coded that! The base URL is set at vroom level. Check line 46 of src/routing/orshttp_wrapper.cpp

[jcoupey]

It looks as if the docker setting still somehow uses the old API even on the release_5.0 branch: all POST requests end up with a 404 and old-style GET requests succeed, e.g.

http://localhost:8080/ors/routes?profile=foot-walking&coordinates=8.676581,49.418204|8.692803,49.409465

[nilsnolde]

That's fine @jcoupey, we still support both APIs for a year, so that won't change:)

[jcoupey]

we still support both APIs for a year

OK

I got it running locally by changing the path from /ors/ to /openrouteservice-5.0/ in orshttp_wrapper.cpp

Does not work for my local ORS setup. I unsuccessfully tried a couple variants of this direct request using curl:

curl --header "Content-Type:application/json" --data '{"locations":[[8.694724,49.409358],[8.676624,49.415887],[8.683491,49.407064],[8.684864,49.415664]]}' http://localhost:8080/ors/v2/matrix/driving-car

[sfendrich]

Does it work on port 8082?

[jcoupey]

Does it work on port 8082?

No, I can't reach port 8082 after running sudo docker-compose up. But the requests I send to port 8080 are logged in docker/logs/tomcat/localhost_access_log.2019-02-28.txt.

[jcoupey]

I started a release/1.4 branch because I want to tag a first release candidate soon. I plan to merge this PR on both master and the release branch when I've been able to test locally.

[sfendrich]

Did you make any progress? I overlooked the 404 hidden behind the scrollbar. Apparently the service is not running correctly or at a different URL. Here are a few things you could try for debugging:

• Does curl http://localhost:8080/ors/health or curl http://localhost:8080/openrouteservice-5.0/health return status=ready?
• Does tomcat have a ors.log? Does it contain error-messages? Otherwise it should contain passages similar to

2019-03-07 11:19:32,082 INFO [routing.RoutingProfileManager] - [1] Profiles: 'driving-car', location: '<PATH>/openrouteservice/graphs/vehicles-car'.
2019-03-07 11:19:32,175 INFO [borders.CountryBordersReader] - Border geometries read
2019-03-07 11:19:32,183 INFO [borders.CountryBordersReader] - 4 countries read in 1 hiearchies
2019-03-07 11:19:32,184 INFO [borders.CountryBordersReader] - 4 country IDs read
2019-03-07 11:19:32,184 INFO [borders.CountryBordersReader] - ISO 3166-1 CCA2 codes enabled for all countries
2019-03-07 11:19:32,185 INFO [borders.CountryBordersReader] - ISO 3166-1 CCA3 codes enabled for all countries
2019-03-07 11:19:32,185 INFO [borders.CountryBordersReader] - Border ids data read
2019-03-07 11:19:32,185 INFO [borders.CountryBordersReader] - Border openness data read
2019-03-07 11:19:32,255 INFO [routing.RoutingProfileManager] - [1] FlagEncoders: 1, bits used [UNKNOWN]/32.
2019-03-07 11:19:32,255 INFO [routing.RoutingProfileManager] - [1] Capacity: [UNKNOWN]. (edges - 17103, nodes - 12455)
2019-03-07 11:19:32,255 INFO [routing.RoutingProfileManager] - [1] Total time: 0.173s.
2019-03-07 11:19:32,256 INFO [routing.RoutingProfileManager] - [1] Finished at: 2019-03-07 11:19:32.

[jcoupey]

Yes, http://localhost:8080/ors/health does return a 200 with status=ready. Hitting http://localhost:8080/openrouteservice-5.0/health gives me a 404. I'm seeing this logged normally:

$sudo tail -f logs/tomcat/localhost_access_log.2019-03-07.txt 
172.18.0.1 - - [07/Mar/2019:10:43:20 +0000] "GET /ors/health HTTP/1.1" 200 38
172.18.0.1 - - [07/Mar/2019:10:44:58 +0000] "GET /openrouteservice-5.0/health HTTP/1.1" 404 -

The docker/logs/ors folder is empty, while docker/logs/tomcat contains two non-empty files localhost_access_log.2019-03-07.txt (output above) and catalina.2019-03-07.log.

Here is the full output for the docker-compose up command.

[jcoupey]

Based on the health-check, my understanding is that http://localhost:8080/ors/v2/matrix/driving-car should be a valid endpoint to retrieve a matrix with a POST request, right?

[sfendrich]

Yes, that's right. The health-check indicates that the service is running and has loaded the graphs successfully. From the logs everything looks good. If the endpoint does not exist it might be that the matrix-service is deactivated in the app.config. Can you request a route? For example

curl --header "Content-Type:application/json" --data '{"coordinates":[[8.694724,49.409358],[8.676624,49.415887],[8.683491,49.407064],[8.684864,49.415664]]}' http://localhost:8080/ors/v2/directions/driving-car

[jcoupey]

Does not seem to be related with a service deactivated as the POST directions command also gives me a 404. For what it's worth, I'm not able to get a directions request either using GET: http://localhost:8080/ors/v2/directions/driving-car/?coordinates=8.34234,48.23424|8.34423,48.26424 is also a 404.

@sfendrich is this working for you locally on the ORS release_5.0 branch using docker?

[TimMcCauley]

@jcoupey big sorry, I think this is related to:

https://github.com/GIScience/openrouteservice/pull/435/files

Can you please change the Dockerfile from

COPY $APP_CONFIG /ors-core/openrouteservice/WebContent/WEB-INF/app.config

to

COPY $APP_CONFIG /ors-core/openrouteservice/src/main/resources/app.config

and rebuild the container?

[jcoupey]

Thanks @TimMcCauley. Just patching the Dockerfile did not work, but rebuilding everything from your feature-#434-dockerfile branch (at a16a739a) did the trick. Should be enough to check this PR locally!

[jcoupey]

The POST request setup within orshttp_wrapper.cpp seems fine, but I sometime experience strange parsing errors. Example with this input file, using the driving-car profile (OSM data is the default Heidelberg map provided).

I'm hitting an assertion:

$ vroom -i test_car.json -r ors -a driving-car:localhost -p driving-car:8080 -g
vroom: routing/orshttp_wrapper.cpp:186: virtual void vroom::routing::OrsHttpWrapper::add_route_info(vroom::Route&) const: Assertion `!infos.Parse(json_content.c_str()).HasParseError()' failed.
Aborted (core dumped)

due to the fact that json_content is not valid json. Logging the variable shows that one of the route requests ends up containing some garbage:

[...]
"metadata":{"attribution":"openrouteservice.org, OpenStreetMap cont
161
ributors",
[...]

The weird thing is that the same route request using curl is ok.

curl --header "Content-Type:application/json" --data '{"coordinates":[[8.694724,49.409358],[8.687954,49.406841],[8.682332,49.403182],[8.681345,49.406785],[8.680530,49.416585],[8.682418,49.414519],[8.688598,49.415245],[8.694724,49.409358]],"geometry_simplify":"false","continue_straight":"false"}' http://localhost:8080/ors/v2/directions/driving-car

@sfendrich Have you experienced this kind of behaviour? Can you reproduce with the test file?

[sfendrich]

This looks like an HTTP issue. We have to check the header of ORS' response whether it it is sent in chunks. The 161 could be the size of the next data-chunk.
If this is the reason, I'll check whether we can easily configure ORS to send the response as a single message.

[sfendrich]

The reason is Tomcat serving chunked transfer encoding.

[sfendrich]

Following several forum posts, it seems like tomcat has no option to disable chunked transfer encoding (CTE). They argue that the HTTP1.1 standard requires all parties to be able to understand CTE anyways, so there is no point about disabling it. The correct solution for HTTP1.1 would be to enable vroom to parse such messages. This also concerns the OSRM wrapper.

A possible workaround that solved this issue on my setup is to request HTTP1.0, where CTE did not exist (member function OrsHttpWrapper::build_query).

[jcoupey]

@sfendrich thanks for investigating on this.

The correct solution for HTTP1.1 would be to enable vroom to parse such messages

The current implementation for requests uses boost::asio::buffers that don't really care about HTTP standards. It seems like CTE is handled by boost's beast library, that adds the HTTP logic on top of asio. There is a sample example for a client code we could use to replace the current send_then_receive code.

This also concerns the OSRM wrapper

I think the reason why this never hit us with OSRM is that osrm-routed is a custom baked server that only partially implement the standard, so probably CTE is not used at all.

A possible workaround is to request HTTP1.0

Is there any drawback on the ORS side for doing this?

[sfendrich]

I don't think falling back to HTTP1.0 would have a major drawback as Tomcat handles the protocol version automatically. So, maybe this would be a good solution for this PR and reimplementing the HTTP1.1-parsing with boost.beast could be done in a separate issue as it has a broader scope.
@jcoupey Do your tests pass with HTTP1.0?

[jcoupey]

Yes, switching to HTTP/1.0 does the trick. I'm OK to go for that solution right now and ticket the move to 1.1 using boost.beast.

[jcoupey]

@sfendrich I just did a nitpicking review. ;-) Ideally you could also run scripts/format.sh to check consistent formatting with clang-format, and probably add or update a changelog entry?

[sfendrich]

Done.
@jcoupey Thanks a lot for all the comments and the support!

[jcoupey]

I'd like to merge this PR in the release/v1.4 branch to tag a new release candidate but I don't really have a clear view on what ORS compatibility we can offer/announce.

As far as I can tell:

• we're fine with ORS 5.0.0 using a maven build (I did not try that though)
• using docker on 5.0.0 still hit the same error (the patch provided above by @TimMcCauley is now in the development branch for ORS, but not in master).

Are there plans to merge the docker edits any time soon?

[TimMcCauley]

@jcoupey today, stay tuned -

[TimMcCauley]

@jcoupey it is merged now into master.