belliottsmith commented on code in PR #67: URL: https://github.com/apache/cassandra-accord/pull/67#discussion_r1374967616
########## accord-core/src/test/java/accord/verify/ElleVerifier.java: ########## @@ -0,0 +1,411 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package accord.verify; + +import clojure.java.api.Clojure; +import clojure.lang.ArraySeq; +import clojure.lang.IFn; +import clojure.lang.IMapEntry; +import clojure.lang.IPersistentCollection; +import clojure.lang.IPersistentMap; +import clojure.lang.ISeq; +import clojure.lang.IteratorSeq; +import clojure.lang.Keyword; +import clojure.lang.PersistentArrayMap; +import clojure.lang.PersistentVector; +import clojure.lang.RT; +import com.google.common.base.StandardSystemProperty; +import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Sets; + +import java.util.ArrayList; +import java.util.Iterator; +import java.util.List; +import java.util.RandomAccess; +import java.util.Set; +import java.util.stream.Collectors; +import java.util.stream.IntStream; + +public class ElleVerifier implements Verifier +{ + public static class Support + { + public static boolean allowed() + { + // Elle only works on JDK 11 + int jdkVersion = Integer.parseInt(StandardSystemProperty.JAVA_VERSION.value().split("\\.")[0]); + return !(jdkVersion == 1 /* 1.8 */ || jdkVersion == 8); + } + } + + // In order to build the jepsen history, we need the full history... so must buffer everything + private final List<Event> events = new ArrayList<>(); + + @Override + public Checker witness(int start, int end) + { + List<Action> actions = new ArrayList<>(); + return new Checker() + { + @Override + public void read(int index, int[] seq) + { + actions.add(new Read(index, seq)); + } + + @Override + public void write(int index, int value) + { + actions.add(new Append(index, value)); + } + + @Override + public void close() + { + // When a range read is performed, if the result was no matching keys then history isn't clear. + // Since StrictSerializabilityVerifier uses indexes and not pk values, it is not possible to find expected keys and putting empty result for them... + if (actions.isEmpty()) + return; + // TODO (coverage): pass client id as process? + events.add(new Event(0, Event.Type.ok, end, actions)); + } + }; + } + + @Override + public void close() + { + if (events.isEmpty()) + throw new IllegalArgumentException("No events seen"); + + Object eventHistory = Clj.history.invoke(Event.toClojure(events)); + events.clear(); + PersistentArrayMap result = (PersistentArrayMap) Clj.check.invoke(Clj.elleListAppendOps, eventHistory); + Object isValid = result.get(Keys.valid); + if (isValid == Boolean.TRUE) + return; + if (isValid == Keys.unknown) + { + // Elle couldn't figure out if the history is bad or not... why? + Object anomalyTypes = result.get(Keys.anomalyTypes); + if (anomalyTypes != null) + { + ArraySeq seq = (ArraySeq) anomalyTypes; + if (!seq.isEmpty()) + { + boolean empty = false; + for (Object type : seq) + { + if (type == Keys.emptyTransactionGraph) + { + empty = true; + continue; // nothing to see here + } + throw new AssertionError("Unexpected anomaly type detected: " + type); + } + if (empty) + return; // all good + } + } + } + throw new HistoryViolation(-1, "Violation detected: " + result); + } + + private static abstract class Action extends java.util.AbstractList<Object> implements RandomAccess + { + enum Type + { + append, r; + + final Keyword keyword; + + Type() + { + keyword = RT.keyword(null, name()); + } + } + private final Action.Type type; + private final int key; + private final Object value; + + protected Action(Action.Type type, int key, Object value) + { + this.type = type; + this.key = key; + this.value = value; + } + + @Override + public Object get(int index) + { + switch (index) + { + case 0: return type.keyword; + case 1: return key; + case 2: return value; + default: throw new IndexOutOfBoundsException(); + } + } + + @Override + public int size() + { + return 3; + } + } + + private static class Read extends Action + { + protected Read(int key, int[] seq) + { + // TODO (optimization): rather than vector of boxed int, can we use the interfaces so we can stay primitive array? + super(Type.r, key, PersistentVector.create(IntStream.of(seq).mapToObj(Integer::valueOf).collect(Collectors.toList()))); + } + } + + private static class Append extends Action + { + protected Append(int key, int value) + { + super(Type.append, key, value); + } + } + + private static class Event extends ObjectPersistentMap + { + enum Type + { + ok, fail; // invoke, info Review Comment: The comments linked above suggest invoke isn’t used; I’m just very surprised by this if it’s true, as I can’t see how you would detect some invalid traces. ########## accord-core/src/test/java/accord/verify/ElleVerifier.java: ########## @@ -0,0 +1,411 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package accord.verify; + +import clojure.java.api.Clojure; +import clojure.lang.ArraySeq; +import clojure.lang.IFn; +import clojure.lang.IMapEntry; +import clojure.lang.IPersistentCollection; +import clojure.lang.IPersistentMap; +import clojure.lang.ISeq; +import clojure.lang.IteratorSeq; +import clojure.lang.Keyword; +import clojure.lang.PersistentArrayMap; +import clojure.lang.PersistentVector; +import clojure.lang.RT; +import com.google.common.base.StandardSystemProperty; +import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Sets; + +import java.util.ArrayList; +import java.util.Iterator; +import java.util.List; +import java.util.RandomAccess; +import java.util.Set; +import java.util.stream.Collectors; +import java.util.stream.IntStream; + +public class ElleVerifier implements Verifier +{ + public static class Support + { + public static boolean allowed() + { + // Elle only works on JDK 11 + int jdkVersion = Integer.parseInt(StandardSystemProperty.JAVA_VERSION.value().split("\\.")[0]); + return !(jdkVersion == 1 /* 1.8 */ || jdkVersion == 8); + } + } + + // In order to build the jepsen history, we need the full history... so must buffer everything + private final List<Event> events = new ArrayList<>(); + + @Override + public Checker witness(int start, int end) + { + List<Action> actions = new ArrayList<>(); + return new Checker() + { + @Override + public void read(int index, int[] seq) + { + actions.add(new Read(index, seq)); + } + + @Override + public void write(int index, int value) + { + actions.add(new Append(index, value)); + } + + @Override + public void close() + { + // When a range read is performed, if the result was no matching keys then history isn't clear. + // Since StrictSerializabilityVerifier uses indexes and not pk values, it is not possible to find expected keys and putting empty result for them... + if (actions.isEmpty()) + return; + // TODO (coverage): pass client id as process? + events.add(new Event(0, Event.Type.ok, end, actions)); + } + }; + } + + @Override + public void close() + { + if (events.isEmpty()) + throw new IllegalArgumentException("No events seen"); + + Object eventHistory = Clj.history.invoke(Event.toClojure(events)); + events.clear(); + PersistentArrayMap result = (PersistentArrayMap) Clj.check.invoke(Clj.elleListAppendOps, eventHistory); + Object isValid = result.get(Keys.valid); + if (isValid == Boolean.TRUE) + return; + if (isValid == Keys.unknown) + { + // Elle couldn't figure out if the history is bad or not... why? + Object anomalyTypes = result.get(Keys.anomalyTypes); + if (anomalyTypes != null) + { + ArraySeq seq = (ArraySeq) anomalyTypes; + if (!seq.isEmpty()) + { + boolean empty = false; + for (Object type : seq) + { + if (type == Keys.emptyTransactionGraph) + { + empty = true; + continue; // nothing to see here + } + throw new AssertionError("Unexpected anomaly type detected: " + type); + } + if (empty) + return; // all good + } + } + } + throw new HistoryViolation(-1, "Violation detected: " + result); + } + + private static abstract class Action extends java.util.AbstractList<Object> implements RandomAccess + { + enum Type + { + append, r; + + final Keyword keyword; + + Type() + { + keyword = RT.keyword(null, name()); + } + } + private final Action.Type type; + private final int key; + private final Object value; + + protected Action(Action.Type type, int key, Object value) + { + this.type = type; + this.key = key; + this.value = value; + } + + @Override + public Object get(int index) + { + switch (index) + { + case 0: return type.keyword; + case 1: return key; + case 2: return value; + default: throw new IndexOutOfBoundsException(); + } + } + + @Override + public int size() + { + return 3; + } + } + + private static class Read extends Action + { + protected Read(int key, int[] seq) + { + // TODO (optimization): rather than vector of boxed int, can we use the interfaces so we can stay primitive array? + super(Type.r, key, PersistentVector.create(IntStream.of(seq).mapToObj(Integer::valueOf).collect(Collectors.toList()))); + } + } + + private static class Append extends Action + { + protected Append(int key, int value) + { + super(Type.append, key, value); + } + } + + private static class Event extends ObjectPersistentMap + { + enum Type + { + ok, fail; // invoke, info Review Comment: The comments linked above suggest invoke isn’t used; I’m just very surprised by this if it’s true, as I can’t see how you would detect some invalid traces. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]

