Copies of ways across changesets with iD 2.10.0

[tdedecko]

I'm noticing a significant increase in duplicate ways created using iD 2.10.0. It looks like multiple changesets are created with copies of new added ways. This creates multiple of the same way overlapping in OSM.

A few examples:

Bamoa Region changes have copies of the same ways.
https://www.openstreetmap.org/changeset/61262385
https://www.openstreetmap.org/changeset/61261588
https://www.openstreetmap.org/changeset/61260828

Springfield Missouri Region changes have copies of the same ways.
https://www.openstreetmap.org/changeset/61273972
https://www.openstreetmap.org/changeset/61272346

This could possibly be a user error but it looks exceptional enough to report this and see if this can be corrected.

[bhousel]

Thanks for reporting @tdedecko - this is a concerning issue and I'd like to dig in a bit more today to see if it's still happening.

Relevant code for uploading changesets is here:
https://github.com/openstreetmap/iD/blob/master/modules/services/osm.js#L544-L612

I don't see anything really obviously wrong with it.

I know of 2 possible situations where iD might duplicate a user's changes:

server error from post

This can happen if the POST /api/0.6/changeset/#id/upload returns an error but the OSM server applies the changes anyway. This is outside our control, and most likely to occur on a bad internet connection. It's probably not what happened in the examples you linked to above, because it looks like the user did other changes on top of the duplicated ones.

user closes iD tab while saving

This is what I think happened in this situation above. I have heard that it is possible for duplicates to occur if the user:

1. clicks save
2. closes the browser window without waiting for the save to finish (the POST is sent and the OSM server applies the changes)
3. then the user chooses to "restore their changes" the next time they open iD

Again this is really hard to control. Could you reach out to this one user and see if that's what they did?

I can probably put in some additional check to remove the local changes from browser storage, if the window is being unloaded and an open changeset POST is inflight.

[mmd-osm]

All changesets in question are fairly large, the upload took at least 30 seconds to complete, in case of cs 61261588 even 13 minutes(!).

At this time there's no way to cancel an upload process (#4717). If the user loses patience after 10s and hits escape (or reloads the page?), it would be entirely possible to upload the same stuff again, while the server happily continues with the previous upload.

[bhousel]

All changesets in question are fairly large, the upload took at least 30 seconds to complete, in case of cs 61261588 even 13 minutes(!).

Hey @mmd-osm was that 13 minute upload a server issue? I'm curious how did you determine how long it took? That seems pretty unusual.

I can definitely imagine the user closing their browser assuming that it was broken, then refreshing iD and trying to save again later.

[mmd-osm]

It's in the changeset metadata: https://api.openstreetmap.org/api/0.6/changeset/61261588

created_at="2018-08-01T12:14:43Z"
closed_at="2018-08-01T13:27:54Z"

iD always follows the "create new changeset", "upload changes", "close changeset" pattern. Once you create a new changeset, it will be automatically closed 1 hour later. During "upload changes", the server will update that "closed_at" with every object being processed.

So the last change for this changeset was processed at 2018-08-01T12:27:54Z, exactly 1 hour before it got automatically closed. (In theory, the upload could also have taken 1 hour and 13 minutes, and the changeset close was triggered by iD. This scenario seemed extremely unlikely to me, though.)

One reason for such extremely long processing times could be some other huge changeset being processed on the server at the same time, which happened to hold some database locks for the same objects as in this changeset (=database lock wait situation).

I did some analysis on this topic, see openstreetmap/openstreetmap-website#1710 (comment) - there were even more extreme cases than 13 minutes.

[bhousel]

I did this: "I can probably put in some additional check to remove the local changes from browser storage, if the window is being unloaded and an open changeset POST is inflight."

[mmd-osm]

I don’t think that’s a good idea. The POST is not guaranteed to eventually succeed, there may be still a version conflict even very late during processing and the whole transaction is rolled back. If you also remove the local storage this will result in data loss!

At this time there’s really no clean way of preventing such duplicates. You can’t just check if something similar already exists. It may still be part of an ongoing transaction and not yet committed to the database.

Only with the changeset upload redesign the time window will become significantly shorter.

[bhousel]

If you also remove the local storage this will result in data loss!

This only happens if the user gives up and closes their iD window after sending the changeset. I'm ok with this.

[mmd-osm]

I’m a bit confused about the comment ‘the user can be prompted to restore those changes the next time they start iD’. If that’s still possible then this change should be ok.

‘Removing changes from the browser storage’ reads more like all the data would be gone, though.

[bhousel]

I’m a bit confused about the comment ‘the user can be prompted to restore those changes the next time they start iD’. If that’s still possible then this change should be ok.

It's not still possible, I hope.

‘Removing changes from the browser storage’ reads more like all the data would be gone, though.

Yes, all the data will be gone from the user's storage. Because presumably that data has already been delivered to the OSM API in a changeset. If there is data loss here, it's the API's fault, not iD's.

[mmd-osm]

Only in case you receive a http 200 ok response from the changeset upload call, you can be sure that the data has in fact been committed to the database. If that’s not the case, anything might happen, like a rollback due to conflicting versions (which the server cannot resolve on its own). Sending data to the OSM API won’t be sufficient in this case to decide if local data can be discarded. The client is really responsible for error handling.

Which brings us back to the initial point: at this time there’s really no clean way of preventing such duplicates. If the connection fails intermittently, or the user loses patience, you just don't know what the status on the db is - the change might have been successfully written, there might be some error (e.g. conflict), or it might even be still 'in flight' for a large changeset.

[mmd-osm]

Ok, here's a bit of a more systematic approach. It depends on an upstream change, but would allow for a much better issue analysis. Deleting local storage without further details is just not helpful, after all.

• Before uploading a changeset to the OSM API, keep changeset number in local storage

• Let's assume the upload gets interrupted (network dies, user reloads).

• Upon reloading iD, we still know the changeset number, as it has been stored in the local storage.

• Now, let's take a look at the changeset metadata for the changeset in question, e.g. https://api.openstreetmap.org/api/0.6/changeset/61261588

In the future, this call will also return the number of changes contained in the changeset (see zerebubuth/openstreetmap-cgimap#156).

• Based on this information, the following cases are possible:

Decision Matrix	changes_count == 0	changes_count > 0
changeset closed	Previous upload failed, keep local storage	Previous upload succeeded, clear local storage
changeset open	Unclear, might have failed, or still in flight, keep local storage, wait some time... until changeset gets closed, or changes_count > 0	Previous upload succeeded, clear local storage

The bottom left case is the nasty one, as it would require some extensive wait time for a user.