Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Vector time does not get reset correctly when deleting a file #223

Open
tobous opened this issue Aug 29, 2018 · 4 comments

Comments

Projects
None yet
2 participants
@tobous
Copy link
Member

commented Aug 29, 2018

Saros does not seem to reset the vector time correctly when a file is deleted.
This leads to issues when a file is deleted and a new file with the same name is subsequently created. When this new file is edited by one of the participants, the vector time between the clients de-synchronizes, resulting in errors on the host side and edits for the created file not being shared.

ERROR 14:47:00,826 [DispatchContext] (ConcurrentDocumentServer.java:197) failed to transform checksum activity: ChecksumActivity(path: SPath [project=P/alp (EclipseProjectImpl), projectRelativePath=src/test/Created.java], hash: 743766400, length: 41, jupiterTimestamp: [4,2])
de.fu_berlin.inf.dpp.concurrent.jupiter.TransformationException: precondition #2 violated (Remote vector time (2) is greater than local vector time (0)).
	at de.fu_berlin.inf.dpp.concurrent.jupiter.internal.Jupiter.isCurrent(Jupiter.java:147)
	at de.fu_berlin.inf.dpp.concurrent.jupiter.internal.JupiterDocumentServer.withTimestamp(JupiterDocumentServer.java:131)
	at de.fu_berlin.inf.dpp.concurrent.management.JupiterServer.withTimestamp(JupiterServer.java:114)
	at de.fu_berlin.inf.dpp.concurrent.management.ConcurrentDocumentServer.withTimestamp(ConcurrentDocumentServer.java:195)
	at de.fu_berlin.inf.dpp.concurrent.management.ConcurrentDocumentServer.transformIncoming(ConcurrentDocumentServer.java:128)
	at de.fu_berlin.inf.dpp.session.internal.ActivityHandler.directServerActivities(ActivityHandler.java:422)
	at de.fu_berlin.inf.dpp.session.internal.ActivityHandler.handleIncomingActivities(ActivityHandler.java:144)
	at de.fu_berlin.inf.dpp.session.internal.SarosSession.exec(SarosSession.java:734)
	at de.fu_berlin.inf.dpp.session.internal.ActivitySequencer$3.run(ActivitySequencer.java:379)
	at de.fu_berlin.inf.dpp.util.ThreadUtils$1.run(ThreadUtils.java:38)
ERROR 14:47:01,638 [main] (ConcurrentDocumentClient.java:173) Error during transformation of: JupiterActivity(timestamp: [0,0], operation: Insert(38,'\t',38), source: intellij_test_2@saros-con.imp.fu-berlin.de/Saros)
de.fu_berlin.inf.dpp.concurrent.jupiter.TransformationException: Precondition #3 violated (Vector time does not match): [0,0] , [4,2]
	at de.fu_berlin.inf.dpp.concurrent.jupiter.internal.Jupiter.checkPreconditions(Jupiter.java:301)
	at de.fu_berlin.inf.dpp.concurrent.jupiter.internal.Jupiter.receiveJupiterActivity(Jupiter.java:174)
	at de.fu_berlin.inf.dpp.concurrent.management.JupiterClient.receive(JupiterClient.java:48)
	at de.fu_berlin.inf.dpp.concurrent.management.ConcurrentDocumentClient.receiveActivity(ConcurrentDocumentClient.java:171)
	at de.fu_berlin.inf.dpp.concurrent.management.ConcurrentDocumentClient.transformFromJupiter(ConcurrentDocumentClient.java:118)
	at de.fu_berlin.inf.dpp.session.internal.ActivityHandler$3.run(ActivityHandler.java:365)
	at de.fu_berlin.inf.dpp.util.ThreadUtils$1.run(ThreadUtils.java:38)

Steps to Reproduce

  1. Alice and Bob start a session with a file A.
  2. Alice deletes file A.
  3. Alice creates a new file A (with the same name).
@srossbach

This comment has been minimized.

Copy link
Contributor

commented Sep 25, 2018

Interesting comment found here:

https://github.com/saros-project/saros/blob/master/de.fu_berlin.inf.dpp.core/src/de/fu_berlin/inf/dpp/concurrent/management/ConcurrentDocumentClient.java#L189

So yes you are right, it is only called during a file recovery activity but deletes are ignored.

So the question is:

Should this explicitly called by a component that detects the file deletion, or should the ActivityHandler or the ConcurrentDocumentClient (invoked by the ActivityHandler) handle this when they see a FileSystemModification activity ?

I guess file moves are also not handled correctly.

@tobous

This comment has been minimized.

Copy link
Member Author

commented Sep 26, 2018

Yes, moves are not handled correctly as well. I did not mention it because, even though I also noticed the problem for moved files, I did not explicitly test for it.

I am torn on where to put the reset.

Doing it in the component that detects the deletion/move would be preferable from a performance standpoint as it would not add additional checks for every dispatched/received activity. This overhead is especially annoying since file deletions/moves are most likely only a small minority of send activities.

But this would also means that the reset would have to be done in the IDE specific part of the implementation.

A solution in the core (in the ActivityHandler or the ConcurrentDocumentClient) would be better from this point of view, as it would avoid a potential source of bugs for future implementations/changes to the IDE specific logic.

I think I would still prefer doing it in the detection logic and adding it to some kind of core javadoc to avoid/minimize errors of omission for future changes/implementations.

@tobous

This comment has been minimized.

Copy link
Member Author

commented Sep 30, 2018

@srossbach Is just calling ConcurrentDocumentClient.reset(SPath) on the host and on the client side enough or do I have to add another call. I find the javadoc of reset(SPath) a bit confusing. Isn't the ConcurrentDocumentServer only preset on the host? How should the clients call reset (as mentioned in the javadoc)?

I am only asking as I am still having VectorTime problems even though I am resetting the document clients. I will also do some more testing, but since you seem to have a better understanding of the consistency stuff I also wanted to ask you.

@srossbach

This comment has been minimized.

Copy link
Contributor

commented Oct 1, 2018

The server part runs in a different thread than the client part, and IIRC Jupiter documents are "created" on the fly -> if you reset a file and receive a change again the document is present again.

The Jupiter stuff is not really my part of the whole application, it is a big bag box for me. I do not even know how the algorithm work in detail.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.