Skip to content
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.

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

Add button for complete member download. #5420

Open
slhh opened this issue Oct 20, 2018 · 6 comments
Open

Add button for complete member download. #5420

slhh opened this issue Oct 20, 2018 · 6 comments
Labels
usability An issue with ease-of-use or design

Comments

@slhh
Copy link
Contributor

slhh commented Oct 20, 2018

The "all members" section has recently got buttons to download individual members. This is a nice improvement, but doesn't cover many usecases well, which need downloading all members.

Full member download buttons should be added to

  • the "all members" section headline of incomplete relations.
  • the items of the multiselection list, which are representing incompletely loaded relations.
    This one might be the most important case. E.g. potential polygon operations like splitting or merging require multiple operands, and completely downloaded relations.

To prevent blocking the UI a long time, limit using the full relation API call to reasonable low member counts. For larger relations start downloading in small portions consecutively, and stop downloading when the user clicks elsewhere.

To prevent overloading the graph:
Short term solution: stop downloading when a fixed limit of loaded nodes is reached.
Long term solution: Do not load the elemnts into the graph, but a graph extension data structure.
The graph needs to get a second (sub)set of functions, which do also work with the elements of the graph extension. Some operations have to use these extended functions, but rendering etc. can continue to use the normal graph.

@bhousel
Copy link
Member

bhousel commented Oct 20, 2018

I've been thinking about this.. I'm very interested to get #5180 merged and add some kind of background worker queue to iD. (chatted with @kepta about this recently)

This would let us download relations in the background, especially for the extra-problematic kinds like routes and multipolygons. I agree we can use the /full call, which should be pretty quick. Maybe we should set a rule like "don't use the /full call for a relation with more than 2000 children" (to prevent iD from getting stuck near coastlines or whatever).

My goal really would be to never even need to bother the user with asking them to download relations.

@bhousel bhousel added the considering Not Actionable - still considering if this is something we want label Oct 20, 2018
@slhh
Copy link
Contributor Author

slhh commented Oct 21, 2018

@bhousel

Maybe we should set a rule like "don't use the /full call for a relation with more than 2000 children" (to prevent iD from getting stuck near coastlines or whatever).

We can't limit the number of children including indirect ones, and for the number of direct ones, we do likely need to be much more restrictive. 2000 way members with 2000 nodes each would result in loading 4 million nodes into the graph. Typical numbers would be much smaller, but some nearly worst case situations might exist. The api timeout isn't really protective.

When issuing the call, the relation itself has already been downloaded. Therefore, we can count the number of member nodes, ways, and relations separately in advance. We do not need to care about the (direct) member nodes. Member relations are larger, but unlikely an issue, because the API call doesn't recursively include their members. The number of member ways is what needs to be limited, because they can potentially result in loading a large amount of nodes.

If we can't use the relation/full we should use the following steps:

  1. a multi fetch call for all member nodes together,
  2. for each way member a way/full call
  3. a multi fetch call for all member relations together,
  4. download all members of member relations appling the algorithm recursively

We can't just issue a relation/full call for each member relation to replace steps 3. and 4. because we can't know whether the call would last too long.

@mmd-osm
Copy link
Contributor

mmd-osm commented Oct 21, 2018

Calling way/full for each way member can in fact be much slower than calling relation/full for the whole relation, though it is very difficult to estimate the expected runtime. I used https://www.openstreetmap.org/relation/148838 as an example, which has some 700+ way members:

Download full relation via relation/full

time curl -s --compressed https://api.openstreetmap.org/api/0.6/relation/148838/full > /dev/null

real 0m14,220s
user 0m0,295s
sys 0m0,069s

Download each member way via way/full

