dcapwell commented on code in PR #3777:
URL: https://github.com/apache/cassandra/pull/3777#discussion_r1915754665
##########
src/java/org/apache/cassandra/cql3/statements/TransactionStatement.java:
##########
@@ -324,39 +337,130 @@ List<TxnWrite.Fragment> createWriteFragments(ClientState
state, QueryOptions opt
return fragments;
}
- AccordUpdate createUpdate(ClientState state, QueryOptions options,
Map<Integer, NamedSelect> autoReads, Consumer<Key> keyConsumer)
+ AccordUpdate createUpdate(ClusterMetadata cm, ClientState state,
QueryOptions options, Map<Integer, NamedSelect> autoReads, Set<Key> keys)
{
- return new TxnUpdate(createWriteFragments(state, options, autoReads,
keyConsumer), createCondition(options), null, false);
+ checkArgument(keys.isEmpty(), "Construct update before reads so the
key set can be used to determine commit consistency level");
+ List<TxnWrite.Fragment> writeFragments = createWriteFragments(state,
options, autoReads, keys);
+ ConsistencyLevel commitCL = consistencyLevelForAccordCommit(cm, keys,
options.getConsistency());
+ return new TxnUpdate(writeFragments, createCondition(options),
commitCL, false);
}
Keys toKeys(SortedSet<Key> keySet)
{
return new Keys(keySet);
}
+ private static TransactionalMode transactionalModeForSingleKey(Keys keys)
+ {
+ return Schema.instance.getTableMetadata(((AccordRoutableKey)
keys.get(0)).table()).params.transactionalMode;
+ }
+
+ private ConsistencyLevel consistencyLevelForAccordRead(ClusterMetadata cm,
Set<Key> keys, ConsistencyLevel consistencyLevel)
+ {
+ // Write transactions are read/write so it creates a read and ends up
needing a consistency level
Review Comment:
> Txn.Kind for a read/write transaction is write
yes, read/write and write both have the same kind. The only difference is
the `read.keys()` field
> dependency management queries for reads despite the fact that the read is
empty it will find a write only transaction despite the fact that it only has
writes
I don't follow this sentence... are you trying to talk about
`accord.messages.PreAccept#calculateDeps` or something else? If you mean this
then we use `txn.keys()` which would be the union of read/write.
> The way things actually work is heavily slanted towards Accord having
reads and read/write transactions only.
checking if there are reads is a thing we do in accord today
```
if (txn.read().keys().isEmpty()) ... write only logic ...
else ... logic that does reads ...
```
I push back on this as I am seeing a lot of code that says it wishes that
reads could be null to solve problems... but each code path that hits that
could always known if this is a `Key` or `Range` txn (which seems to be the
whole reason it matters... as far as i can tell). I am trying to flesh out why
these concerns keep coming up.
--
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]
