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

Ghost points test still failing in some circumstances #28

Open gmaclennan opened 7 years ago

gmaclennan commented 7 years ago

It seems like the ghost points test still randomly fails: https://travis-ci.org/digidem/osm-p2p-server/builds/191945300#L2390

Notice the 'SKIP' message which is produced by this code: https://github.com/digidem/osm-p2p-server/blob/master/lib/check_ref_ex.js

The code already tries to replicate race condition issues by adding a random delay to database queries, so I would think this bug is more likely related to id or version sorting issues, given how rarely it seems to cause the test to fail.

We really need to track down and fix the bug that required this skip code in the first place.

hackergrrl commented 7 years ago

Thank you very much for noticing this and reporting! I'll spend some time on this and see what I can see.

hackergrrl commented 7 years ago

After reading some code and thinking about this, I think the issue at hand is at a deeper level than a timestamp mis-sort at the osm-p2p-server layer. Consider the following:

The test in question performs two forks on a way (the ORIGINAL WAY): one is a deletion of a point (the DELETED WAY), and the other is a modification of one point id to another (MODIFIED WAY). Since the test failure is reporting that a way returned contains a node that has already been deleted, it means that it's the ORIGINAL WAY that's being returned here, since neither the ORIGINAL WAY nor the DELETED WAY contain a deleted point. This should not be possible: both forks come causally after the ORIGINAL WAY, so there shouldn't be any means for that way to be returned anymore.

The good news is that this relatively small test may provide us with the means to locate the squash the long standing "SKIP bug".