disconnect client if it submits an already accepted changeset based on a...#2091
Conversation
…n an old revision
There was a problem hiding this comment.
btw, you can conveniently just throw the error here, and have the catch block do the work for you... DRYs things up ;)
|
I think disconnecting is a very drastic measure, since it could happen pretty often that two users delete the same character concurrently.. I don't think there is currently any code that deals with updating the client with missed changes. I think this is probably a good way of fixing this behavior. |
|
@marcelklehr did you want to see this in 1.4? |
|
Nah, I think we should deal with this properly. My concern is, that a client with high latency might send the same cs a second time after some timeout. With this fix in place it would get disconnected. |
|
@webzwo0i did you want to submit a fix that doesn't disconnect the user? |
|
yes, please give me some days to test |
|
@marcelklehr are you happy with this? :) |
|
in case two clients edit concurrently and submit the same cs, the first client gets ACCEPT_COMMIT and the second gets NEW_CHANGES. |
|
The question is indeed, what will the client do? I think it will try to resend its changes after rebasing them against new changes. Looks good to me, if your test succeeds then it's good to merge, imo. |
…mitted-twice disconnect client if it submits an already accepted changeset based on a...
3 participants
...n old revision
a changeset that is based on an old revision and contains the same changes that have produced baseRev+1 will satisfy the Changeset.follow checks and a new revision is pushed to all clients, but won't contain any real changes (e.g. if a cs that deletes all default text is submitted twice with baseRev:0, "Z:6c<6b|5-6b$", a new revision 2 that contains "Z:1>0$" will be pushed).
there is no limit how old a baseRev can be compared to the current head revision - which is good, because there cannot be a good approximation for this, as it depends on the number of clients, the size of the pad etc.
but it is bad in the described case as it can be used to insert an endless amount of revisions and the consequence are endless db round trips.
I left a "TODO" notice in the patch, because as I understand it, the client should stop sending changesets, receive all revisions and update its view of the pad to the latest head, and check if the changeset is still okay and resubmit it. but I couldn't find code that somehow does this, so I don't know what really happens, in case the client has missed some NEW_CHANGES.