CASSANDRA-18100 CAS and serial read support for Accord

[aweisberg]

CASSANDRA-18100

[maedhroz]

I have as one of the TODO items in CASSANDRA-18107 latency metrics recording in AccordService#coordinate(), probably as a "Transaction" item in ClientRequestsMetricsHolder. Maybe we could just throw that in here, avoid BB, and kill two birds w/ one stone?

[aweisberg]

Seems reasonable. Do we want separate metrics for transactions that are write only since they generally have a very different latency profile?

[maedhroz]

Do we want separate metrics for transactions that are write only

I'm fine w/ having separate read-only, write-only, and "mixed" versions. It's nicer to have more granularity when we don't need it than less when we do :p

[aweisberg]

This looks more complicated then just latency since it's counting a bunch of different things. Abort, timeout, unavailable, mutation size. I think we need to think a little harder about metrics for Accord transactions.

[maedhroz]

Agreed, although we already handle similar concerns in CASClientWriteRequestMetrics -> CASClientRequestMetrics -> ClientRequestMetrics, where we distinguish between those things.

[maedhroz]

I'm fine if you want to defer to CASSANDRA-18107

[maedhroz]

With this in place, do we still need the per-value checks in compareWithOperator()? I think the only thing we're not currently double-validating is the usage in TxnCondition#applies().

[aweisberg]

It's probably redundant, but that is such a fast thing to check at runtime I think it's fine to do it in both?

I added this check and moved it earlier because I was unsure if we had those unset values in practice and in which types of bounds. It's a little easier to find out the unset value is unset when the bound is constructed rather than later when it is applied.

Don't feel strongly either way.

[maedhroz]

It's a little easier to find out the unset value is unset when the bound is constructed rather than later when it is applied.

Would it make sense to have the value == UNSET_BYTE_BUFFER check in the TxnCondition.Value constructor? We'd find out earlier I guess.

In terms of whether we'd want to remove the duplicate checks in compareWithOperator(), I'll leave it up to you. If I'm able to make it private again when I finish the CQL integration, perhaps I'd remove them, but I'm fine either way at this point.

[aweisberg]

For CQL3CasRequest this would be later not earlier? The bounds are created when the request is first created from the parser/planner bits.

[maedhroz]

LGTM

[aweisberg]

Merged as a3ccd4d93d1e1ed3ca65e2c3ad3e0126a2c8bf8f