export wayids="611062614,220098085,220098083,533290121,533290123,395152584,533290124,533290127,166291105,324955159,333925550,533290115,166291106,29644431,533290116,533290118,196547815,55478303,483345310,322881640,624442977,533290130,533290133,611151625,624442975,370165112,290528824,370165116,42294763,192384007,223218239,192384006,53424221,166291098,599028928,53424218,599028929,29336265,53424216,370169193,53424214,511901179,511901182,511901183,511901180,166291095,511901181,166291094,482121533,307093961,307093960,284681074,307093963,307093962,621334159,59150350,74810365,192380159,307093956,307093959,307093958,307093953,273302470,307093952,376970741,41900530,376970742,307093955,41900529,307093954,59150345,42120461,59150355,59150353,272171936,332019795,440873438,377337314,379524415,34365868,377350621,307093996,75102168,298809966,298809965,166712908,585519427,34365865,34365866,307093994,307093988,307093991,307093990,196818967,42296616,307093987,307094013,307094012,377353676,307094015,307094011,377353675,533290111,482114314,307094005,307094004,307094007,49041750,307094006,193430588,307094001,193430587,307094000,166712914,307094003,377350595,307094002,434810411,580684127,589709539,284676914,511830349,136036299,34365917,136036298,31373837,34365919,193430623,192381099,377350561,379524417,379524416,436704740,432759359,589708496,307093941,307093942,580684128,432330296,307093936,580684129,307093939,539497430,307093938,539497431"

IFS=","
for way in $wayids
do
     curl --compressed -s https://api.openstreetmap.org/api/0.6/way/$way/full > /dev/null
done

real 1m13,926s
user 0m2,673s
sys 0m0,845s

By the way, JOSM seems to use a slightly different approach: multi fetch all member ways, and then lots of multi fetch calls to download all unknown node ids (subject to URL GET parameter size limits).

@slhh
Copy link
Contributor Author

slhh commented Oct 23, 2018

Parent relation of downloaded elements, which haven't already been in the graph, would still be missing anyway.
This would break operations because iD's operations seem to rely on all parents of elements being downloaded.

To get the parents we would need to issue a subsequent (relations for element) API call foreach downloaded element, recursively including new results of this step.
This seems to be a nightmare in view of performance.

We can skip the download of parent relations, and flag downloaded elements in the graph to have incomplete parent relations. iD's operations would need to be modified to respect this flag.

Another approch would be to store the downloaded elements separately from the graph.
Most operations, rendering, etc. can stay unchanged. Only operations which require access to the separately stored elements need to be modified.
For the separately stored elements there wouldn't be the assumption of parent relations being loaded.
The separate storage could be kept simple, no need to put modified elements there, no need for a history, no need for D3 access.
The graph can offer some functions for unified access to the elements which are stored inside or separately.

@1ec5
Copy link
Collaborator

1ec5 commented Apr 30, 2023

As someone who routinely loads large route relations into iD, I’m not terribly concerned about the potential performance hit from pushing the proposed button. The user has opted into it by pressing the download button. It would be more surprising to the user if we automatically download members in the background (#6656).

In fact, you get all the members today when you open iD with a relation parameter (such as by opening iD from a relation’s page on osm.org). I’ve been using that behavior as a workaround for iD’s tendency to scramble relations that loop back on themselves: osm-americana/openstreetmap-americana#811 (comment). If we don’t have a mechanism for preventing a monstrously complex relation from being loaded automatically at launch, then I don’t think we need to block this proposed button on complicated ideas like a separate temporary graph. That said, a chunked download would be interesting to consider.

@tyrasd
Copy link
Member

tyrasd commented Mar 6, 2024

There is now (#10149) a button to download all members of a relation in the list of relations of a selected entity. Here it is in action:

I'm reopening the issue as there are also use cases for a similar button on the members list of a selected relation, which is currently still missing.

@tyrasd tyrasd reopened this Mar 6, 2024
@tyrasd tyrasd added usability An issue with ease-of-use or design and removed considering Not Actionable - still considering if this is something we want labels Mar 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
usability An issue with ease-of-use or design
Projects
None yet
Development

No branches or pull requests

5 participants