belliottsmith commented on code in PR #50:
URL: https://github.com/apache/cassandra-accord/pull/50#discussion_r1261640182
##########
accord-core/src/main/java/accord/local/SaveStatus.java:
##########
@@ -126,21 +219,68 @@ public static SaveStatus enrich(SaveStatus status, Known
known)
case Accepted:
case AcceptedInvalidate:
case PreCommitted:
-
if (known.isSatisfiedBy(status.known))
return status;
return get(status.status, status.known.merge(known));
+
+ case Truncated:
+ switch (status)
+ {
+ default: throw new AssertionError();
+ case Erased:
+ if (!known.outcome.isOrWasApply() || known.executeAt
!= ExecuteAtKnown)
+ return Erased;
+
+ case TruncatedApply:
+ if (known.outcome != Outcome.Apply)
+ return TruncatedApply;
+
+ case TruncatedApplyWithOutcome:
+ if (known.deps != DepsKnown)
+ return TruncatedApplyWithOutcome;
+
+ case TruncatedApplyWithDeps:
+ if (!known.isDefinitionKnown())
+ return TruncatedApplyWithDeps;
+
+ return Applied;
+ }
}
return status;
}
- public static SaveStatus merge(SaveStatus a, Ballot acceptedA, SaveStatus
b, Ballot acceptedB)
+ public static SaveStatus merge(SaveStatus a, Ballot acceptedA, SaveStatus
b, Ballot acceptedB, boolean preferKnowledge)
Review Comment:
The problem you highlight I don't think is an issue, because Accept doesn't
actually provide much knowledge - just the definition itself. As of today we
don't _logically_ need the definition at all to apply, just the Writes and
Result[1]. So we don't need (e.g.) a TruncatedApplyWithDefinitionOnly.
However, we probably want to optimise Writes to exclude anything that is in
the definition, at which point we may need to tweak these states a little.
[1] This actually is a gap in the implementation I need to address since I
made eviction more aggressive in the most recent patch set (off the back of
formalising how we do eviction for sharing with the researchers).
##########
accord-core/src/main/java/accord/local/SaveStatus.java:
##########
@@ -126,21 +219,68 @@ public static SaveStatus enrich(SaveStatus status, Known
known)
case Accepted:
case AcceptedInvalidate:
case PreCommitted:
-
if (known.isSatisfiedBy(status.known))
return status;
return get(status.status, status.known.merge(known));
+
+ case Truncated:
+ switch (status)
+ {
+ default: throw new AssertionError();
+ case Erased:
+ if (!known.outcome.isOrWasApply() || known.executeAt
!= ExecuteAtKnown)
+ return Erased;
+
+ case TruncatedApply:
+ if (known.outcome != Outcome.Apply)
+ return TruncatedApply;
+
+ case TruncatedApplyWithOutcome:
+ if (known.deps != DepsKnown)
+ return TruncatedApplyWithOutcome;
+
+ case TruncatedApplyWithDeps:
+ if (!known.isDefinitionKnown())
+ return TruncatedApplyWithDeps;
+
+ return Applied;
+ }
}
return status;
}
- public static SaveStatus merge(SaveStatus a, Ballot acceptedA, SaveStatus
b, Ballot acceptedB)
+ public static SaveStatus merge(SaveStatus a, Ballot acceptedA, SaveStatus
b, Ballot acceptedB, boolean preferKnowledge)
Review Comment:
The _specific_ problem you highlight I don't think is an issue, because
Accept doesn't actually provide much knowledge - just the definition itself. As
of today we don't _logically_ need the definition at all to apply, just the
Writes and Result[1]. So we don't need (e.g.) a
TruncatedApplyWithDefinitionOnly.
However, we probably want to optimise Writes to exclude anything that is in
the definition, at which point we may need to tweak these states a little.
[1] This actually is a gap in the implementation I need to address since I
made eviction more aggressive in the most recent patch set (off the back of
formalising how we do eviction for sharing with the researchers).
--
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]
