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

Joseph Lynch commented on CASSANDRA-14303:
------------------------------------------

[~jjirsa] thank you for the feedback. Do you want me to create a different 
ticket for the style fixes or would you like me to just post another patch 
here? Also if you could indicate what the style issues are I'd be happy to fix 
them, as far as I could tell the patch satisfied the [style 
guide|http://cassandra.apache.org/doc/latest/development/code_style.html].
{quote}Going from simplestrategy|replication_factor=3 to 
nts|replication_factor=3 silently multiplies the replica count and violates 
consistency if you have more than one DC.

(Yes, it's true that going from simple strategy to NTS is almost always going 
to violate consistency, but we don't hide that fact behind an auto-expansion of 
old arguments).
{quote}
I see your concern (and I communicated this behavior in the comment above as 
well as included a unit test of the behavior) but I do not agree this is any 
different from the status quo and is in my opinion the least surprising thing 
we could do. Indeed altering from SS to NTS, as you mention, will always 
violate consistency and require a repair; it can also lose data if you don't 
include replicas in each datacenter. Furthermore this operation is not silent, 
the user is warned equally as they were before what has happened when they do 
this:
{noformat}
cqlsh> CREATE KEYSPACE test_r WITH replication = {'class': 'SimpleStrategy', 
'replication_factor': 1};
cqlsh> ALTER KEYSPACE test_r WITH replication = {'class': 
'NetworkTopologyStrategy'};

Warnings :
When increasing replication factor you need to run a full (-full) repair to 
distribute the data.

cqlsh> DESCRIBE KEYSPACE test_r 

CREATE KEYSPACE test_r WITH replication = {'class': 'NetworkTopologyStrategy', 
'dc1': '1', 'dc2': '1'}  AND durable_writes = true;

# This is the same as if they had manually specified 1

cqlsh> ALTER KEYSPACE test_r WITH replication = {'class': 
'NetworkTopologyStrategy', 'dc1': 1, 'dc2': 1};

Warnings :
When increasing replication factor you need to run a full (-full) repair to 
distribute the data.

cqlsh> DESCRIBE KEYSPACE test_r 

CREATE KEYSPACE test_r WITH replication = {'class': 'NetworkTopologyStrategy', 
'dc1': '1', 'dc2': '1'}  AND durable_writes = true;
{noformat}
This user interface, in my opinion, follows the principle of least surprise and 
recommends the correct action (run repair). The other viable option is to 
error, in which case users are just going to type manually what I did in the 
patch anyways or worse yet not type all the datacenters and lose data.

I'm happy to error as we did before if that assuages your concern (although I 
don't agree), but I think the core problem is that we ship the database with SS 
tables instead of NTS tables, and don't support safe alters from SS to NTS. For 
example we can see that any other SS->NTS strategy other than expanding out to 
all datacenters just silently loses data which is ... probably worse:
{noformat}
cqlsh:test_r> ALTER KEYSPACE test_r WITH replication = {'class': 
'SimpleStrategy', 'replication_factor': 1}
          ... ;
cqlsh:test_r> DESCRIBE CLUSTER  

Cluster: cassandra_trunk_nts
Partitioner: Murmur3Partitioner

Range ownership:
                    -9223372036854775808  [127.0.0.1]
                                       0  [127.0.0.2]
                    -9223372036854775708  [127.0.0.3]
                                     100  [127.0.0.4]

cqlsh:test_r> ALTER KEYSPACE test_r WITH replication = {'class': 
'NetworkTopologyStrategy'} ;               

Warnings :
When increasing replication factor you need to run a full (-full) repair to 
distribute the data.

cqlsh:test_r> DESCRIBE CLUSTER  

Cluster: cassandra_trunk_nts
Partitioner: Murmur3Partitioner

Range ownership:
                    -9223372036854775808  [127.0.0.3, 127.0.0.1]
                                       0  [127.0.0.4, 127.0.0.2]
                    -9223372036854775708  [127.0.0.3, 127.0.0.2]
                                     100  [127.0.0.4, 127.0.0.1]
# ^^ no data loss, just needs repair

cqlsh:test_r> ALTER KEYSPACE test_r WITH replication = {'class': 
'NetworkTopologyStrategy', 'dc1': 1} ;
cqlsh:test_r> DESCRIBE CLUSTER  

Cluster: cassandra_trunk_nts
Partitioner: Murmur3Partitioner

Range ownership:
                    -9223372036854775808  [127.0.0.1]
                                       0  [127.0.0.2]
                    -9223372036854775708  [127.0.0.2]
                                     100  [127.0.0.1]

# ^^ User probably just lost data{noformat}
I can explore other topologies but at least in this case the only thing that 
doesn't lose data is to to change SS:1 -> NTS:(1 per dc)... which is what the 
patch does when the user explicitly asks to move from SS to NTS (as in we 
didn't do it without their knowledge).

> Auto-expand replication_factor for NetworkTopologyStrategy
> ----------------------------------------------------------
>
>                 Key: CASSANDRA-14303
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-14303
>             Project: Cassandra
>          Issue Type: Improvement
>          Components: Configuration
>            Reporter: Joseph Lynch
>            Assignee: Joseph Lynch
>            Priority: Minor
>              Labels: 4.0-feature-freeze-review-requested
>             Fix For: 4.x
>
>
> Right now when creating a keyspace with {{NetworkTopologyStrategy}} the user 
> has to manually specify the datacenters they want their data replicated to 
> with parameters, e.g.:
> {noformat}
>  CREATE KEYSPACE test WITH replication = {'class': 'NetworkTopologyStrategy', 
> 'dc1': 3, 'dc2': 3}{noformat}
> This is a poor user interface because it requires the creator of the keyspace 
> (typically a developer) to know the layout of the Cassandra cluster (which 
> may or may not be controlled by them). Also, at least in my experience, folks 
> typo the datacenters _all_ the time. To work around this I see a number of 
> users creating automation around this where the automation describes the 
> Cassandra cluster and automatically expands out to all the dcs that Cassandra 
> knows about. Why can't Cassandra just do this for us, re-using the previously 
> forbidden {{replication_factor}} option (for backwards compatibility):
> {noformat}
>  CREATE KEYSPACE test WITH replication = {'class': 'NetworkTopologyStrategy', 
> 'replication_factor': 3}{noformat}
> This would automatically replicate this Keyspace to all datacenters that are 
> present in the cluster. If you need to _override_ the default you could 
> supply a datacenter name, e.g.:
> {noformat}
> > CREATE KEYSPACE test WITH replication = {'class': 
> > 'NetworkTopologyStrategy', 'replication_factor': 3, 'dc1': 2}
> > DESCRIBE KEYSPACE test
> CREATE KEYSPACE test WITH replication = {'class': 'NetworkTopologyStrategy', 
> 'dc1': '2', 'dc2': 3} AND durable_writes = true;
> {noformat}
> On the implementation side I think this may be reasonably straightforward to 
> do an auto-expansion at the time of keyspace creation (or alter), where the 
> above would automatically expand to list out the datacenters. We could allow 
> this to be recomputed whenever an AlterKeyspaceStatement runs so that to add 
> datacenters you would just run:
> {noformat}
> ALTER KEYSPACE test WITH replication = {'class': 'NetworkTopologyStrategy', 
> 'replication_factor': 3}{noformat}
> and this would check that if the dc's in the current schema are different you 
> add in the new ones (_for safety reasons we'd never remove non explicitly 
> supplied zero dcs when auto-generating dcs_). Removing a datacenter becomes 
> an alter that includes an override for the dc you want to remove (or of 
> course you can always not use the auto-expansion and just use the old way):
> {noformat}
> // Tell it explicitly not to replicate to dc2
> > ALTER KEYSPACE test WITH replication = {'class': 'NetworkTopologyStrategy', 
> > 'replication_factor': 3, 'dc2': 0}
> > DESCRIBE KEYSPACE test
> CREATE KEYSPACE test WITH replication = {'class': 'NetworkTopologyStrategy', 
> 'dc1': '3'} AND durable_writes = true;{noformat}



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to