[OSM-dev] Potlatch causing DB inconsistency?

[Richard Fairhurst]

Stefan Baebler wrote:

> - potlatch decides to delete old way and create a new one for some
> reason [stupid (breaking history), but legal]

Er, no. Potlatch never "decides" of its own accord to delete an old  
way and create a new one. You look at the source and tell me where it  
does that.

> - potlatch deletes old way and all the nodes it can (those that aren't
> used by other ways where other rivers join the riverbank) [stupid, but
> ok]

Er, no. That's not stupid, that's what deleting a way does.

> - potlatch creates a new way, consisting of old (now mostly deleted)
> nodes [illegal]

Er, no. When Potlatch _created_ way 25397686 the nodes were live.  
Nothing illegal there.

> Not really sure this has anything to do with missing transactions,
> more likely some bad logic.

Right, so try actually thinking about it rather than simply sounding  
off with the first assumptions that spring to mind, and throwing  
pejorative words around as if you know instantly where the blame lies  
and whatever muppet wrote Potlatch is clearly a moron not to have  
thought of it.

http://www.openstreetmap.org/api/0.5/way/25397686/history shows that  
the first version of the way had 2 nodes, the second had 155 nodes.

The extra 153 nodes were old nodes. So it looks like the user was  
merging ways.

Because we don't have a server-side merge operation, a merge takes  
place by appending the nodes from way B to way A, saving way A, then  
deleting way B. Two separate operations. (There is no way to do this  
without "breaking history".)

Saving way A will set all its constituent nodes to visible=true  
(controllers/amf_controller.rb#L334). Deleting way B will not set  
nodes used in other ways (i.e. including A) to visible=false; only  
those that were used in this way only (models/way.rb#L237). If you can  
prove _how_ this is "bad logic" or point to an error in the code then  
that would actually be helpful.

But if these operations execute at the same time, without  
transactions, you are going to get inconsistencies exactly like this.  
Potlatch tries its best to make sure that they don't: if you look at  
http://trac.openstreetmap.org/browser/applications/editors/potlatch/way.as#L264 , you'll see that the call to deleteMergedWays only happens after the new way has been successfully  
saved.

Now my guess is that this is where something has gone askew.  
Experience also suggests that this is more likely to happen at a time  
of high server load.

So if I were to make a random stab in the dark, I would guess that the  
user was merging ways hither and thither; that the first "save"  
operation (putway) took a long time to execute due to high load; and  
that somehow, perhaps due to Potlatch incorrectly chaining the list of  
ways to delete when ways are merged, a subsequent "delete" operation  
(deleteway) was sent while putway was still executing over some of the  
same nodes.

Like I said, amf_controller has been so thoroughly rewritten for 0.10  
that there is little point raising bugs against 0.9c now. If you can  
find steps to reproduce against 0.10, that starts to be helpful.

> But why is potlatch today STILL showing the riverbank as it would be  
> there, but in fact it isn't?

http://trac.openstreetmap.org/browser/sites/rails_port/app/controllers/amf_controller.rb#L160

There ain't nothing inherently flawed about that code (and I didn't  
even write it so can say that with impunity). I guess  
way.nodes.collect isn't fussy about node.visible. To be frank I'm not  
convinced that it's a bad thing: assuming that we can fix whatever's  
causing the inconsistency in the first place, then exposing such nodes  
means they'll be invisibly fixed over time as people edit and reupload  
the ways.

Incidentally, Lester Caine HAS the trademark round here on RANDOM use  
of capitalised words in the MIDDLE of sentences.

cheers
Richard