ifesdjeen commented on code in PR #181:
URL: https://github.com/apache/cassandra-accord/pull/181#discussion_r1994173035
##########
accord-core/src/test/java/accord/impl/basic/InMemoryJournal.java:
##########
@@ -331,31 +330,50 @@ public void purge(CommandStores commandStores,
EpochSupplier minEpoch)
for (Map.Entry<TxnId, List<Diff>> e2 : localJournal.entrySet())
{
List<Diff> diffs = e2.getValue();
- int from;
+
if (diffs.isEmpty()) continue;
List<Diff> subset = diffs;
if (isPartialCompaction)
{
- subset = new ArrayList<>();
- int tmp1 = random.nextInt(diffs.size());
- int tmp2 = random.nextInt(diffs.size());
- from = Math.min(tmp1, tmp2);
- int max = Math.max(tmp1, tmp2);
- for (int i = from; i < max; i++)
- subset.add(diffs.get(i));
+ int count = diffs.size();
+ subset = new ArrayList<>(diffs);
+ int removeCount = random.nextInt(diffs.size());
+ while (removeCount-- > 0)
+ {
+ int removeIndex = random.nextInt(diffs.size());
+ if (subset.get(removeIndex) == null)
+ continue;
+ subset.set(removeIndex, null);
+ --count;
+ }
+
+ if (count == 0)
+ continue;
}
- InMemoryJournal.Builder builder = reconstruct(subset, ALL);
- if (builder == null || builder.saveStatus() == null)
- continue; // Partial compaction, no save status present
+ Builder[] builders = new Builder[diffs.size()];
+ for (int i = 0 ; i < subset.size() ; ++i)
+ {
+ if (subset.get(i) == null) continue;
+ Builder builder = new Builder(e2.getKey(), ALL);
+ builder.apply(subset.get(i));
+ builders[i] = builder;
+ }
- if (builder.saveStatus().status == Truncated ||
builder.saveStatus().status == Invalidated)
- continue; // Already truncated
+ Builder builder = new Builder(e2.getKey(), ALL);
+ for (int i = builders.length - 1; i >= 0 ; --i)
+ {
+ Builder diffBuilder = builders[i];
Review Comment:
nit: maybe rename to `current`?
##########
accord-core/src/main/java/accord/impl/CommandChange.java:
##########
@@ -329,50 +341,142 @@ public Cleanup maybeCleanup(Input input, Cleanup cleanup)
newSaveStatus = SaveStatus.TruncatedUnapplied;
saveStatus = newSaveStatus;
}
- forceSetNulls(eraseKnownFieldsMask[newSaveStatus.ordinal()]);
+ forceSetNulls(clearFields,
eraseKnownFieldsMask[newSaveStatus.ordinal()]);
}
+ this.cleanup = cleanup;
return cleanup;
}
- protected void setNulls(int mask)
+ protected void setNulls(boolean clearFields, int newFlags)
{
- mask &= ~(flags >>> 16); // limit ourselves to those fields that
have not already been set (high 16 bits are those already-set fields)
- forceSetNulls(mask);
+ newFlags &= ~(flags >>> 16); // limit ourselves to those fields
that have not already been set (high 16 bits are those already-set fields)
+ forceSetNulls(clearFields, newFlags);
+ }
+
+ protected boolean forceSetNulls(boolean clearFields, int newFlags)
+ {
+ newFlags &= ~nulls(flags); // limit ourselves to those fields that
are not already null
+ newFlags = nulls(newFlags); // limit ourselves to those fields
that are now being set to null
+ if (newFlags == 0)
+ return false;
+
+ if (clearFields)
+ {
+ int iterable = toIterableSetFields(newFlags);
+ for (Field next = nextSetField(iterable); next != null;
iterable = unsetIterable(next, iterable), next = nextSetField(iterable))
+ {
+ switch (next)
+ {
+ default: throw new UnhandledEnum(next);
+ case PARTICIPANTS: participants = null;
break;
+ case SAVE_STATUS: saveStatus = null;
break;
+ case PARTIAL_DEPS: partialDeps = null;
break;
+ case EXECUTE_AT: executeAt = null;
break;
+ case EXECUTES_AT_LEAST: executesAtLeast = null;
break;
+ case MIN_UNIQUE_HLC: minUniqueHlc = 0;
break;
+ case DURABILITY: durability = null;
break;
+ case ACCEPTED: acceptedOrCommitted = null;
break;
+ case PROMISED: promised = null;
break;
+ case WAITING_ON: waitingOn = null;
break;
+ case PARTIAL_TXN: partialTxn = null;
break;
+ case WRITES: writes = null;
break;
+ case CLEANUP: cleanup = null;
break;
+ case RESULT: result = null;
break;
+ }
+ }
+ }
+ flags |= newFlags;
+ return true;
}
- protected void forceSetNulls(int mask)
+ // only populate regular fields that are not already set, but apply
any Cleanup if it is stronger than any already present
+ public boolean fillInMissingOrCleanup(boolean clearFields, Builder add)
{
- mask &= ~nullMask(flags); // limit ourselves to those fields that
are not already null
- mask = nullMask(mask); // limit ourselves to those fields that are
now being set to null
- int iterable = toIterableSetFields(mask);
- for (Field next = nextSetField(iterable); next != null; iterable =
unsetIterable(next, iterable), next = nextSetField(iterable))
+ int addFlags = notAlreadySet(not(CLEANUP, add.flags), flags);
+ if (addFlags == 0)
+ return addCleanup(true, clearFields, add.cleanup);
+
+ setNulls(false, addFlags);
+ int iterable = toIterableSetFields(notNulls(addFlags));
+ for (Field next = nextSetField(iterable) ; next != null; next =
nextSetField(iterable = unsetIterable(next, iterable)))
{
switch (next)
{
default: throw new UnhandledEnum(next);
- case PARTICIPANTS: participants = null;
break;
- case SAVE_STATUS: saveStatus = null;
break;
- case PARTIAL_DEPS: partialDeps = null;
break;
- case EXECUTE_AT: executeAt = null;
break;
- case EXECUTES_AT_LEAST: executesAtLeast = null;
break;
- case MIN_UNIQUE_HLC: minUniqueHlc = 0;
break;
- case DURABILITY: durability = null;
break;
- case ACCEPTED: acceptedOrCommitted = null;
break;
- case PROMISED: promised = null;
break;
- case WAITING_ON: waitingOn = null;
break;
- case PARTIAL_TXN: partialTxn = null;
break;
- case WRITES: writes = null;
break;
- case CLEANUP: cleanup = null;
break;
- case RESULT: result = null;
break;
+ case PARTICIPANTS: participants = add.participants;
break;
+ case SAVE_STATUS: saveStatus = add.saveStatus;
break;
+ case PARTIAL_DEPS: partialDeps = add.partialDeps;
break;
+ case EXECUTE_AT: executeAt = add.executeAt;
break;
+ case EXECUTES_AT_LEAST: executesAtLeast =
add.executesAtLeast; break;
+ case MIN_UNIQUE_HLC: minUniqueHlc = add.minUniqueHlc;
break;
+ case DURABILITY: durability = add.durability;
break;
+ case ACCEPTED: acceptedOrCommitted =
add.acceptedOrCommitted; break;
+ case PROMISED: promised = add.promised;
break;
+ case WAITING_ON: waitingOn = add.waitingOn;
break;
+ case PARTIAL_TXN: partialTxn = add.partialTxn;
break;
+ case WRITES: writes = add.writes;
break;
+ case RESULT: result = add.result;
break;
}
}
- flags |= mask;
+ addCleanup(true, clearFields, add.cleanup);
+ return true;
+ }
+
+ // returns true if we made a material update to the Builder;
+ // that is, if we cleared a non-null field or if we are already
mask-only
+ public boolean clearSuperseded(boolean clearFields, Builder
superseding)
+ {
+ int unset = flags & superseding.flags;
+ if (notNulls(unset) == 0 && notNulls(flags) != 0)
+ return false;
+
+ forceSetNulls(clearFields, setFieldsMask(unset));
+ return true;
+ }
+
+ public boolean addCleanup(boolean saveCleanup, boolean clearFields,
Cleanup addCleanup)
+ {
+ if (addCleanup == null || addCleanup == NO)
+ return false;
+
+ if (cleanup != null && addCleanup.compareTo(cleanup) <= 0)
+ return false;
+
+ if (saveCleanup)
+ cleanup = addCleanup;
+
+ if (saveStatus != null &&
saveStatus.compareTo(cleanup.appliesIfNot) >= 0)
Review Comment:
can't we end up with null for `cleanup`? (full disclosure: Intellij has
highlighted that cleanup might be null, but it seems to be a valid point)
--
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]
