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

Check refinement of high-level spec abs with all models #6509

Merged
merged 11 commits into from
Oct 2, 2024

Conversation

lemmy
Copy link
Contributor

@lemmy lemmy commented Sep 27, 2024

Replaces #6493 and depends on #6508.

@lemmy lemmy added the tla TLA+ specifications label Sep 27, 2024
@lemmy lemmy marked this pull request as ready for review September 30, 2024 04:45
@lemmy lemmy requested review from a team and achamayou September 30, 2024 04:45
@lemmy lemmy added the run-long-verification Run Long Verification jobs label Sep 30, 2024
@lemmy lemmy force-pushed the mku-refine branch 2 times, most recently from d5e8fbe to 6963930 Compare October 1, 2024 13:59
lemmy and others added 8 commits October 1, 2024 07:49
with MaxUncommittedCount (see max_uncommitted_tx_count in raft.h)
that restricts the length by which a log may be extended in each step.

Signed-off-by: Markus Alexander Kuppe <github.com@lemmster.de>
(Maintains the checked state space)

Signed-off-by: Markus Alexander Kuppe <github.com@lemmster.de>
Clearly, refinement mapping belongs in ccfraft, but we couldn't do
that up until recently because of a TLC limitation that was resolved
by tlaplus/tlaplus#441.

Depends on TLA+ PR tlaplus/tlaplus#1014

Signed-off-by: Markus Alexander Kuppe <github.com@lemmster.de>
Add node may copy the longest log from some other node and extend the copied log in one step.

Background: microsoft#6493 (comment)

Co-authored-by: Amaury Chamayou <amchamay@microsoft.com>
Signed-off-by: Markus Alexander Kuppe <github.com@lemmster.de>
…ion, simulation).

Signed-off-by: Markus Alexander Kuppe <github.com@lemmster.de>
…fixes

by expressing abs!Extend and abs!CopyMaxAndExtend axiomatically.

Signed-off-by: Markus Alexander Kuppe <github.com@lemmster.de>
MUC does not apply to some log entries, i.e, signatures, ...

Signed-off-by: Markus Alexander Kuppe <github.com@lemmster.de>
Signed-off-by: Markus Alexander Kuppe <github.com@lemmster.de>
@achamayou
Copy link
Member

Unless I am missing something, this change is adding very substantially to the runtime of long model checks:

Before: https://github.com/microsoft/CCF/actions/runs/11107110210/job/30856948052
image
image

After: https://github.com/microsoft/CCF/actions/runs/11127867170/job/30921202022?pr=6509
image
image

These are already by far our longest running jobs, my fervent hope is that we find ways to speed them up so we can relax the state space restrictions, and this seems to be taking us in the wrong direction.

@lemmy
Copy link
Contributor Author

lemmy commented Oct 2, 2024

Unless I am missing something, this change is adding very substantially to the runtime of long model checks:

[...]

These are already by far our longest running jobs, my fervent hope is that we find ways to speed them up so we can relax the state space restrictions, and this seems to be taking us in the wrong direction.

This doesn't seem directly related to this pull request. There's nothing preventing us from relaxing the state-space restrictions and skipping the refinement checks in those models.

@achamayou
Copy link
Member

This doesn't seem directly related to this pull request. There's nothing preventing us from relaxing the state-space restrictions and skipping the refinement checks in those models.

Agreed, but there remains an open question about the value of the abstract model, and of the refinement. We are already severely constrained in the space we can check, adding the refinement makes that substantially worse. I am not sure what the utility of having it and not checking would be.

@lemmy lemmy added this pull request to the merge queue Oct 2, 2024
Merged via the queue into microsoft:main with commit 76b2dcf Oct 2, 2024
16 of 19 checks passed
@lemmy lemmy deleted the mku-refine branch October 2, 2024 15:53
@lemmy
Copy link
Contributor Author

lemmy commented Oct 2, 2024

This was merged despite the failure of two long-running jobs. The last successful checks were completed with commit bbd4a4f. The causes seems to be that StartTerm..MaxTermLimit+1 was changed to StartTerm..StartTerm + TermCount, which is not equivalent (+1).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
run-long-verification Run Long Verification jobs tla TLA+ specifications
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants