Page MenuHomePhabricator

Phabricator silently overwrites concurrent changes (no conflict detection)
Open, LowPublic

Assigned To
Authored By
hnesland
Apr 8 2014, 11:52 AM
Referenced Files
None
Tokens
"The World Burns" token, awarded by SvHu."Love" token, awarded by aklapper."The World Burns" token, awarded by jane."The World Burns" token, awarded by revi."The World Burns" token, awarded by nemobis.

Description

When having multiple users editing the same Phriction or Maniphest documents, sometimes one will overwrite some other change, without noticing.

It would be nice to get at least an indication of the edit conflict. It would be super cool if we actually could auto-merge these edits whenever possible.

The changes include not only edits of title and description, also users CCed and projects associated.

Related report: https://phabricator.wikimedia.org/T78236

Related Objects

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
chad triaged this task as Low priority.Apr 29 2014, 2:51 AM
chad added a subscriber: chad.

Seems like a reasonable request.

See T5060 for more thoughts on resolving.

We lost some edits again yesterday because of this issue due to another set of people happening to touch the same page roughly at the same time like in T5060.

Note that for this issue to happen you don't need to have 2 people edit the doc at the exact same time: sometimes people start editing a doc while they are working on something and might do the actual save half an hour later or more. This leaves a big opportunity for another edit to happen in between.

We're trying to transition a cross-functional org to Phabricator and non-engineering people are never going to understand why some of their edits sometimes mysteriously and silently disappeared from the docs.

Seems like a big deficiency of an otherwise very good wiki. It would be really helpful and hopefully easy to implement at least rejection of saves on top of an updated doc. That's frustrating UX but much much better than the current situation. Thanks!

Adding @epriestley - who may have thoughts on at least a quick fix (not lose work, warn of updates).

Just to clarify, you don't actually lose any work, right? This is a surprise/confusion/etc issue, and the document may not end up in the state you expect it to be in, but the work is preserved by the edit history?

We can probably do conflict detection fairly easily, but conflict resolution is more difficult.

The work is not "lost" since it's in the history indeed. It's super annoying to fix things this way though, and it implies you already know some contents has disappeared. In the 2 occurrences that I know of after a few days with Phab at our org with a subset of people, they were discovered randomly, so there might be others.

Conflict detection is the bare minimum indeed. If you can add to the warning message a link to the diff of what has changed in between that's even better :)

Krenair added a subscriber: Krenair.

We ( Wikimedia ) see this problem (lack of conflict detection) often in Maniphest, but it probably applies to more than just these two projects... See https://phabricator.wikimedia.org/T78236

This task I believe is specifically for Phriction, but maybe it is generalizable.

In a general sense, I wonder if we should consider having /edit/ pages also track changes on the object and display notifications when the underlying object changes. Beyond notifications, everything seems really difficult to get correct.

Yeah I wasn't sure whether to generalise this or open a new task. In BZ we'd have to open new bugs, but here we can just add extra projects... :)

qgil renamed this task from Warn/merge if document is edited while editing to Phabricator silently overwrites concurrent changes (no conflict detection).Jan 13 2015, 7:59 AM
qgil updated the task description. (Show Details)
qgil edited projects, added Maniphest; removed Wikimedia.

I have edited the description in order to bring Maniphest in, as discussed above. In Wikimedia we are finding ourselves in this problem too often.

We can probably do conflict detection fairly easily, but conflict resolution is more difficult.

Wondering: If someone wanted to contribute to improving the current situation, which underlying work would need to be done?

I didn't realize this got morphed into something "general".

The Phriction part is done and seems nice.

See my comments on T7340 - we're not doing live update stuff anytime soon. I guess more Phriction-style conflict detection could come down the pipe eventually.

This keeps being a problem for Wikimedia. Situations when the probability of clashes include someone sends an email to wikitech-l (more than 1000 subscribers), one team or more are working on something specific on a specific day, or, like this weekend, we organize a hackathon with 200 developers around.

Conflict detection with the possibility to press "Back" in your browser without losing your data would be absolutely fine.

We could do parts of this prior to T9132, but completing it first gives us much better tools for solving this everywhere, and a realistic chance of building a not-totally-awful resolution UX.

greggrossmeier moved this task from Details to Backlog on the Wikimedia board.

D14390 introduces "ApplicationEditor", which includes more race-resistant controls for editing things like projects and subscribers. In effect, this should merge edits of these fields correctly in all cases where there are no conflicts. Here's an example:

  • Users A and B both start editing an object which has projects: X, Y.
  • User A adds project Z, then saves.
  • User B removes project Y, then saves.

In the old behavior, these would apply like this:

  • A: Set projects = X, Y, Z
  • B: Set projects = X

User B's edit would win, overwriting the state of the object, and user A's edit would be lost.

In the new behavior, these apply like this:

  • A: Add projects: Z.
  • B: Remove projects: Y.

These edits serialize correctly and the object ends up with projects: X, Z (i.e., both edits are accurately reflected).

This won't roll out to all applications for a bit but is probably not too far away.

