keith-turner commented on code in PR #5214:
URL: https://github.com/apache/accumulo/pull/5214#discussion_r1917416617
##########
test/src/main/java/org/apache/accumulo/test/fate/FateInterleavingIT.java:
##########
@@ -235,38 +243,50 @@ protected void testInterleaving(FateStore<FilTestEnv>
store, ServerContext sctx)
waitFor(store, fateId);
}
- var expectedIds =
- Set.of(fateIds[0].canonical(), fateIds[1].canonical(),
fateIds[2].canonical());
-
Scanner scanner = client.createScanner(FATE_TRACKING_TABLE);
- Iterator<Entry<String,String>> iter =
scanner.stream().map(FateInterleavingIT::toIdStep)
- .filter(e -> e.getValue().contains("::call")).iterator();
-
- SortedMap<String,String> subset = new TreeMap<>();
-
- Iterators.limit(iter, 3).forEachRemaining(e -> subset.put(e.getKey(),
e.getValue()));
-
- // Should see the call() for the first steps of all three fates come
first in time
- assertTrue(subset.values().stream().allMatch(v ->
v.startsWith("FirstOp")));
- assertEquals(expectedIds, subset.keySet());
-
- subset.clear();
-
- Iterators.limit(iter, 3).forEachRemaining(e -> subset.put(e.getKey(),
e.getValue()));
-
- // Should see the call() for the second steps of all three fates come
second in time
- assertTrue(subset.values().stream().allMatch(v ->
v.startsWith("SecondOp")));
- assertEquals(expectedIds, subset.keySet());
-
- subset.clear();
-
- Iterators.limit(iter, 3).forEachRemaining(e -> subset.put(e.getKey(),
e.getValue()));
-
- // Should see the call() for the last steps of all three fates come last
in time
- assertTrue(subset.values().stream().allMatch(v ->
v.startsWith("LastOp")));
- assertEquals(expectedIds, subset.keySet());
+ var iter = scanner.stream().map(FateInterleavingIT::toIdStep).iterator();
+
+ // we should see the following execution order for all fate ids:
+ // FirstOp::isReady1, FirstOp::isReady2, FirstOp::call,
+ // SecondOp::isReady1, SecondOp::isReady2, SecondOp::call,
+ // LastOp::isReady1, LastOp::isReady2, LastOp::call
+ // the first isReady of each op will defer the op to be executed later,
allowing for the FATE
+ // thread to interleave and work on another fate id, but may not always
interleave.
+ // It is unlikely that the FATE will not interleave at least once in a
run, so we will check
+ // for at least one occurrence.
+ int interleaves = 0;
+ int i = 0;
+ Map.Entry<FateId,String> prevOp = null;
+ var expRunOrder = List.of("FirstOp::isReady1", "FirstOp::isReady2",
"FirstOp::call",
+ "SecondOp::isReady1", "SecondOp::isReady2", "SecondOp::call",
"LastOp::isReady1",
+ "LastOp::isReady2", "LastOp::call");
+ var fateIdsToExpRunOrder = Map.of(fateIds[0], new
ArrayList<>(expRunOrder), fateIds[1],
+ new ArrayList<>(expRunOrder), fateIds[2], new
ArrayList<>(expRunOrder));
+
+ while (iter.hasNext()) {
+ var currOp = iter.next();
+ FateId fateId = currOp.getKey();
+ String currStep = currOp.getValue();
+ var expRunOrderFateId = fateIdsToExpRunOrder.get(fateId);
+
+ // An interleave occurred if we do not see <FateIdX, OpN::isReady2>
immediately after
+ // <FateIdX, OpN::isReady1>
+ if (prevOp != null && prevOp.getValue().contains("isReady1")
+ && !currOp.equals(new
AbstractMap.SimpleImmutableEntry<>(prevOp.getKey(),
+ prevOp.getValue().replace('1', '2')))) {
+ interleaves++;
+ }
Review Comment:
Wondering if it would be better to relax this and look for any interleaving
of FateIds in the sequence of operations. This would increment the interleave
counter in more cases.
```suggestion
// is the current operation passed its first step?
boolean passedFirtStep = !currStep.equals(expRunOrder.get(0));
// was the previous op a different fate id?
boolean prevFateIdDiffered = prevOp != null &&
!prevOp.getKey().equals(fateId);
if(passedFirstStep && prevFateIdDiffered){
interleaves++;
}
```
--
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: notifications-unsubscr...@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
