Modify

Opened 4 years ago

Last modified 4 months ago

#12303 needinfo enhancement

[Patch] When downloading objects with Overpass API, use recurse up to fetch referrers

Reported by: simon04 Owned by: simon04
Priority: normal Milestone:
Component: Core Version:
Keywords: mirrored_download overpass Cc: Penegal, tyr_asd

Description

Since #7670, "Download object" can use the Overpass API instead of the OSM API. Since the Overpass API supports the recurse up operator <;, we could make use of the for fetching referrers of the object.

Launching "Download object" with n288207873 and "Download referrers" as well as "Download relation members" set, we have a bunch of HTTP requests:

INFO: GET https://api.openstreetmap.org/api/0.6/nodes?nodes=288207873 -> 200
INFO: GET https://api.openstreetmap.org/api/0.6/node/288207873/ways -> 200 (943 B)
INFO: Way 26,309,072 with 85 nodes has incomplete nodes because at least one node was missing in the loaded data.
INFO: Way 25,648,174 with 13 nodes has incomplete nodes because at least one node was missing in the loaded data.
INFO: GET https://api.openstreetmap.org/api/0.6/node/288207873/relations -> 200 (184 B)
INFO: GET https://api.openstreetmap.org/api/0.6/way/26309072/full -> 200
INFO: GET https://api.openstreetmap.org/api/0.6/way/25648174/full -> 200
INFO: GET https://api.openstreetmap.org/api/0.6/nodes?nodes=288207917,288207914,1088131027,288207911,2939204923,288207910,278027141,288207924,288207920,1088131320,1088131071,288207882,2217273857,2217273859,1088131312,288207872,1318297762,1088131176,391657967,1318297764,1088130924,1088131040,288207853,2217262568,1088131099,288207852,2217273833,279583979,288207855,288207854,2217273835,2217273837,288207851,2217273839,288207845,288207844,279583971,288207847,288207846,2217062382,288977653,288207841,288207840,2217262566,288207843,288977655,288207842,2217273831,2217062378,415506827,288207869,288207868,288207871,288207870,288207865,288207864,372888428,530245199,288207867,2217273854,372888431,288207866,288207861,2217273840,279583986,288207860,288207863,288207862,2217273843,288207857,288207856,2217273845,288207859,288207858,2217273847,2217262538,1088131007,279583950,279583951,2217262531,2217262533,1088131254,2217262534,288207939,1088130986,288207839,2217262556,279583964,2217262544,2217262547,2217262548,2217262549,279583959,2217262551,1088130980 -> 200

Ideally, we could strip this down to one Overpass call. The question is just how to combine node(288207873) and >; and <; in order to accomplish this. Help is welcome … :)

Attachments (4)

12303.patch (8.6 KB) - added by GerdP 4 months ago.
12303.2.patch (14.7 KB) - added by GerdP 4 months ago.
work in progress
12303.3.patch (14.5 KB) - added by GerdP 4 months ago.
12303.4.patch (19.4 KB) - added by GerdP 4 months ago.

Download all attachments as: .zip

Change History (29)

comment:1 Changed 4 years ago by simon04

Milestone: 16.0216.03

comment:2 Changed 4 years ago by Don-vip

Milestone: 16.0316.04

Milestone renamed

comment:3 Changed 4 years ago by Don-vip

Milestone: 16.0416.05

comment:4 Changed 4 years ago by Don-vip

Milestone: 16.0516.06

comment:5 Changed 4 years ago by Don-vip

Keywords: overpass added
Milestone: 16.0616.07

comment:6 Changed 4 years ago by Don-vip

Milestone: 16.0716.08

comment:7 Changed 4 years ago by Don-vip

Milestone: 16.08

comment:8 Changed 2 years ago by anonymous

