[
https://issues.apache.org/jira/browse/CASSANDRA-5062?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13626789#comment-13626789
]
Sylvain Lebresne commented on CASSANDRA-5062:
---------------------------------------------
bq. If the PaxosState.propose is not locked vs prepare for that row
I'm lost. I totally agree with you that we should lock propose vs prepare.
But we could have an array of 1024 Lock object somewhere, and *both* propose
and prepare for key k would do:
{noformat}
Lock lock = locks[k.hashCode() % 1024];
lock.lock();
try {
// do whatever you should
} finally {
lock.release();
}
{noformat}
I.e. exactly what we used to do to protect 2ndary indexes writes.
I'm not critizing the fact that propose and prepare are synchronized together,
they should be. And yes, having 1 lock per row would be crazy, so I'm fine
having just a fixed amount of locks. Because I'm not worried about local
contention on the PaxosState propose and prepare method.
The contention I'm more worried about is that because we may use the same
PaxosState object for say row k1 and k2, a proposer on k1 will contend with a
proposer on k2, even though those are different rows. And that contention might
mean one of the proposer will get pre-empted during propose and will have to
restart from scratch. Which is costly, because that's a bunch of round-trip
message wasted.
To take a step back, what I'm saying is that a priori, the way I would have
coded this, would have been to just use the system table for paxos states,
without trying to "cache" them in memory manually (which is the reason why we
need to bucket them, so it don't get crazy) and just trust our storage engine
initially. And we would use the locks to protect the propose and prepare phase
as described above.
Now I'm not necessarily saying that doing what I've just described would be
necessarily more efficient, but at least it's not absolutely clear to me that
the current patch is always a win. Because what it means on slightly faster
local reads (and since we do read-by-name, even if we were to read the system
table directly, we wouldn't hit disk most of the time, so it's not totally
clear how much faster it is) against higher contention at the paxos level
(which, as said above, is potentially order of magnitudes more costly). Again,
I don't what's best, but since it feels to me that having one paxos state per
row is more "natural", I feel the current way of keeping paxos states in memory
is a bit of a premature optimisation, and thus I would have prefered keeping
that for later if we realise there is a need to optimise.
bq. it's more likely that the replicas we don't wait for will stay out of date
longer than they "should" for the MRC check
First, that wouldn't matter for the correctness of the overall algorithm, so at
this point I would say that it's not a big deal, at best a performance
argument. Second, we can absolutely do what we do for read repair, i.e. when we
receive responses after the initial QUORUM, we repair in the background if
needed. We have prior arts and that's not very hard to do.
bq. So I see this as two alternatives, each with drawbacks, rather than one
strictly better than the other
I couldn't disagree more. Unless I misread the code, in which case I'm sorry,
the current patch can get in a state where for a row no CAS operation on that
row can be done at all (they will all UAE) until the user does a manual repair,
because if less than a quorum of replica have received the last MRC commit
message, there is no mechanism to heal that situation (since we UAE *before* we
may heal replica if we are in a situation where say only quorum nodes are
alive, but less than quorum have the last MRC).
Also, again unless I read this wrong, the current patch may throw UAE
exceptions even when no replica in the cluster are dead and there is no real
reason to throw (again, in the case where less than a quorum of replica that
respond have the MRC, which can happen even during normal operation with no
errors).
And I think that both of those (not combined, each of them) are big deals.
Blockers in fact.
bq. And I prefer the alternative that already has code written.
So to be clear, the "alternative" I'm suggesting is a few lines of changes. I'm
happy to provide a commit with said change if it's not very clear what I'm
talking about.
bq. If too many replicas are down and we don't get a promise, your logic says
we should retry with a new (higher) ballot
Absolutely not. I have no problem with throwing an UAE if too many replica are
*down*, what else could we do? I have a problem with throwing a UAE if enough
(i.e. a quorum) replica are up *but* not enough have the last MRC.
> 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
