cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Sylvain Lebresne (Commented) (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (CASSANDRA-4093) schema_* CFs do not respect column comparator which leads to CLI commands failure.
Date Fri, 30 Mar 2012 17:00:31 GMT

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

Sylvain Lebresne commented on CASSANDRA-4093:
---------------------------------------------

bq. The thing about backwards-compatibility hacks is you tend to get stuck with them.

I suppose it depends of ones own definition of a hack, but I really don't consider this patch
as a hack. As soon as we consider CT to be first class citizen, adding the ability to specify
to which component a ColumnDefinition applies (which is all the patch does) feels like a rather
natural extension to me. And as said, if we decide to allow secondary indexes on any component
of a CT, which I though was something we wanted ultimately, we'll have to add that componentIndex
information. So breaking compatibility to avoid adding a simple info to ColumnDefinition that
we'll very likely end up adding anyway later feels like the wrong choice to me.

bq. We've made huge strides towards simplifying this in CQL3 and composite PKs. I don't think
we can afford to dilute this with footnotes about how you can escape hatch back to the old
world

I don't understand. As of the attached patch, there is 0 footnotes to add to CQL3. Currently
it's pretty much *exclusively* a thrift backward-compatibility patch. If you're referring
to me saying that we could later consider allowing index on a full composite in CQL3, then
I'm sorry, let's forget about that for now. This patch does not force us at all to do that.

bq. dragging in all our old legacy baggage again. Put another way: it's time to bury "wtf
is a supercolumn" once and for all.

I don't understand what you're referring to. Again, what this patch does is recognizing that
CT are now first class citizen and thus that it makes sense that a ColumnDefinition may refer
to any of the component. And while it is true that we don't *yet* use that generalization,
it *trivially* allows us to support backward compatibility for thrift, hence the idea of doing
that generalization now. I don't see where this patch drag *any* old legacy baggage so I'd
love to get more precision on what you're referring to exactly. 

bq. Of our hundreds of deployments of 0.8 and 1.0, I can live with one or two needing to wait
for 1.1.2 or whatever to upgrade.

But that is *not* what will happen. Anyone that use an index on a CT currently won't be able
to upgrade *ever*. Because their index will just be not be supported anymore. If they upgrade,
C* will crash. Seriously, they will be completely screwed. Their only option will be some
complex manual data model migration which will be impossible to do without any downtime for
the application (since no version will both support their existing index *and* the new possibly-introduced-in-1.2
indexing of the the last component of a CT).

bq. TLDR: backwards compatibility in this specific case is high cost, low benefit.

I couldn't disagree more. I still don't see what this high cost is. I wrote the patch and
I just don't see at all the high cost. The patch is fairly trivial (it's not small in size
but for a good part because it include thrift generated code) and has barely any user side
visibility. More precisely, the only visibility it has is on the thrift side, and even then
it's an optional field whose default is the right one for the thrift side. On the other side,
the benefit is huge imo because it means we don't screw up the people that use C* in original
ways.
                
> schema_* CFs do not respect column comparator which leads to CLI commands failure.
> ----------------------------------------------------------------------------------
>
>                 Key: CASSANDRA-4093
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-4093
>             Project: Cassandra
>          Issue Type: Bug
>          Components: Tools
>    Affects Versions: 1.1.0
>            Reporter: Dave Brosius
>            Assignee: Sylvain Lebresne
>             Fix For: 1.1.0
>
>         Attachments: 4093.txt, CASSANDRA-4093-CD-changes.patch
>
>
> ColumnDefinition.{ascii, utf8, bool, ...} static methods used to initialize schema_*
CFs column_metadata do not respect CF comparator and use ByteBufferUtil.bytes(...) for column
names which creates problems in CLI and probably in other places.
> The CompositeType validator throws exception on first column
> String columnName = columnNameValidator.getString(columnDef.name);
> Because it appears the composite type length header is wrong (25455)
> AbstractCompositeType.getWithShortLength
> java.lang.IllegalArgumentException
> 	at java.nio.Buffer.limit(Buffer.java:247)
> 	at org.apache.cassandra.db.marshal.AbstractCompositeType.getBytes(AbstractCompositeType.java:50)
> 	at org.apache.cassandra.db.marshal.AbstractCompositeType.getWithShortLength(AbstractCompositeType.java:59)
> 	at org.apache.cassandra.db.marshal.AbstractCompositeType.getString(AbstractCompositeType.java:139)
> 	at org.apache.cassandra.cli.CliClient.describeColumnFamily(CliClient.java:2046)
> 	at org.apache.cassandra.cli.CliClient.describeKeySpace(CliClient.java:1969)
> 	at org.apache.cassandra.cli.CliClient.executeShowKeySpaces(CliClient.java:1574)
> (seen in trunk)

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

Mime
View raw message