This probably can in fact now be reduced to one Overpass call using the 'complete' block statement, new to Overpass API 0.7.55, which does the recursion for referrers on the server side. Using the right input set and most probably a loop limit of 1 (simply enough to get parents of the newly discovered elements that aren't part of the original union query)...

https://wiki.openstreetmap.org/wiki/Overpass_API/Overpass_QL#The_block_statement_complete

comment:9 Changed 19 months ago by simon04

Ticket #16537 has been marked as a duplicate of this ticket.

comment:10 Changed 19 months ago by simon04

Cc: Penegal tyr_asd added

comment:11 Changed 4 months ago by GerdP

Owner: changed from simon04 to GerdP
Status: newassigned

See also #18566 and #18624. Working on a class which should calculate the best strategy to download the wanted objects, using either Overpass API or OSM multi-fetch or the /full suffix.

comment:12 Changed 4 months ago by GerdP

The corresponding single overpass query for a list of objects could look like this

(
  node(id:70674932,...); 
  way(id:29201179,...) -> .w; 
  rel(id:69202,...) -> .r;
) -> .i;  // .i : the input set
(
  .i;rel(br); // if any relations are in the input list
  .i;rel(bw); // if any ways ...
  .i;rel(bn);(.i;way(bn);>;); // if any nodes ...
) -> .p; // .p contains the parents and the input set
(.p; - (.w;.r;);); // subtract relations and ways that were in the input set
out meta;

The actual query depends on the existence of the elements. If the initial list doesn't contain relations some parts of the query would be omitted.
The result of this query should be the same as the sequnce of API requests done by the current code.

comment:13 Changed 4 months ago by GerdP

Ah, sorry, forget the above. The subtract clause doesn't work. This was a last minute change. I try to solve the problem that I sometimes get more data then wanted.

comment:14 Changed 4 months ago by GerdP

This seems to deliver the same result as the queries used without overpass api:

(
  (
    node(id:70674932)->.n;.n;rel(bn);.n;way(bn);
    way(id:29201179);rel(bw);
    rel(id:69202);rel(br);
  );node(w);node(r);
);
out meta;
Version 1, edited 4 months ago by GerdP (previous) (next) (diff)

Changed 4 months ago by GerdP

Attachment: 12303.patch added

comment:15 Changed 4 months ago by GerdP

Owner: changed from GerdP to simon04
Status: assignedneedinfo
Summary: When downloading objects with Overpass API, use recurse up to fetch referrers[Patch] When downloading objects with Overpass API, use recurse up to fetch referrers

The patch implements the retrival of parents via overpass api if that overpass api is enabled. This is faster when many objects are requested but typically much slower for a single object.
Not sure if I should change the code so that it doesn't use overpass for less than - say - 5 objects?

comment:16 Changed 4 months ago by skyper

At least, leave overpass as fallback if OSM-Server is not responding/down.

comment:17 Changed 4 months ago by GerdP

Do you mean to use overpass if OSM server produced a timeout while requesting parents for a small number of objects?

comment:18 in reply to:  17 Changed 4 months ago by skyper

Replying to GerdP:

Do you mean to use overpass if OSM server produced a timeout while requesting parents for a small number of objects?

Yes, if you set it to only use OSM server for a small number.

comment:19 Changed 4 months ago by simon04

By default, download.overpass.for-multi-fetch is false and thus the new way of fetching is inactive.

For the example from the ticket description, still two requests are made:

2020-02-11 21:27:12.479 INFO: POST https://overpass-api.de/api/interpreter (25 B) ...
2020-02-11 21:27:12.479 TRACE: BODY: node(288207873);out meta;
2020-02-11 21:27:13.614 INFO: POST https://overpass-api.de/api/interpreter -> HTTP/1.1 200 (976 ms; 362 B)
2020-02-11 21:27:13.619 INFO: POST https://overpass-api.de/api/interpreter (85 B) ...
2020-02-11 21:27:13.619 TRACE: BODY: (node(288207873)->.n;.n;rel(bn);.n;way(bn);)->.pn;((.pn;);node(w);node(r););out meta;
2020-02-11 21:27:14.950 INFO: POST https://overpass-api.de/api/interpreter -> HTTP/1.1 200 (1.2 s; 2.93 kB)

Concerning the implementation, you could simplify genOverpassQuery by using some Java 8 features:

        TreeMap<OsmPrimitiveType, Set<PrimitiveId>> primitivesMap = children.stream()
                .collect(Collectors.groupingBy(PrimitiveId::getType, TreeMap::new, Collectors.toSet()));

The switch cases CLOSEDWAY and MULTIPOLYGON are irrelevant.

comment:20 Changed 4 months ago by GerdP

Yes, there is still a lot to improve. I think we have too many different classes for the download tasks.

reg. Java 8 features: I think many of them are almost unreadable, and this is one of them. I only understand what it does because I know what it should replace.

reg. switch: I think I added them only to calm down SonarLint. I know they are not needed.

comment:21 in reply to:  20 Changed 4 months ago by simon04

Replying to GerdP:

reg. Java 8 features: I think many of them are almost unreadable, and this is one of them.

For me it's the opposite: once getting used to the effect of the various terminal operation of streams, expressions involving streams are much more concise (to read) and better convey the programmer's intent. I think the expression Collectors.groupingBy(PrimitiveId::getType) is a good example for my point.

comment:22 in reply to:  19 Changed 4 months ago by GerdP

Replying to simon04:

By default, download.overpass.for-multi-fetch is false and thus the new way of fetching is inactive.

For the example from the ticket description, still two requests are made:

2020-02-11 21:27:12.479 INFO: POST https://overpass-api.de/api/interpreter (25 B) ...
2020-02-11 21:27:12.479 TRACE: BODY: node(288207873);out meta;
2020-02-11 21:27:13.614 INFO: POST https://overpass-api.de/api/interpreter -> HTTP/1.1 200 (976 ms; 362 B)
2020-02-11 21:27:13.619 INFO: POST https://overpass-api.de/api/interpreter (85 B) ...
2020-02-11 21:27:13.619 TRACE: BODY: (node(288207873)->.n;.n;rel(bn);.n;way(bn);)->.pn;((.pn;);node(w);node(r););out meta;
2020-02-11 21:27:14.950 INFO: POST https://overpass-api.de/api/interpreter -> HTTP/1.1 200 (1.2 s; 2.93 kB)

As a next step I'll move all code to generate overpass queries to static methods in MultiFetchOverpassObjectReader.

Changed 4 months ago by GerdP

Attachment: 12303.2.patch added

work in progress

comment:23 Changed 4 months ago by GerdP

The patch implements the static method, but I don't see yet where to use it.
For the example above the generated query looks like this:

node(288207873)->.n;.n;way(bn)->.wn;.n;rel(bn)->.rn;((.n;.wn;.rn;);node(w););out meta;

Performance isn't great. There are so many ways to get the wanted result and I am not yet sure which one performs best...

Changed 4 months ago by GerdP

Attachment: 12303.3.patch added

comment:24 Changed 4 months ago by GerdP

12303.3.patch generates this which seems faster:

node(288207873)->.n;.n;way(bn)->.wn;.n;rel(bn)->.rn;(.wn;node(w);.rn;);out meta;

The possible query is only logged for now, see comment:23

Changed 4 months ago by GerdP

Attachment: 12303.4.patch added

comment:25 Changed 4 months ago by GerdP

With the new patch a single download is performed when overpass api is enabled.

@Simon04: Please let me know what you think about the new class DownloadFromOverpassTask. I'd rather prefer to reduce the number of DownloadXYZTask classes but I don't fully understand the threading concept.

There is more to do to handle the fact the overpass doesn't return deleted objects.
Try to download deleted node 1212 with the different APIs.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as needinfo The owner will remain simon04.
as The resolution will be set.
to The owner will be changed from simon04 to the specified user.
to The owner will be changed from simon04 to the specified user.
as duplicate The resolution will be set to duplicate.The specified ticket will be cross-referenced with this ticket
The owner will be changed from simon04 to anonymous.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.