You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by "maedhroz (via GitHub)" <gi...@apache.org> on 2023/03/23 19:51:53 UTC

[GitHub] [cassandra-accord] maedhroz opened a new pull request, #37: add shareable APPLIED and INVALIDATED implementations of Result

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

   patch by Caleb Rackliffe; reviewed by ? for CASSANDRA-18355


-- 
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


[GitHub] [cassandra-accord] belliottsmith commented on a diff in pull request #37: add shareable APPLIED and INVALIDATED implementations of Result

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


##########
accord-core/src/main/java/accord/api/Write.java:
##########
@@ -30,5 +31,5 @@
  */
 public interface Write
 {
-    AsyncChain<Void> apply(Seekable key, SafeCommandStore safeStore, Timestamp executeAt, DataStore store);
+    AsyncChain<Void> apply(Seekable key, SafeCommandStore safeStore, Timestamp executeAt, DataStore store, Command.Executed command);

Review Comment:
   Do we need to provide the whole command, or could we provide just the `PartialTxn` or `Update`?



-- 
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


[GitHub] [cassandra-accord] maedhroz commented on pull request #37: add shareable APPLIED and INVALIDATED implementations of Result

Posted by "maedhroz (via GitHub)" <gi...@apache.org>.
maedhroz commented on PR #37:
URL: https://github.com/apache/cassandra-accord/pull/37#issuecomment-1481814074

   https://app.circleci.com/pipelines/github/maedhroz/cassandra?branch=CASSANDRA-18355


-- 
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


[GitHub] [cassandra-accord] dcapwell commented on a diff in pull request #37: add shareable APPLIED and INVALIDATED implementations of Result

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


##########
accord-core/src/main/java/accord/api/Write.java:
##########
@@ -30,5 +31,5 @@
  */
 public interface Write
 {
-    AsyncChain<Void> apply(Seekable key, SafeCommandStore safeStore, Timestamp executeAt, DataStore store);
+    AsyncChain<Void> apply(Seekable key, SafeCommandStore safeStore, Timestamp executeAt, DataStore store, Command.Executed command);

Review Comment:
   I kinda don't like exposing `Command` in the API and kinda prefer `TxnId`... you do the following in C*
   
   ```
         // Pull updates for this key that did not require completion form the transaction itself and apply them: 
               TxnUpdate txnUpdate = (TxnUpdate) command.partialTxn().update();
               assert txnUpdate != null : "PartialTxn should contain an update if we're applying a write!";
               List<Update> updates = txnUpdate.completeUpdatesForKey((RoutableKey) key);
               updates.forEach(update -> results.add(update.write(timestamp, nowInSeconds)));
   ```
   
   so you will need it, but I just kinda don't like it...



-- 
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


[GitHub] [cassandra-accord] maedhroz merged pull request #37: add shareable APPLIED and INVALIDATED implementations of Result

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


-- 
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


[GitHub] [cassandra-accord] maedhroz commented on a diff in pull request #37: add shareable APPLIED and INVALIDATED implementations of Result

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


##########
accord-core/src/main/java/accord/api/Write.java:
##########
@@ -30,5 +31,5 @@
  */
 public interface Write
 {
-    AsyncChain<Void> apply(Seekable key, SafeCommandStore safeStore, Timestamp executeAt, DataStore store);
+    AsyncChain<Void> apply(Seekable key, SafeCommandStore safeStore, Timestamp executeAt, DataStore store, Command.Executed command);

Review Comment:
   @belliottsmith above suggests using just the `PartialTxn` or the `Update`. You're suggesting perhaps passing `TxnId` and getting the command from `SafeCommandStore` (along w/ the key, which I think we also have available)?



-- 
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