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

Add check for mismatch of operations in history #10

Merged
merged 1 commit into from
Jun 28, 2021
Merged

Add check for mismatch of operations in history #10

merged 1 commit into from
Jun 28, 2021

Conversation

aquarhead
Copy link

@aquarhead aquarhead commented Apr 15, 2021

This is intended to fix jepsen-io/maelstrom#14

I can see the result by injecting the op-mismatch as :internal anomalies, which will (cause maelstrom to) output like this if I simply use append for all µ-op functions:

 :workload {:valid? false,
            :anomaly-types (:internal),
            :anomalies {:internal (([[:r 6 nil] [:append 6 nil]])
                                   ([[:r 9 nil] [:append 9 [1]]]
                                    [[:r 9 nil] [:append 9 [1]]])
                                   ([[:r 9 nil] [:append 9 nil]]
                                    [[:r 8 nil] [:append 8 nil]])
                                   ([[:r 8 nil] [:append 8 nil]]))},

I still need to find a place to let the checker realize the :op-mismatch anomalies though, it seems I would need to add it somewhere in consistency_model.clj? And at this point I'm not sure if this should be considered as a "consistency model", or which one (I'm interested in this field but I'm not fluent in the terminologies). Could you point out where I should add it?

Also this is my first time contributing to Clojure projects (actually first time working on any Clojure projects), so kindly inform me if there're better ways to approach things. There're also probably some excessive comments, let me know if they should be removed.

And obviously the error message/return should be improved.

@aphyr
Copy link
Contributor

aphyr commented Apr 20, 2021

Hey, thanks for this! I think you're on the right track here, but Elle's checkers are complicated and I'm gonna need to look at this carefully (and get tests in place) before merging. Might be a little bit--I've got a bunch of unfinished Elle work in progress that I gotta button up first, and that's waiting on me getting time off from client work.

@aphyr aphyr merged commit 1eb532c into jepsen-io:master Jun 28, 2021
@aphyr
Copy link
Contributor

aphyr commented Jun 28, 2021

So this breaks... 28 different tests, haha--mostly because we were relying on not needing to generate completely well-formed histories in the test code. Not a huge deal, but it does create a lot of work for me. In the future, I'd appreciate it if you could run the tests first!

@aquarhead
Copy link
Author

Sorry about that xD

I'm completely new to Clojure projects but will keep this in mind in the future :)

@aphyr
Copy link
Contributor

aphyr commented Jun 28, 2021

Errrgh, okay, this is gonna take me several hours to fix, and I'm on the clock for a client. I'm gonna revert this for now, but you're welcome to take a pass at fixing the tests yourself!

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.

txn-list-append workload fails to detect invalid return for read operations
2 participants