dcapwell commented on code in PR #3432:
URL: https://github.com/apache/cassandra/pull/3432#discussion_r1693461885
##########
src/java/org/apache/cassandra/service/accord/serializers/CommandStoreSerializers.java:
##########
@@ -153,9 +153,9 @@ public RedundantBefore.Entry deserialize(DataInputPlus in,
int version) throws I
long endEpoch = in.readUnsignedVInt();
if (endEpoch == 0) endEpoch = Long.MAX_VALUE;
else endEpoch = startEpoch + 1 + endEpoch;
- TxnId bootstrappedAt = CommandSerializers.txnId.deserialize(in,
version);
TxnId locallyAppliedOrInvalidatedBefore =
CommandSerializers.txnId.deserialize(in, version);
TxnId shardAppliedOrInvalidatedBefore =
CommandSerializers.txnId.deserialize(in, version);
+ TxnId bootstrappedAt = CommandSerializers.txnId.deserialize(in,
version);
Review Comment:
I am working on a test, and seems that there is still an issue...
the equality check failed here:
```
expected:
{(tbl:-Inf,tbl:pk1]=(195970445814877,195970445814878,[195970445814808,7046332644952687526,9(RX),1277444686]),
(tbl:pk1,tbl2:pk2]=null,
(tbl2:pk2,tbl2:pk3]=(228431633815866,228431633815867,...
but was:
{(tbl:-Inf,tbl:pk1]=(195970445814877,195970445814880,[195970445814808,7046332644952687526,9(RX),1277444686]),
(tbl:pk1,tbl2:pk2]=null,
(tbl2:pk2,tbl2:pk3]=(228431633815866,228431633815869,...
```
##########
src/java/org/apache/cassandra/service/accord/async/AsyncLoader.java:
##########
@@ -83,7 +83,11 @@ private static <K, V, S extends AccordSafeState<K, V>> void
referenceAndAssemble
List<AsyncChain<?>> listenChains)
{
S safeRef = cache.acquire(key);
- context.put(key, safeRef);
+ if (context.putIfAbsent(key, safeRef) != null)
Review Comment:
this is for double counting the ref?
##########
src/java/org/apache/cassandra/service/accord/AccordSafeCommandsForKey.java:
##########
@@ -25,6 +25,7 @@
import accord.api.Key;
import accord.local.CommandsForKey;
import accord.local.SafeCommandsForKey;
+import org.apache.cassandra.utils.concurrent.Ref;
Review Comment:
bad import?
##########
src/java/org/apache/cassandra/service/accord/async/AsyncOperation.java:
##########
@@ -301,6 +300,13 @@ public void start(BiConsumer<? super R, Throwable>
callback)
{
Invariants.checkState(this.callback == null);
this.callback = callback;
+ if (commandStore.inStore())
Review Comment:
why this change? personally I would just call `this.run()` for better
maintain... for example what happens if `loader.load` fails?
##########
src/java/org/apache/cassandra/service/accord/AccordSafeCommand.java:
##########
@@ -27,9 +27,33 @@
import accord.local.Listeners;
import accord.local.SafeCommand;
import accord.primitives.TxnId;
+import org.apache.cassandra.utils.concurrent.Ref;
public class AccordSafeCommand extends SafeCommand implements
AccordSafeState<TxnId, Command>
{
+ public static class DebugAccordSafeCommand extends AccordSafeCommand
Review Comment:
this is dead code, do you expect to make this a flag or was this just for
your own debugging?
--
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]
