search

digidem / osm-p2p-server

Peer-to-peer OpenStreetMap API v0.6 Server for osm-p2p-db
BSD 2-Clause "Simplified" License
87 stars 10 forks source link

Splitting a way and deleting half corrupts DB #22

Closed gmaclennan closed 8 years ago

gmaclennan commented 8 years ago

If you create a new way, and commit the changes, then modify the way to remove some of the nodes, deleting the now unused nodes, the data does not actually get deleted from the db.

I have created a failing test: https://github.com/digidem/osm-p2p-server/blob/master/test/split_delete_way.js

There seem to be two parts to this error: one, deleted nodes are not being deleted from the db, despite not being used in any way (if they are not used, they should be deleted).

It also seems like there is an error upstream in the indexing. Querying the way directly only returns one version (i.e. no forks) but the bbox request returns both the original version and the modified version - i.e. the bbox query (built from the index) has two versions, but it seems like there is only one fork.

This problem I think is at a root of a lot of problems we have had with the Waorani database.

This is the xml returned by the bbox query after running the test. Directly querying way id 12059823620099955508 returns only one version, but the bbox has two.

<?xml version="1.0" encoding="UTF-8"?>
<osm version="0.6" generator="osm-p2p">
  <bounds minlat="63" maxlat="66" minlon="-123" maxlon="-120"/>
  <node lat="64" lon="-121" changeset="9674494680445552958" id="9457674000928778467" version="0574666a6885b2e0943b7091ad08609e59cd878ffdf53a844608b2e7766324c1"></node>
  <way changeset="9097047276795674746" id="12059823620099955508" version="c53254a2b09d53d212aa6fa6cc64cac51332176a4eca5e04b18e82f33a34e7a3">
    <nd ref="9457674000928778467"/>
    <nd ref="4109479314793594201"/>
    <nd ref="16646991785683865867"/>
  </way>
  <node lat="64.1" lon="-121.1" changeset="9674494680445552958" id="4109479314793594201" version="b88061ffb5a5c1aa22816395afaf0606d29d0cd27b7f40861475297ed4c08d1c"></node>
  <node lat="64.2" lon="-121.2" changeset="9674494680445552958" id="16646991785683865867" version="47b43b5109ab971d3fd5771857de5f1d4e59b0b44d41f43dc6c08846d9d63df7"></node>
  <node lat="64.3" lon="-121.3" changeset="9674494680445552958" id="9066630715587242710" version="901f3032bad894e8de0024a4fd126f9967bce91fc0285325fc926f57edb47494"></node>
  <way changeset="9674494680445552958" id="12059823620099955508" version="e27389ef42c51af2516019d94a82c5ed41e6bfd337616bb66d735d81afdd5b78">
    <nd ref="9457674000928778467"/>
    <nd ref="4109479314793594201"/>
    <nd ref="16646991785683865867"/>
    <nd ref="9066630715587242710"/>
    <nd ref="10834996833648773424"/>
  </way>
  <node lat="64.4" lon="-121.4" changeset="9674494680445552958" id="10834996833648773424" version="8edee4a4ad2686d6b2e5dc652beddccbe03ff7b765db16f6ad83f68c9afa2143"></node>
</osm>
ghost commented 8 years ago

Fixed in 1.12.1. The problem was that the if-unsued calculation in lib/del.js did not take into consideration if a way or relation was modified in the current batch, which may change if a node is unused.

gmaclennan commented 8 years ago

@substack, just to be clear here that the code is matching the spec: basically the DB should never delete a node/way/relation that is used in another way/relation. Normally it would throw an error if you tried, but what if-unused does it tell the API to 'silently fail' when you try to delete something that is used. I haven't looked through this part of the code but I wanted to make sure this is what we are doing.

ghost commented 8 years ago

The DB will throw an error if you try to delete something that hasn't been used and if-unused isn't set.