eadler added a project: Restricted Project.Apr 7 2016, 7:28 AM
eadler moved this task from Restricted Project Column to Restricted Project Column on the Restricted Project board.

I think nowadays the biggest issue (as it is the most work to manually correct afterwards) is two folks editing a task description at the same time and silently overwriting.

Yesterday's downstream example:

  1. After Qgil's task desc edit in https://phabricator.wikimedia.org/T135327#2317469 , I started editing the task desc.
  2. I finished and saved my edit in https://phabricator.wikimedia.org/T135327#2317529
  3. But in the meantime Qgil had made another task desc edit in https://phabricator.wikimedia.org/T135327#2317493
  4. So I had to edit again in https://phabricator.wikimedia.org/T135327#2317535 to include Qgil's changes into my changes.

Other in-the-meantime changes like overwriting a change in the Priority dropdown are obviously easier to correct/revert manually, but also apply:

  1. Go to https://phabricator.wikimedia.org/T119622 (set to high priority) in one browser with account Malyacko, click "Edit Task", set "Priority" dropdown to "Normal", don't click "Save Changes" yet
  2. Go to https://phabricator.wikimedia.org/T119622 in another browser with account AKlapper, click "Edit Task", set "Priority" dropdown to "Low", don't click "Save Changes" yet
  3. As Malyacko, click "Save Changes"
  4. As AKlapper, click "Save Changes"
  5. Currently no warning is shown for AKlapper that the displayed Priority changed in the meantime.
  6. See https://phabricator.wikimedia.org/T119622#2322342

In any case (edit the task desc, changing the Priority dropdown value), to have the last editor become aware that they just overwrote a change that had taken place in the meantime, the last editor needs to look at the last-but-one change above their last change listed in the single task view above the "Weigh In / Take Action" area. Not everybody does.

To keep it simple, I would have appreciated some kind of warning dialog when I saved the task desc similar to the "Edge already exists" etc. dialog. Something like "Note: This task has been edited in the meantime by others. Saving your edit will overwrite other people's edit(s) made in the meantime. If you choose to cancel your edit, make sure to copy your potential text changes to your clipboard to not lose them. [Cancel my edit] [Save my edit]" or such.

I'm obviously not happy about the "use the clipboard" but no better idea how to help people not to lose their stuff, plus we've seen (technically unrelated and not covered in this task) comments about "Phab didn't remember my stuff when I clicked around" like https://phabricator.wikimedia.org/T132850, https://phabricator.wikimedia.org/T113930, https://phabricator.wikimedia.org/T86463 which comes into play regarding overall perception.

Copying in some context from elsewhere:

  • I want to improve this in this in the long run.
  • I don't want to ship a version of this feature which:
    • has a lot of false positives (warnings about conflicts when none exist); or
    • gives users very little information about conflicts (so they can't tell what's wrong).
  • When possible, I want to provide good tools for resolving conflicts (automatic resolution if possible; guidance on assessing and resolving conflicts when not).

Particularly, I don't want to ship this:

This task has been edited while you were editing it. [ Cancel ] [ Save Anyway ]

...because this will create many false positives (where the simultaneous edit does not legitimately conflict) and gives users almost no information to understand the conflict.

Specifically, I'm worried that users will hit this a few times, cancel out, reload the page, see that there aren't really any conflicts, and become conditioned to ignore it, and we'll approximately be no better off than we are today, just with software that is more frustrating to use.

If real-time notifications are configured, we also already pop a similar warning up on the page in the lower left corner ("This page has been updated, click to reload."), it just isn't modal. I think this version of the notification is much better than a modal dialog on submit for general notifications of updates (you learn about potential conflicts faster, users who aren't performing an edit learn about updates, it's not intrusive, it's not misleading, etc).

The minimum version of this feature I'm comfortable shipping must be able to detect actual conflicts all of the time (or, at least, almost all of the time -- there are probably some weird, complicated edge cases) and give users accurate, specific information (e.g., this would appear only if you were also attempting to change the priority):

While you were editing this task, other users made conflicting edits:

alincoln changed priority from Low to High.

You can apply your changes anyway, or revise them before continuing.
[ Revise Changes ] [ Apply Changes ]


On text diffs (descriptions, titles) this also gives users very little help in resolving conflicts, particularly given how poor prose diffs are today. Improved prose diffs (T3353 / T7643) would help a lot, and at least give users a fighting chance to resolve edit conflicts without actively working against them as we do today. Better would be real 3-up conflict diffing (probably 2x 1-up, not 3-column?), inline display of the diff, and automatic merging (or, at least, an option to accept an automatic merge) of non-conflicting edits, and possibly actual conflict resolution/merge tools.


Beyond this, it would be nice to let users pick which edits to discard from the conflict dialog (e.g., revise changes, apply changes, or uncheck some changes in order to discard them, and then apply the rest) so conflicts like priority/status that accompany a more substantive change (like a comment) can be resolved in 1-2 clicks instead of canceling, removing/reverting the action, and submitting again. I don't think this is necessary to ship the feature, though.

eadler moved this task from Restricted Project Column to Restricted Project Column on the Restricted Project board.Jul 4 2016, 9:05 PM