cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alex Petrov (JIRA)" <j...@apache.org>
Subject [jira] [Comment Edited] (CASSANDRA-13992) Don't send new_metadata_id for conditional updates
Date Tue, 14 Nov 2017 08:45:00 GMT

    [ https://issues.apache.org/jira/browse/CASSANDRA-13992?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16251096#comment-16251096
] 

Alex Petrov edited comment on CASSANDRA-13992 at 11/14/17 8:44 AM:
-------------------------------------------------------------------

bq. METADATA_CHANGED tells the client if it needs to update its local copy of the metadata.


You're right, great point. Sure, I've changed the patch to _always_ send metadata (by avoiding
setting {{SKIP_METADATA}} for LWTs) and _never_ send metadata id when statement is LWT (by
avoiding setting {{METADATA_CHANGED}} for LWTs)). Technically, {{EMPTY}} part isn't even necessary
in that case, but we don't have to calculate it, so why not. 

[~KurtG] to give a bit of context, {{METADATA_CHANGED}} flag is instructing the driver to
cache a newly received version of metadata (alongside with a new metadata ID). While {{SKIP_METADATA}}
flag is hinting the driver to use already cached metadata from previous responses. If I understood
it correctly, what [~omichallat] proposed here was to avoid setting {{METADATA_CHANGED}} flag,
so driver wouldn't cache the metadata, _but_ still send a metadata all the time (since it's
potentially changing on each request).

bq. I'm with Olivier that that's a hacky addition to the driver

There's no addition to the driver (also, was no addition in the previous version of the patch).
The only difference is that we can spare the driver a couple of cycles. Behaviour was right
in both cases.

I've pulled in the last version of the driver, added comments and prettified it a bit. If
we all agree that this behaviour is correct, can anyone take a short look at it?

The patch can be found:

|[here|https://github.com/apache/cassandra/compare/trunk...ifesdjeen:CASSANDRA-13992]|


was (Author: ifesdjeen):
bq. METADATA_CHANGED tells the client if it needs to update its local copy of the metadata.


You're right, great point. Sure, I've changed the patch to _always_ send metadata (by avoiding
setting {{SKIP_METADATA}} for LWTs) and _never_ send metadata id when statement is LWT (by
avoiding setting {{METADATA_CHANGED}} for LWTs)). Technically, {{EMPTY}} part isn't even necessary
in that case, but we don't have to calculate it, so why not. 

[~KurtG] to give a bit of context, {{METADATA_CHANGED}} flag is instructing the driver to
cache a newly received version of metadata (alongside with a new metadata ID). While {{SKIP_METADATA}}
flag is hinting the driver to use already cached metadata from previous responses. If I understood
it correctly, what [~omichallat] proposed here was to avoid setting {{METADATA_CHANGED}} flag,
so driver wouldn't cache the metadata, _but_ still send a metadata all the time (since it's
potentially changing on each request).

bq. I'm with Olivier that that's a hacky addition to the driver

There's no addition to the driver (also, was no addition in the previous version of the patch).
The only difference is that we can spare the driver a couple of cycles. Behaviour was right
in both cases.

I've pulled in the last version of the driver, added comments and prettified it a bit. If
we all agree that this behaviour is correct, can anyone take a short look at it?

> Don't send new_metadata_id for conditional updates
> --------------------------------------------------
>
>                 Key: CASSANDRA-13992
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-13992
>             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}
> CREATE TABLE foo (k int PRIMARY KEY)
> {code}
> And the prepared statement
> {code}
> INSERT INTO foo (k) VALUES (?) IF NOT EXISTS
> {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
k.
> 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
(v6.4.14#64029)

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org


Mime
View raw message