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

added WriteAndVerify committing #44

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

@YuJuncen
Copy link
Author

/cc @3pointer @overvenus Would you take a look? Thanks!

Copy link

@lance6716 lance6716 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe also consider client crash / storage response lost scenario.

----

\* MV CONSTANT definitions CLIENTS
const_171964209906974000 ==

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this TLA spec is not heavy, can you add more clients?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I run it with 3 clients and no error reported. Also if someone wants to verify with more clients, he may modify the model constants manually.

I prefer keeping two clients by default because the state transform graph can still be read by a human... Starting from 3 the state transform gets too complex to be read...

PutAndVerify/WriteAndVerify.tla Outdated Show resolved Hide resolved

(* Delete the file when aborted. *)
RollBack(c) == /\ clState[c] = "aborted"
/\ storage[c] /= "none"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this necessary?

Copy link
Author

@YuJuncen YuJuncen Oct 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In fact not. This won't create new states but just adding arrows between states.

However our implmentation includes this, so I left it here.

@YuJuncen
Copy link
Author

@lance6716 I added crashing and RWLock cases to the model, PTAL and thanks~

Copy link

@lance6716 lance6716 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rest lgtm

PutAndVerify/WriteAndVerify.tla Outdated Show resolved Hide resolved
/\ clState' = [ clState EXCEPT ![c] = "committed" ]
/\ UNCHANGED << clFavor >>

BeRO(c) == /\ clState[c] = "begin"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's the actual meaning of BeRO? client can generate "empty-ro" in storage but I don't know why it should exist

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BeRO means become a "read-only" node. Then the algorithm become a implementation of RWLock instead of Mutex.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally, there can be many storage slots contains "written-ro" or exactly one storage slot contains "written". (RWLock)

Signed-off-by: Hillium <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants