You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by rhtyd <gi...@git.apache.org> on 2016/04/25 11:26:52 UTC

[GitHub] cloudstack pull request: engine/schema: fix upgrade path to work w...

GitHub user rhtyd opened a pull request:

    https://github.com/apache/cloudstack/pull/1517

    engine/schema: fix upgrade path to work with MySQL 5.7

    Found this issue when using MySQL 5.7 with Ubuntu 16.04. The upgrade path fix removes an invalid `IGNORE` param that is deprecated now, in the upgrade path we run the alter statement to add an index only if it does not exist so we're good.
    
    For MySQL 5.7, we'll also need to update the docs at some point to include `server-id` along with other parameters. Some of the SQL statements used throughout engine/schema don't adhere to SQL 99 standard which is enforced by default in MySQL 5.7, therefore the following sql-mode (for backward compatibility with mysql 5.6 modes) will be necessary for anyone willing to use MySQL 5.7 (until we fix codebase wide raw and generated sql statements to be SQL99 compliant):
    
    sql-mode="STRICT_TRANS_TABLES,NO_ENGINE_SUBSTITUTION,ERROR_FOR_DIVISION_BY_ZERO,NO_ZERO_DATE,NO_ZERO_IN_DATE,NO_ENGINE_SUBSTITUTION"
    server-id   = 1
    innodb_rollback_on_timeout=1
    innodb_lock_wait_timeout=600
    max_connections=350
    log-bin=mysql-bin
    binlog-format = 'ROW'
    
    /cc @swill @jburwell @agneya2001 @wido @DaanHoogland  and others

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/shapeblue/cloudstack mysql-5.7-upgradefix

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/cloudstack/pull/1517.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #1517
    
----
commit c63ea0a240df0c5af2802b424d70083bbfd0e919
Author: Rohit Yadav <ro...@shapeblue.com>
Date:   2016-04-25T09:17:22Z

    engine/schema: fix upgrade path to work with MySQL 5.7
    
    Found this issue when using MySQL 5.7 with Ubuntu 16.04 with following settings:
    
    sql-mode="STRICT_TRANS_TABLES,NO_ENGINE_SUBSTITUTION,ERROR_FOR_DIVISION_BY_ZERO,NO_ZERO_DATE,NO_ZERO_IN_DATE,NO_ENGINE_SUBSTITUTION"
    server-id   = 1
    innodb_rollback_on_timeout=1
    innodb_lock_wait_timeout=600
    max_connections=350
    log-bin=mysql-bin
    binlog-format = 'ROW'
    
    Signed-off-by: Rohit Yadav <ro...@shapeblue.com>

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: engine/schema: fix upgrade path to work w...

Posted by wido <gi...@git.apache.org>.
Github user wido commented on the pull request:

    https://github.com/apache/cloudstack/pull/1517#issuecomment-214852970
  
    Ah, yes, I see @rhtyd !
    
    LGTM


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: engine/schema: fix upgrade path to work w...

Posted by koushik-das <gi...@git.apache.org>.
Github user koushik-das commented on the pull request:

    https://github.com/apache/cloudstack/pull/1517#issuecomment-215038164
  
    @rhtyd Thats right, setup will anyway do the upgrades.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: engine/schema: fix upgrade path to work w...

Posted by rhtyd <gi...@git.apache.org>.
Github user rhtyd commented on the pull request:

    https://github.com/apache/cloudstack/pull/1517#issuecomment-214617500
  
    @jburwell I've not tested, but if it is using/enforcing SQL99 it should fail too (or it could be mysql 5.7.4+ issue);
    
    "As of MySQL 5.7.4, the IGNORE clause for ALTER TABLE is removed and its use produces an error."
    from http://dev.mysql.com/doc/refman/5.7/en/alter-table.html
    



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: engine/schema: fix upgrade path to work w...

Posted by rhtyd <gi...@git.apache.org>.
Github user rhtyd commented on the pull request:

    https://github.com/apache/cloudstack/pull/1517#issuecomment-214229683
  
    I found this article by Oracle MySQL team useful: https://www.digitalocean.com/community/tutorials/how-to-prepare-for-your-mysql-5-7-upgrade


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: engine/schema: fix upgrade path to work w...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/cloudstack/pull/1517


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: engine/schema: fix upgrade path to work w...

Posted by swill <gi...@git.apache.org>.
Github user swill commented on the pull request:

    https://github.com/apache/cloudstack/pull/1517#issuecomment-215077801
  
    Thanks for the validation discussion.  I think this is ready...


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: engine/schema: fix upgrade path to work w...

Posted by wido <gi...@git.apache.org>.
Github user wido commented on the pull request:

    https://github.com/apache/cloudstack/pull/1517#issuecomment-214734875
  
    So what happens if the Index already exists?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: engine/schema: fix upgrade path to work w...

Posted by rhtyd <gi...@git.apache.org>.
Github user rhtyd commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/1517#discussion_r61145609
  
    --- Diff: engine/schema/src/com/cloud/upgrade/dao/Upgrade410to420.java ---
    @@ -1297,7 +1297,7 @@ private void addHostDetailsIndex(Connection conn) {
                         s_logger.debug("Index already exists on host_details - not adding new one");
    --- End diff --
    
    @wido the index is not added, if you look at the code above ^^ the if statement to check if index already exists with given key name and it only adds the index if it does not exist otherwise skips.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: engine/schema: fix upgrade path to work w...

Posted by koushik-das <gi...@git.apache.org>.
Github user koushik-das commented on the pull request:

    https://github.com/apache/cloudstack/pull/1517#issuecomment-215017916
  
    Code changes LGTM.
    @swill The change is in upgrade code and CI won't be able to test it. So I think this can be merged.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: engine/schema: fix upgrade path to work w...

Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on the pull request:

    https://github.com/apache/cloudstack/pull/1517#issuecomment-214574233
  
    Do these instructions apply to MariaDB as well?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: engine/schema: fix upgrade path to work w...

Posted by rhtyd <gi...@git.apache.org>.
Github user rhtyd commented on the pull request:

    https://github.com/apache/cloudstack/pull/1517#issuecomment-215024033
  
    Thanks @koushik-das 
    CI will be able to test it though as cloudstack-setup-database deploys 4.0 schema and ACS mgmt server upgrades it or DatabaseUpgrader runs the upgrade path as well. Travis deployed database and is green confirms upgrade path fix does not break with existing MySQL 5.6 or less version.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---