[
https://issues.apache.org/jira/browse/CASSANDRA-5062?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13625529#comment-13625529
]
Sylvain Lebresne commented on CASSANDRA-5062:
---------------------------------------------
A few other remarks on v3:
* In SP.cas(), after having read the CASed values, I don't think
Objects.equals() will work because of the timestamps. Which makes me remark
that in theory having the API take a ColumnFamily as 'expected' is slightly
weird because we will ignore the timestamps (I don't suppose the intent was to
take them into account). Not a huge big deal (and not one we'll have on the CQL
side), so probably not worth complicating said API, but may be worth a comment.
* Also, the timestamp of the columns we write during the commit should be the
ballot timestamp. I don't see it done by the patch.
* SystemTable.savePaxosCommit doesn't seem to delete in_progress_ballot (it
deletes 'proposal' only).
* In SystemTable.loadPaxosState, we must be careful that UntypedResultSet.Row
doesn't handle null well (and Row.fromBytes doesn't either).
* It's more of a nit, but I think there is a bunch of places where we could
reuse the new Commit class. In particular, ProposeRequest is kind of a
duplicate. But there is a few other places too (the propose and commit method
in PaxosState could take a Commit directly, in PrepareCallback, the inProgress
ballot and update could be grouped, ...).
* Also, the paxos state CF basically holds 2 pairs of ballot/update, the
in_progress ones and the MRC ones. Could be nice to name them to reflect that
symetry (something like
in_progress_ballot/in_progress_upd/most_recent_commit_ballot/most_recent_commit_upd)?
* The patch has minors updates to the cassandra.in.sh file that should probably
be reverted.
For the record, I'm not a fan of having PaxosState share "buckets" (versus
directly reading the SystemTable). I kind of understand the reasoning of
keeping things in memory, but I can't stop to think that it's premature
optimisation (that creates artificial contention and make the code slightly
harder to reason about imo). Anyway, as far as I can tell it's not incorrect,
but I'm not totally sold on it.
Also, I think we're lacking something around reads and around CL.SERIAL. I
think we at least need to support CL.SERIAL reads, that would have to go
through paxos (so they play inProgress values). But I would a relatively big
fan to also allow a (non-SERIAL) CL in cas() that would be applied to the final
commit of the cas write. That way, if you have a successful cas call at
CL.QUORUM, you can do a normal QUORUM read and be guaranteed to see your value,
which is nice (of course, if the cas write actually timeout, you'd have to do a
SERIAL read to check the up to date state, but at least in the normal
non-failing case you don't need SERIAL reads).
> Support CAS
> -----------
>
> Key: CASSANDRA-5062
> URL: https://issues.apache.org/jira/browse/CASSANDRA-5062
> Project: Cassandra
> Issue Type: New Feature
> Components: API, Core
> Reporter: Jonathan Ellis
> Fix For: 2.0
>
> Attachments: half-baked commit 1.jpg, half-baked commit 2.jpg,
> half-baked commit 3.jpg
>
>
> "Strong" consistency is not enough to prevent race conditions. The classic
> example is user account creation: we want to ensure usernames are unique, so
> we only want to signal account creation success if nobody else has created
> the account yet. But naive read-then-write allows clients to race and both
> think they have a green light to create.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
