-
Notifications
You must be signed in to change notification settings - Fork 20
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
Journal with command store #102
base: trunk
Are you sure you want to change the base?
Conversation
@@ -666,6 +672,11 @@ public final boolean isTruncated() | |||
return status().hasBeen(Status.Truncated); | |||
} | |||
|
|||
public static boolean isTruncated(Status status) | |||
{ | |||
return status.hasBeen(Status.Truncated); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Truncated (Cleanup, Maybe, DefinitionErased, ExecuteAtErased, DepsErased, Outcome.Erased),
Invalidated (Persist, Maybe, NoOp, NoExecuteAt, NoDeps, Outcome.Invalidated),
wouldn't that include Invalidated
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So I have simply copied the code from above; my intention was to make non-static version use the static one as I did with isStable
. You are right isTruncated
is included here.
@@ -1240,7 +1251,7 @@ public boolean equals(Object o) | |||
if (!super.equals(o)) return false; | |||
Executed executed = (Executed) o; | |||
return Objects.equals(writes, executed.writes) | |||
&& Objects.equals(result, executed.result); | |||
&& Objects.equals(result, executed.result); // TODO: find a different way to check/assert? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why this todo? what is the issue with the check?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO
should be //TODO (classification 1, classification 2, etc.): msg
This is so we can find them easier and find the why and when to fix issues.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the main classifications are priority ones, and should go first:
desired
expected
required
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reverted this; this was a tmp TODO.
@@ -558,6 +558,9 @@ protected void start(BiConsumer<? super R, Throwable> callback) | |||
|
|||
public void reply(Id replyingToNode, ReplyContext replyContext, Reply send, Throwable failure) | |||
{ | |||
if (replyingToNode == Id.NONE) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when is this case in the protocol?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This used to be the case during replay. But you are right, maybe with my latest changes this will not be the case anymore.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed now.
c87c1f8
to
432d3ce
Compare
432d3ce
to
5f794c1
Compare
6a9482b
to
4a8566a
Compare
1f18dbd
to
70e61b4
Compare
Accord counterpart of apache/cassandra#3409