[jira] [Commented] (CASSANDRA-11502) Fix denseness and column metadata updates coming from Thrift

2016-04-27 Thread Aleksey Yeschenko (JIRA)

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

Aleksey Yeschenko commented on CASSANDRA-11502:
---

Committed as 
[e5c40278001bf3a9582085a58941e5f4765f118c|https://github.com/apache/cassandra/commit/e5c40278001bf3a9582085a58941e5f4765f118c]
 to 2.2 and merged with 3.0 and trunk, thanks. Did some manual testing w/ 
cqlsh/nodetool to make sure sparse CFs w/ clustering columns don't pass 
{{isThriftCompatibleTest()}}, and it seems like we are all good.

> 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=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)


[jira] [Commented] (CASSANDRA-11502) Fix denseness and column metadata updates coming from Thrift

2016-04-27 Thread Aleksey Yeschenko (JIRA)

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

Aleksey Yeschenko commented on CASSANDRA-11502:
---

bq. I help someone a few days ago with an upgrade problem and was able to do an 
update on a CQL table, but that was on some 2.0 version so must have been on 
some version from before we introduced that.

Do you have that table schema handy? I might as well check if the check for 
that fails in 2.1+ and open a new ticket if so.

> 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=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)


[jira] [Commented] (CASSANDRA-11502) Fix denseness and column metadata updates coming from Thrift

2016-04-27 Thread Sylvain Lebresne (JIRA)

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

Sylvain Lebresne commented on CASSANDRA-11502:
--

bq. but I, instead, feel more paranoid about leaving it in

Fair enough, I'm good getting rid of it.

bq. I think we should be safe here b/c of the {{isThriftCompatible()}} guard in 
{{CassandraServer::system_update_column_family()}}.

You're right. I got confused because I help someone a few days ago with an 
upgrade problem and was able to do an update on a CQL table, but that was on 
some 2.0 version so must have been on some version from before we introduced 
that. Would still be great to double check but +1 on the patch in any 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=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)


[jira] [Commented] (CASSANDRA-11502) Fix denseness and column metadata updates coming from Thrift

2016-04-26 Thread Aleksey Yeschenko (JIRA)

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

Aleksey Yeschenko commented on CASSANDRA-11502:
---

Re 1: If you leave it alone, then {{is_dense}} will forever be stuck at true 
and never recomputed. This is breaking upgrading to 3.0 (now, we'll still need 
to address this in 3.0, for users upgrading from C* that don't have this patch 
in, but that's what CASSANDRA-11315 is for). Now, I 100% get the caution, but 
I, instead, feel more paranoid about leaving it in - having the logic 
duplicated in two places has already led to this one bug. And, a lot less 
critically, in preparation for Thrift removal I want to push any Thrift code as 
close to the edges as possible.

Re 2: I *think* we should be safe here b/c of the {{isThriftCompatible()}} 
guard in {{CassandraServer::system_update_column_family()}}. If we aren't, then 
we have a bug in {{update_column_family()}}. I'll double check.



> 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=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)


[jira] [Commented] (CASSANDRA-11502) Fix denseness and column metadata updates coming from Thrift

2016-04-26 Thread Sylvain Lebresne (JIRA)

[ 
https://issues.apache.org/jira/browse/CASSANDRA-11502?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=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=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)


[jira] [Commented] (CASSANDRA-11502) Fix denseness and column metadata updates coming from Thrift

2016-04-07 Thread Aleksey Yeschenko (JIRA)

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

Aleksey Yeschenko commented on CASSANDRA-11502:
---

||branch||testall||dtest||
|[11502-2.2|https://github.com/iamaleksey/cassandra/tree/11502-2.2]|[testall|http://cassci.datastax.com/view/Dev/view/iamaleksey/job/iamaleksey-11502-2.2-testall]|[dtest|http://cassci.datastax.com/view/Dev/view/iamaleksey/job/iamaleksey-11502-2.2-dtest]|
|[11502-3.0|https://github.com/iamaleksey/cassandra/tree/11502-3.0]|[testall|http://cassci.datastax.com/view/Dev/view/iamaleksey/job/iamaleksey-11502-3.0-testall]|[dtest|http://cassci.datastax.com/view/Dev/view/iamaleksey/job/iamaleksey-11502-3.0-dtest]|

2.2 version makes sure we recalculate and remove redundant column. 3.0 versions 
preserves denseness, but cleans up unnecessary Thrift special casing from 
{{SchemaKeyspace::makeUpdateTableMutation}}. It's unnecessary because despite 
Thrift not knowing about non regular/static columns, 
{{ThriftConversion::internalFromThrift}} will include them from the current 
CFM, so we are not going to lose anything.

> 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=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)