cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Robert Stupp (JIRA)" <>
Subject [jira] [Commented] (CASSANDRA-13992) Don't send new_metadata_id for conditional updates
Date Thu, 16 Nov 2017 13:57:00 GMT


Robert Stupp commented on CASSANDRA-13992:

As elaborated above, LWTs definitely need special handling as their result set is (or can
be) different for each invocation. Remembering (and evicting) metadata for that result set
(which is in the "ok" case just one column {{[applied]}} - i.e. not much) is probably not
beneficiary. Doing that might be worth another look in a separate ticket at a later point.
But I don't expect much from that kind of optimization.

I'm not excited about adding a "special value" to indicate that the metadata is empty (neither
an empty {{byte[]}} nor the MD5 over an empty {{byte[]}}). Just omitting the metadata flags
introduced by CASSANDRA-10786 is sufficient.

Olivier correctly pointed out that (unnecessary) additional processing should be avoided -
and I second that. Looking at the {{METADATA_CHANGED}} flag is very cheap - comparing values
is more expensive (checking a bit in a CPU register or L1 cache line vs. many dloads). Keeping
the performance aspect aside, it also looks cleaner.

Since we do not need those metadata-flags for LWTs, we can just omit those and it "magically"
works - and that's pretty much what [~ifesdjeen]'s patch does.

I've written a [unit test|]
that verifies the expected behavior on the protocol level.

+1 on [~ifesdjeen]'s patch. I'd like to see the new unit test being added.
Only change that would be good to have is to move the {{boolean hasConditions()}} function
up to {{CQLStatement}} and implement it there as {{public default boolean hasConditions()
{ return false; } }}. By that you can remove the {{instanceof}} and type casts in the change
in {{ExecuteMessage}} and probably also save the {{isLWT}} variable as it would just be a
call to {{statement.hasConditions()}}.

> Don't send new_metadata_id for conditional updates
> --------------------------------------------------
>                 Key: CASSANDRA-13992
>                 URL:
>             Project: Cassandra
>          Issue Type: Bug
>            Reporter: Olivier Michallat
>            Assignee: Kurt Greaves
>            Priority: Minor
> This is a follow-up to CASSANDRA-10786.
> Given the table
> {code}
> {code}
> And the prepared statement
> {code}
> {code}
> The result set metadata changes depending on the outcome of the update:
> * if the row didn't exist, there is only a single column \[applied] = true
> * if it did, the result contains \[applied] = false, plus the current value of column
> The way this was handled so far is that the PREPARED response contains no result set
metadata, and therefore all EXECUTE messages have SKIP_METADATA = false, and the responses
always include the full (and correct) metadata.
> CASSANDRA-10786 still sends the PREPARED response with no metadata, *but the response
to EXECUTE now contains a {{new_metadata_id}}*. The driver thinks it is because of a schema
change, and updates its local copy of the prepared statement's result metadata.
> The next EXECUTE is sent with SKIP_METADATA = true, but the server appears to ignore
that, and still sends the metadata in the response. So each response includes the correct
metadata, the driver uses it, and there is no visible issue for client code.
> The only drawback is that the driver updates its local copy of the metadata unnecessarily,
every time. We can work around that by only updating if we had metadata before, at the cost
of an extra volatile read. But I think the best thing to do would be to never send a {{new_metadata_id}}
in for a conditional update.

This message was sent by Atlassian JIRA

To unsubscribe, e-mail:
For additional commands, e-mail:

View raw message