This is an automated email from the ASF dual-hosted git repository.
ifesdjeen pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/cassandra-accord.git
The following commit(s) were added to refs/heads/trunk by this push:
new 92013a8b Fix null field accounting
92013a8b is described below
commit 92013a8b5007226e899e1a82184a34abe65c98cb
Author: Alex Petrov <[email protected]>
AuthorDate: Wed Feb 5 20:34:16 2025 +0100
Fix null field accounting
Patch by Alex Petrov; reviewed by Benedict Elliott Smith for CASSANDRA-20297
---
.../src/main/java/accord/impl/CommandChange.java | 100 +++++++++------------
.../java/accord/impl/basic/InMemoryJournal.java | 69 +++-----------
2 files changed, 55 insertions(+), 114 deletions(-)
diff --git a/accord-core/src/main/java/accord/impl/CommandChange.java
b/accord-core/src/main/java/accord/impl/CommandChange.java
index 0b7115b0..28fb3b23 100644
--- a/accord-core/src/main/java/accord/impl/CommandChange.java
+++ b/accord-core/src/main/java/accord/impl/CommandChange.java
@@ -20,6 +20,7 @@ package accord.impl;
import java.util.function.BiPredicate;
import java.util.function.Function;
+import java.util.function.Predicate;
import java.util.function.ToLongFunction;
import com.google.common.annotations.VisibleForTesting;
@@ -52,13 +53,13 @@ import static accord.impl.CommandChange.Field.DURABILITY;
import static accord.impl.CommandChange.Field.EXECUTES_AT_LEAST;
import static accord.impl.CommandChange.Field.EXECUTE_AT;
import static accord.impl.CommandChange.Field.FIELDS;
+import static accord.impl.CommandChange.Field.MIN_UNIQUE_HLC;
import static accord.impl.CommandChange.Field.PARTIAL_DEPS;
import static accord.impl.CommandChange.Field.PARTIAL_TXN;
import static accord.impl.CommandChange.Field.PARTICIPANTS;
import static accord.impl.CommandChange.Field.PROMISED;
import static accord.impl.CommandChange.Field.RESULT;
import static accord.impl.CommandChange.Field.SAVE_STATUS;
-import static accord.impl.CommandChange.Field.MIN_UNIQUE_HLC;
import static accord.impl.CommandChange.Field.WAITING_ON;
import static accord.impl.CommandChange.Field.WRITES;
import static accord.local.Cleanup.NO;
@@ -74,7 +75,11 @@ import static accord.local.Command.Truncated.erased;
import static accord.local.Command.Truncated.invalidated;
import static accord.local.Command.Truncated.vestigial;
import static accord.local.StoreParticipants.Filter.LOAD;
+import static accord.primitives.Known.Definition.DefinitionErased;
+import static accord.primitives.Known.KnownDeps.DepsErased;
import static accord.primitives.Known.KnownExecuteAt.ApplyAtKnown;
+import static accord.primitives.Known.KnownExecuteAt.ExecuteAtErased;
+import static accord.primitives.Known.Outcome.WasApply;
import static accord.primitives.Status.Durability.NotDurable;
public class CommandChange
@@ -101,6 +106,37 @@ public class CommandChange
public static final Field[] FIELDS = values();
}
+ /**
+ * SaveStatus.Known contains information about erased / nullified fields,
+ * which we can use in order to mark the corresponding fields as changed
+ * and setting them to null when they are erased.
+ */
+ public static int[] saveStatusMasks;
+
+ static
+ {
+ saveStatusMasks = new int[SaveStatus.values().length];
+ for (int i = 0; i < saveStatusMasks.length; i++)
+ {
+ SaveStatus saveStatus = SaveStatus.forOrdinal(i);
+ int mask = 0;
+ if (forceFieldChangedToNullFlag(saveStatus, saveStatus.known::is,
DepsErased))
+ mask |= setFieldIsNullAndChanged(PARTIAL_DEPS, mask);
+ if (forceFieldChangedToNullFlag(saveStatus, saveStatus.known::is,
ExecuteAtErased))
+ mask |= setFieldIsNullAndChanged(EXECUTE_AT, mask);
+ if (forceFieldChangedToNullFlag(saveStatus, saveStatus.known::is,
DefinitionErased))
+ mask |= setFieldIsNullAndChanged(PARTIAL_TXN, mask);
+ if (forceFieldChangedToNullFlag(saveStatus, saveStatus.known::is,
WasApply))
+ mask |= setFieldIsNullAndChanged(RESULT, mask);
+ saveStatusMasks[i] = mask;
+ }
+ }
+
+ private static <T> boolean forceFieldChangedToNullFlag(SaveStatus
saveStatus, Predicate<T> predicate, T erased)
+ {
+ return saveStatus == SaveStatus.Vestigial || predicate.test(erased);
+ }
+
public static class Builder
{
protected final int mask;
@@ -151,72 +187,16 @@ public class CommandChange
this(ALL);
}
- public TxnId txnId()
- {
- return txnId;
- }
-
- public Timestamp executeAt()
- {
- return executeAt;
- }
-
- // TODO: why is this unused in BurnTest
- public Timestamp executeAtLeast()
- {
- return executeAtLeast;
- }
-
public SaveStatus saveStatus()
{
return saveStatus;
}
- public Status.Durability durability()
- {
- return durability;
- }
-
- public Ballot acceptedOrCommitted()
- {
- return acceptedOrCommitted;
- }
-
- public Ballot promised()
- {
- return promised;
- }
-
public StoreParticipants participants()
{
return participants;
}
- public PartialTxn partialTxn()
- {
- return partialTxn;
- }
-
- public PartialDeps partialDeps()
- {
- return partialDeps;
- }
-
- public CommandChange.WaitingOnProvider waitingOn()
- {
- return waitingOn;
- }
-
- public Writes writes()
- {
- return writes;
- }
-
- public Result result()
- {
- return result;
- }
-
public void clear()
{
flags = 0;
@@ -286,7 +266,7 @@ public class CommandChange
public Builder maybeCleanup(Cleanup cleanup)
{
- if (saveStatus() == null)
+ if (saveStatus == null)
return this;
switch (cleanup)
@@ -531,6 +511,10 @@ public class CommandChange
flags = setChanged(PARTICIPANTS, flags);
flags = setChanged(SAVE_STATUS, flags);
}
+
+ if (after.saveStatus() != null)
+ flags |= saveStatusMasks[after.saveStatus().ordinal()];
+
return flags;
}
diff --git a/accord-core/src/test/java/accord/impl/basic/InMemoryJournal.java
b/accord-core/src/test/java/accord/impl/basic/InMemoryJournal.java
index deeceb50..9807c877 100644
--- a/accord-core/src/test/java/accord/impl/basic/InMemoryJournal.java
+++ b/accord-core/src/test/java/accord/impl/basic/InMemoryJournal.java
@@ -83,9 +83,8 @@ import static accord.impl.CommandChange.isChanged;
import static accord.impl.CommandChange.isNull;
import static accord.impl.CommandChange.nextSetField;
import static accord.impl.CommandChange.setChanged;
-import static accord.impl.CommandChange.setFieldIsNull;
+import static accord.impl.CommandChange.setFieldIsNullAndChanged;
import static accord.impl.CommandChange.toIterableSetFields;
-import static accord.impl.CommandChange.unsetFieldIsNull;
import static accord.impl.CommandChange.unsetIterable;
import static accord.impl.CommandChange.validateFlags;
import static accord.local.Cleanup.Input.FULL;
@@ -172,10 +171,10 @@ public class InMemoryJournal implements Journal
if (saved == null)
return null;
- // TODO (expected): match C* and visit in reverse order
Builder builder = null;
- for (Diff diff : saved)
+ for (int i = saved.size() - 1; i >= 0; i--)
{
+ Diff diff = saved.get(i);
if (builder == null)
builder = new Builder(diff.txnId, load);
builder.apply(diff);
@@ -560,15 +559,21 @@ public class InMemoryJournal implements Journal
{
Field field = nextSetField(iterable);
- this.flags = setChanged(field, this.flags);
+ // Since we are iterating in reverse order, we skip the fields
that were
+ // set by entries writer later (i.e. already read ones).
+ if (isChanged(field, this.flags) || isNull(field, mask))
+ {
+ iterable = unsetIterable(field, iterable);
+ continue;
+ }
+
if (isNull(field, diff.flags))
{
- this.flags = setFieldIsNull(field, this.flags);
- setNull(field);
+ this.flags = setFieldIsNullAndChanged(field, this.flags);
}
else
{
- this.flags = unsetFieldIsNull(field, this.flags);
+ this.flags = setChanged(field, this.flags);
deserialize(diff, field);
}
@@ -576,54 +581,6 @@ public class InMemoryJournal implements Journal
}
}
- private void setNull(Field field)
- {
- switch (field)
- {
- case EXECUTE_AT:
- executeAt = null;
- break;
- case EXECUTES_AT_LEAST:
- executeAtLeast = null;
- break;
- case MIN_UNIQUE_HLC:
- minUniqueHlc = 0;
- break;
- case SAVE_STATUS:
- saveStatus = null;
- break;
- case DURABILITY:
- durability = null;
- break;
- case ACCEPTED:
- acceptedOrCommitted = null;
- break;
- case PROMISED:
- promised = null;
- break;
- case PARTICIPANTS:
- participants = null;
- break;
- case PARTIAL_TXN:
- partialTxn = null;
- break;
- case PARTIAL_DEPS:
- partialDeps = null;
- break;
- case WAITING_ON:
- waitingOn = null;
- break;
- case WRITES:
- writes = null;
- break;
- case RESULT:
- result = null;
- break;
- case CLEANUP:
- throw new IllegalStateException();
- }
- }
-
private void deserialize(Diff diff, Field field)
{
switch (field)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
