You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by "belliottsmith (via GitHub)" <gi...@apache.org> on 2025/07/03 09:29:40 UTC

[PR] Fixes 250701 [cassandra-accord]

belliottsmith opened a new pull request, #216:
URL: https://github.com/apache/cassandra-accord/pull/216

   (no comment)


-- 
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: pr-unsubscribe@cassandra.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org


Re: [PR] Fixes 250701 [cassandra-accord]

Posted by "ifesdjeen (via GitHub)" <gi...@apache.org>.
ifesdjeen merged PR #216:
URL: https://github.com/apache/cassandra-accord/pull/216


-- 
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: pr-unsubscribe@cassandra.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org


Re: [PR] Fixes 250701 [cassandra-accord]

Posted by "belliottsmith (via GitHub)" <gi...@apache.org>.
belliottsmith commented on code in PR #216:
URL: https://github.com/apache/cassandra-accord/pull/216#discussion_r2182976735


##########
accord-core/src/main/java/accord/impl/progresslog/WaitingState.java:
##########
@@ -467,6 +480,8 @@ private void runInternal(SafeCommandStore safeStore, SafeCommand safeCommand, De
         awaitRoute = awaitRoute.slice(roundStart, roundEnd);
         // TODO (desired): use some mechanism (e.g. random chance or another counter)
         //   to either periodically fetch the whole remaining route or gradually increase the slice length
+        if (tracing != null)
+            tracing.trace(owner.commandStore, String.format("Blocked until %s. Waiting for %s to satisfy; round %d of %d.", blockedUntil, awaitRoute, roundIndex, (awaitRoute.size() + (roundSize - 1))/roundSize));

Review Comment:
   Correct, we would have bigger problems if it could



-- 
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: pr-unsubscribe@cassandra.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org


Re: [PR] Fixes 250701 [cassandra-accord]

Posted by "ifesdjeen (via GitHub)" <gi...@apache.org>.
ifesdjeen commented on code in PR #216:
URL: https://github.com/apache/cassandra-accord/pull/216#discussion_r2182836432


##########
accord-core/src/main/java/accord/local/durability/ShardDurability.java:
##########
@@ -95,6 +97,7 @@ public Waiting next()
     // TODO (expected): support intra-shard parallelism
     private class ShardScheduler
     {
+        final boolean adhoc;

Review Comment:
   Discussed on slack a bit; could you add a small note about repair/circumstances when we need adhoc? 



##########
accord-core/src/main/java/accord/messages/Propagate.java:
##########
@@ -306,11 +337,19 @@ public Void apply(SafeCommandStore safeStore)
             case PreAccepted:
                 // only preaccept if we coordinate the transaction
                 if (safeStore.ranges().coordinates(txnId).intersects(route) && Route.isFullRoute(route))
+                {
+                    if (trace != null)
+                        trace.trace(safeStore.commandStore(), "Pre-accepting");
                     Commands.preaccept(safeStore, safeCommand, participants, txnId, partialTxn, null, false);
+                }
 
             case NotDefined:
                 if (invalidIf == IfUncommitted)
+                {
+                    if (trace != null)
+                        trace.trace(safeStore.commandStore(), "Mrking invalidIfUncommitted");

Review Comment:
   nit: mrking



##########
accord-core/src/main/java/accord/coordinate/CheckShards.java:
##########
@@ -107,6 +117,9 @@ protected Action process(Id from, CheckStatusReply reply)
         }
         else
         {
+            if (tracing != null)
+                tracing.trace(null, String.format("%s received failure reply %s from %s", getClass().getSimpleName(), reply, from));

Review Comment:
   applies not just here: should we maybe interpolate _inside_ trace? We're already allocating an array to pass to format, but in case we accidentally have a mismatch in number of arguments, it would be good to fall back instead of throwing. 
   
   something vaguely like
   
   ```
       public static String safeFormat(String format, Object... args)
       {
           try
           {
               return String.format(format, args);
           }
           catch (Throwable t)
           {
               return String.format("Could not format, due to %s, falling back: %s", t.getMessage(), format, Arrays.toString(args))
           }
       }
   ```
   
   it's just if we make a mistake in a trace that puts us one step further, not only we can't debug what we wanted to debug but also now the message is broken



-- 
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: pr-unsubscribe@cassandra.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org


Re: [PR] Fixes 250701 [cassandra-accord]

Posted by "ifesdjeen (via GitHub)" <gi...@apache.org>.
ifesdjeen commented on code in PR #216:
URL: https://github.com/apache/cassandra-accord/pull/216#discussion_r2182895044


##########
accord-core/src/main/java/accord/impl/progresslog/WaitingState.java:
##########
@@ -467,6 +480,8 @@ private void runInternal(SafeCommandStore safeStore, SafeCommand safeCommand, De
         awaitRoute = awaitRoute.slice(roundStart, roundEnd);
         // TODO (desired): use some mechanism (e.g. random chance or another counter)
         //   to either periodically fetch the whole remaining route or gradually increase the slice length
+        if (tracing != null)
+            tracing.trace(owner.commandStore, String.format("Blocked until %s. Waiting for %s to satisfy; round %d of %d.", blockedUntil, awaitRoute, roundIndex, (awaitRoute.size() + (roundSize - 1))/roundSize));

Review Comment:
   just to make sure: roundSize can never be 0 right? Checked the code and doesn't seem like it can, but wanted to ask to make sure.



-- 
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: pr-unsubscribe@cassandra.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org


Re: [PR] Fixes 250701 [cassandra-accord]

Posted by "belliottsmith (via GitHub)" <gi...@apache.org>.
belliottsmith commented on code in PR #216:
URL: https://github.com/apache/cassandra-accord/pull/216#discussion_r2182981022


##########
accord-core/src/main/java/accord/coordinate/CheckShards.java:
##########
@@ -107,6 +117,9 @@ protected Action process(Id from, CheckStatusReply reply)
         }
         else
         {
+            if (tracing != null)
+                tracing.trace(null, String.format("%s received failure reply %s from %s", getClass().getSimpleName(), reply, from));

Review Comment:
   Sure. I didn't want to spend too long agonising over the API versus other pressures right now, but I had imagined evolving it in this direction. This particular suggestion seems like an easy tweak that I will work in.



-- 
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: pr-unsubscribe@cassandra.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org