You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by "Jonathan Ellis (Created) (JIRA)" <ji...@apache.org> on 2012/03/08 08:05:04 UTC

[jira] [Created] (CASSANDRA-4017) Unify migrations

Unify migrations
----------------

                 Key: CASSANDRA-4017
                 URL: https://issues.apache.org/jira/browse/CASSANDRA-4017
             Project: Cassandra
          Issue Type: Improvement
          Components: Core
    Affects Versions: 1.1.0
            Reporter: Jonathan Ellis
             Fix For: 1.1.0


Now that we can send a schema as a RowMutation, there's no need to keep separate add/drop/update migration classes around.  Let's just send the schema to our counterparts and let them figure out what changed.  Currently we have "figure out what changed" code to both generate migrations on the sender, and for application on the target, which adds complexity.

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

        

[jira] [Commented] (CASSANDRA-4017) Unify migrations

Posted by "Jonathan Ellis (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/CASSANDRA-4017?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13230552#comment-13230552 ] 

Jonathan Ellis commented on CASSANDRA-4017:
-------------------------------------------

bq. More precisely, SchemaLoader was not using the migration path, but only loading the schema in-memory. Because the patch removes the local special case, this was breaking DefsTest. 

Why can't we still call {{Schema.load(KSMetaData)}} directly?
                
> Unify migrations
> ----------------
>
>                 Key: CASSANDRA-4017
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-4017
>             Project: Cassandra
>          Issue Type: Improvement
>          Components: Core
>    Affects Versions: 1.1.0
>            Reporter: Jonathan Ellis
>            Assignee: Sylvain Lebresne
>             Fix For: 1.1.0
>
>
> Now that we can send a schema as a RowMutation, there's no need to keep separate add/drop/update migration classes around.  Let's just send the schema to our counterparts and let them figure out what changed.  Currently we have "figure out what changed" code to both generate migrations on the sender, and for application on the target, which adds complexity.

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

        

[jira] [Commented] (CASSANDRA-4017) Unify migrations

Posted by "Jonathan Ellis (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/CASSANDRA-4017?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13230596#comment-13230596 ] 

Jonathan Ellis commented on CASSANDRA-4017:
-------------------------------------------

Maybe we can tackle it from this direction, then:

bq. now that means that SchemaLoader needs to be merged with CleanupHelper (otherwise, since the cleanup was happening with the schema loading, the loading was conflicting with the schema of the previous test)

What I did over on CASSANDRA-3967 v2 was to make CleanupHelper manually call into SchemaLoader explicitly rather than leaving it up to the BeforeClass annotations as to which runs first.

We could do something similar as a one-off for MoveTest: instead of extending CleanupHelper and giving up that control, it could call into it manually once it's set the partitioner the way it wants.
                
> Unify migrations
> ----------------
>
>                 Key: CASSANDRA-4017
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-4017
>             Project: Cassandra
>          Issue Type: Improvement
>          Components: Core
>    Affects Versions: 1.1.0
>            Reporter: Jonathan Ellis
>            Assignee: Sylvain Lebresne
>             Fix For: 1.1.0
>
>
> Now that we can send a schema as a RowMutation, there's no need to keep separate add/drop/update migration classes around.  Let's just send the schema to our counterparts and let them figure out what changed.  Currently we have "figure out what changed" code to both generate migrations on the sender, and for application on the target, which adds complexity.

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

        

[jira] [Assigned] (CASSANDRA-4017) Unify migrations

Posted by "Sylvain Lebresne (Assigned) (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/CASSANDRA-4017?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Sylvain Lebresne reassigned CASSANDRA-4017:
-------------------------------------------

    Assignee: Sylvain Lebresne
    
> Unify migrations
> ----------------
>
>                 Key: CASSANDRA-4017
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-4017
>             Project: Cassandra
>          Issue Type: Improvement
>          Components: Core
>    Affects Versions: 1.1.0
>            Reporter: Jonathan Ellis
>            Assignee: Sylvain Lebresne
>             Fix For: 1.1.0
>
>
> Now that we can send a schema as a RowMutation, there's no need to keep separate add/drop/update migration classes around.  Let's just send the schema to our counterparts and let them figure out what changed.  Currently we have "figure out what changed" code to both generate migrations on the sender, and for application on the target, which adds complexity.

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

        

[jira] [Commented] (CASSANDRA-4017) Unify migrations

