cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Sylvain Lebresne (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (CASSANDRA-11502) Fix denseness and column metadata updates coming from Thrift
Date Tue, 26 Apr 2016 13:29:12 GMT

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

Sylvain Lebresne commented on CASSANDRA-11502:
----------------------------------------------

Mostly looks good with the following remarks:
* Regarding the cleanup of {{SchemaKeyspace::makeUpdateTableMutation}} (in both 2.2 and 3.0),
I follow the reasoning and I'm relatively sure you're right, but was that breaking anything?
I mention this because those schema upgrade and thrift stuffs are easily subtle and are unfortunately
not very well covered by tests, so while this does look an ok removal, I'd hate to introduce
some subtle upgrade bug just to remove a few lines of code that will go away in 4.0. Anyway,
I'm not opposing the removal per-se, but just mentioning that if it was completely up to me,
I'd probably leave them in out of (probably unreasonably extreme) caution.
* I believe the removal of {{CLUSTERING_COLUMN}} when sparse table could screw up some CQL
tables. Now, I agree that if someone modifies a CQL table from thrift he is obviously looking
for trouble, but it shouldn't be too hard to avoid the problem by only excluding the column
based on its {{position()}} (only if it correspond to the last componenent of the comparator,
if that comparator is composite). Besides, the patch modify the code to preserve {{STATIC}}
column, which should only ever matters for CQL table so it sounds like you do care about that
case :)


> Fix denseness and column metadata updates coming from Thrift
> ------------------------------------------------------------
>
>                 Key: CASSANDRA-11502
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-11502
>             Project: Cassandra
>          Issue Type: Bug
>          Components: Distributed Metadata
>            Reporter: Aleksey Yeschenko
>            Assignee: Aleksey Yeschenko
>            Priority: Minor
>             Fix For: 2.2.x, 3.0.x, 3.x
>
>
> It was [decided|https://issues.apache.org/jira/browse/CASSANDRA-7744?focusedCommentId=14095472&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14095472]
that we'd be recalculating {{is_dense}} for table updates coming from Thrift on every change.
However, due to some oversight, {{is_dense}} can only go from {{false}} to {{true}}. Once
dense, even adding a {{REGULAR}} column will not reset {{is_dense}} back to {{false}}.
> The recalculation fails because no matter what happens, we never remove the auto-generated
{{CLUSTERING}} and {{COMPACT_VALUE}} columns of a dense table.
> Which ultimately leads to the issue on 2.2 to 3.0 upgrade (see CASSANDRA-11315).
> What we should do is remove the special-case for Thrift in {{LegacySchemaTables::makeUpdateTableMutation}}
and correct the logic in {{ThriftConversion::internalFromThrift}} to remove those columns
when going from dense to sparse.
> This is not enough to fix CASSANDRA-11315, however, as we need to handle pre-patch upgrades,
and upgrades from 2.1. Fixing it in 2.2 means a) getting proper schema from {{DESCRIBE}} now
and b) using the more efficient {{SparseCellNameType}} when you add columns.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Mime
View raw message