Posted by "Sylvain Lebresne (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/CASSANDRA-4017?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13230578#comment-13230578 ] 

Sylvain Lebresne commented on CASSANDRA-4017:
---------------------------------------------

The reason is that when we compute for a migration what has changed, we do that based on the on-disk schema. So calling only Schema.load() means that the on-disk schema is empty and thus some test of DefsTast that try to modify a CF are not recognized as updates but as add and fail (since the cf exists in memory).

Now, an option could be to compute 'what has changed' based on the in-memory schema. However, even if we do that (I honestly don't have much argument for doing it or not doing it, but pleasing the unit tests seems like the worst possible reason to do it), I think it is still nice to use the normal path (i.e. not only load in memory with Schema.load()) to load the test schema because that means we exercise that code which is a good thing and make our unit tests less 'fake'. I'll note for instance that the patch fix a small bug in the current code that serialize/deserialize schema that was caught once I started writing the test schema on-disk but wasn't previously.
                
> Unify migrations
> ----------------
>
>                 Key: CASSANDRA-4017
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-4017
>             Project: Cassandra
>          Issue Type: Improvement
>          Components: Core
>    Affects Versions: 1.1.0
>            Reporter: Jonathan Ellis
>            Assignee: Sylvain Lebresne
>             Fix For: 1.1.0
>
>
> Now that we can send a schema as a RowMutation, there's no need to keep separate add/drop/update migration classes around.  Let's just send the schema to our counterparts and let them figure out what changed.  Currently we have "figure out what changed" code to both generate migrations on the sender, and for application on the target, which adds complexity.

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

        

[jira] [Resolved] (CASSANDRA-4017) Unify migrations

Posted by "Sylvain Lebresne (Resolved) (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/CASSANDRA-4017?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Sylvain Lebresne resolved CASSANDRA-4017.
-----------------------------------------

    Resolution: Fixed

Committed, thanks
                
> Unify migrations
> ----------------
>
>                 Key: CASSANDRA-4017
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-4017
>             Project: Cassandra
>          Issue Type: Improvement
>          Components: Core
>    Affects Versions: 1.1.0
>            Reporter: Jonathan Ellis
>            Assignee: Sylvain Lebresne
>             Fix For: 1.1.0
>
>
> Now that we can send a schema as a RowMutation, there's no need to keep separate add/drop/update migration classes around.  Let's just send the schema to our counterparts and let them figure out what changed.  Currently we have "figure out what changed" code to both generate migrations on the sender, and for application on the target, which adds complexity.

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

        

[jira] [Commented] (CASSANDRA-4017) Unify migrations

Posted by "Jonathan Ellis (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/CASSANDRA-4017?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13225059#comment-13225059 ] 

Jonathan Ellis commented on CASSANDRA-4017:
-------------------------------------------

Tagging 1.1.0 since this may change the schema communication protocol, and I don't want to break things twice...
                
> Unify migrations
> ----------------
>
>                 Key: CASSANDRA-4017
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-4017
>             Project: Cassandra
>          Issue Type: Improvement
>          Components: Core
>    Affects Versions: 1.1.0
>            Reporter: Jonathan Ellis
>             Fix For: 1.1.0
>
>
> Now that we can send a schema as a RowMutation, there's no need to keep separate add/drop/update migration classes around.  Let's just send the schema to our counterparts and let them figure out what changed.  Currently we have "figure out what changed" code to both generate migrations on the sender, and for application on the target, which adds complexity.

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

        

[jira] [Updated] (CASSANDRA-4017) Unify migrations

Posted by "Pavel Yaskevich (Updated) (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/CASSANDRA-4017?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Pavel Yaskevich updated CASSANDRA-4017:
---------------------------------------

    Reviewer: xedin
    
> Unify migrations
> ----------------
>
>                 Key: CASSANDRA-4017
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-4017
>             Project: Cassandra
>          Issue Type: Improvement
>          Components: Core
>    Affects Versions: 1.1.0
>            Reporter: Jonathan Ellis
>            Assignee: Sylvain Lebresne
>             Fix For: 1.1.0
>
>
> Now that we can send a schema as a RowMutation, there's no need to keep separate add/drop/update migration classes around.  Let's just send the schema to our counterparts and let them figure out what changed.  Currently we have "figure out what changed" code to both generate migrations on the sender, and for application on the target, which adds complexity.

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

        

[jira] [Commented] (CASSANDRA-4017) Unify migrations

Posted by "Sylvain Lebresne (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/CASSANDRA-4017?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13230155#comment-13230155 ] 

Sylvain Lebresne commented on CASSANDRA-4017:
---------------------------------------------

I've put some initial patch at https://github.com/pcmanus/cassandra/commits/4017.

As described above, the main idea is to unify code between what happens on the initial node applying the migration and the rest of the nodes. With this patch, when a schema change has to happen, we generate the RowMutation corresponding and send it to all nodes (including the localhost) and all the node use that to figure out what changed and how to update in-memory structures. The patch removes the Migration classes (that weren't doing much anyway) and moves a bit a code around to make it easier to follow a migration path.

Sadly, this ended up posing problem with the unit tests. More precisely, SchemaLoader was not using the migration path, but only loading the schema in-memory. Because the patch removes the local special case, this was breaking DefsTest. The solution was to make SchemaLoader use actual migrations (to get stuffs written to disk). But now that means that SchemaLoader needs to be merged with CleanupHelper (otherwise, since the cleanup was happening with the schema loading, the loading was conflicting with the schema of the previous test).

When that's fixed (which the patch does), MoveTest still broke with a ClassCastException between BytesToken and BigIntegerToken. The reason is that MoveTest use {{ss.setPartitionerUnsafe}} to set the RandomPartitioner. But it does that *after* the schema has been loaded. But now the schema happens to trigger flushing of system tables. Those are flushed with the order preserving partitioner and later conflict with the sstable created after the partitioner change. I'm not sure what the best fix is for this and so the patch doesn't fix that failure. Maybe we could just make MoveTest keep the test partitioner.  But I suppose the intention was to test the random partitioner, so ....

                
> Unify migrations
> ----------------
>
>                 Key: CASSANDRA-4017
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-4017
>             Project: Cassandra
>          Issue Type: Improvement
>          Components: Core
>    Affects Versions: 1.1.0
>            Reporter: Jonathan Ellis
>            Assignee: Sylvain Lebresne
>             Fix For: 1.1.0
>
>
> Now that we can send a schema as a RowMutation, there's no need to keep separate add/drop/update migration classes around.  Let's just send the schema to our counterparts and let them figure out what changed.  Currently we have "figure out what changed" code to both generate migrations on the sender, and for application on the target, which adds complexity.

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

        

[jira] [Commented] (CASSANDRA-4017) Unify migrations

Posted by "Pavel Yaskevich (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/CASSANDRA-4017?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13231612#comment-13231612 ] 

Pavel Yaskevich commented on CASSANDRA-4017:
--------------------------------------------

+1
                
> Unify migrations
> ----------------
>
>                 Key: CASSANDRA-4017
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-4017
>             Project: Cassandra
>          Issue Type: Improvement
>          Components: Core
>    Affects Versions: 1.1.0
>            Reporter: Jonathan Ellis
>            Assignee: Sylvain Lebresne
>             Fix For: 1.1.0
>
>
> Now that we can send a schema as a RowMutation, there's no need to keep separate add/drop/update migration classes around.  Let's just send the schema to our counterparts and let them figure out what changed.  Currently we have "figure out what changed" code to both generate migrations on the sender, and for application on the target, which adds complexity.

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

        

[jira] [Commented] (CASSANDRA-4017) Unify migrations

Posted by "Sylvain Lebresne (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/CASSANDRA-4017?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13231021#comment-13231021 ] 

Sylvain Lebresne commented on CASSANDRA-4017:
---------------------------------------------

bq. We could do something similar as a one-off for MoveTest

Good idea. I've push 2 new commits to my github branch (https://github.com/pcmanus/cassandra/commits/4017). The first one fixes the unit tests using the idea above. The second one is just a small bug fix of the previous patch. I've checked that with all this, both unit and system tests are passing (and the cql3 tests I have).
                
> Unify migrations
> ----------------
>
>                 Key: CASSANDRA-4017
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-4017
>             Project: Cassandra
>          Issue Type: Improvement
>          Components: Core
>    Affects Versions: 1.1.0
>            Reporter: Jonathan Ellis
>            Assignee: Sylvain Lebresne
>             Fix For: 1.1.0
>
>
> Now that we can send a schema as a RowMutation, there's no need to keep separate add/drop/update migration classes around.  Let's just send the schema to our counterparts and let them figure out what changed.  Currently we have "figure out what changed" code to both generate migrations on the sender, and for application on the target, which adds complexity.

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