You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by nvazquez <gi...@git.apache.org> on 2016/04/07 20:22:29 UTC

[GitHub] cloudstack pull request: CLOUDSTACK-9340: General DB Optimization

GitHub user nvazquez opened a pull request:

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

    CLOUDSTACK-9340: General DB Optimization

    ## Description
    In some production environments there were being experimented delays in most of the jobs. A search for DB optimization was taken and some deficiencies were discovered, we can group them in 4 groups:
    * Incorrect PRIMARY key
    * Duplicate PRIMARY KEY
    * Missing indexes (Add indexes to avoid full table scans)
    * Some views query (Change view to improve account retrieval speed)

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

    $ git pull https://github.com/nvazquez/cloudstack graldboptimization

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

    https://github.com/apache/cloudstack/pull/1466.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 #1466
    
----
commit 248677437ac8d1369bf353683a64b21c613a62e7
Author: nvazquez <ni...@gmail.com>
Date:   2016-04-07T18:20:48Z

    CLOUDSTACK-9340: General DB Optimization

----


---
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: CLOUDSTACK-9340: General DB Optimization

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

    https://github.com/apache/cloudstack/pull/1466#issuecomment-208393777
  
    @serg38 thanks, now it is clear to me.
    Thanks for the great job.
    BTW: it seems that travis is with problems again.


---
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: CLOUDSTACK-9340: General DB Optimization

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

    https://github.com/apache/cloudstack/pull/1466#issuecomment-207434839
  
    Hi @rafaelweingartner I executed sql in my env and succeeded, is there a way to trace which line failed?


---
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: CLOUDSTACK-9340: General DB Optimization

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

    https://github.com/apache/cloudstack/pull/1466#issuecomment-207471025
  
    @nvazquez did it fail with the problem?
    for me it is showing as executing and not failed.


---
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: CLOUDSTACK-9340: General DB Optimization

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

    https://github.com/apache/cloudstack/pull/1466#issuecomment-207550783
  
    Thanks @swill, I just wanted to remove all my commits for Travis test to run without my changes, but doing that closed the pr. Now I pushed a commit including a comment line just for Travis test to run without my changes. I plan to include my changes incrementally to see where the failure is


---
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: CLOUDSTACK-9340: General DB Optimization

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

    https://github.com/apache/cloudstack/pull/1466#discussion_r59146860
  
    --- Diff: setup/db/db/schema-481to490.sql ---
    @@ -413,3 +413,89 @@ VIEW `user_vm_view` AS
     
     -- Add cluster.storage.operations.exclude property
     INSERT INTO `cloud`.`configuration` (`category`, `instance`, `component`, `name`, `description`, `default_value`, `updated`, `scope`, `is_dynamic`) VALUES ('Advanced', 'DEFAULT', 'CapacityManager', 'cluster.storage.operations.exclude', 'Exclude cluster from storage operations', 'false', now(), 'Cluster', '1');
    +
    +-- Added in CLOUDSTACK-9340: General DB optimization, 4 cases:
    +
    +----- 1) Incorrect PRIMARY key
    +ALTER TABLE `cloud`.`ovs_tunnel_network` 
    +DROP PRIMARY KEY,
    +ADD PRIMARY KEY (`id`),
    +DROP INDEX `id` ,
    +ADD UNIQUE INDEX `i_to_from_network_id` (`to` ASC, `from` ASC, `network_id` ASC);
    +
    +----- 2) Duplicate PRIMARY KEY
    +ALTER TABLE `cloud`.`user_vm` DROP INDEX `id_2` ,DROP INDEX `id` ;
    --- End diff --
    
    Here you remove duplicated primary keys.
    You are removing indexes and not primary keys per se, right?
    I see you removing two indexes, does that mean that the table has another one? A third one?
    Because if you remove the ID field index, if we use a select by that field, it would cause a full table scan, right?


---
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: CLOUDSTACK-9340: General DB Optimization

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

    https://github.com/apache/cloudstack/pull/1466#issuecomment-207535135
  
    @rafaelweingartner yes, it was my mistake. I wanted to leave the pr with no changes to test if Travis passes, I didn't know that removing my commits will close the pr. I reopened it with a dummy commit


---
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: CLOUDSTACK-9340: General DB Optimization

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

    https://github.com/apache/cloudstack/pull/1466#issuecomment-216893394
  
    
    
    ### CI RESULTS
    
    ```
    Tests Run: 85
      Skipped: 0
       Failed: 1
       Errors: 0
     Duration: 4h 28m 09s
    ```
    
    **Summary of the problem(s):**
    ```
    FAIL: Test redundant router internals
    ----------------------------------------------------------------------
    Traceback (most recent call last):
      File "/data/git/cs1/cloudstack/test/integration/smoke/test_routers_network_ops.py", line 483, in test_02_RVR_Network_FW_PF_SSH_default_routes_egress_false
        "Attempt to retrieve google.com index page should be successful once rule is added!"
    AssertionError: Attempt to retrieve google.com index page should be successful once rule is added!
    ----------------------------------------------------------------------
    Additional details in: /tmp/MarvinLogs/test_network_2XAB1I/results.txt
    ```
    
    
    
    **Associated Uploads**
    
    **`/tmp/MarvinLogs/DeployDataCenter__May_04_2016_05_43_43_5D72GT:`**
    * [dc_entries.obj](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1466/tmp/MarvinLogs/DeployDataCenter__May_04_2016_05_43_43_5D72GT/dc_entries.obj)
    * [failed_plus_exceptions.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1466/tmp/MarvinLogs/DeployDataCenter__May_04_2016_05_43_43_5D72GT/failed_plus_exceptions.txt)
    * [runinfo.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1466/tmp/MarvinLogs/DeployDataCenter__May_04_2016_05_43_43_5D72GT/runinfo.txt)
    
    **`/tmp/MarvinLogs/test_network_2XAB1I:`**
    * [failed_plus_exceptions.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1466/tmp/MarvinLogs/test_network_2XAB1I/failed_plus_exceptions.txt)
    * [results.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1466/tmp/MarvinLogs/test_network_2XAB1I/results.txt)
    * [runinfo.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1466/tmp/MarvinLogs/test_network_2XAB1I/runinfo.txt)
    
    **`/tmp/MarvinLogs/test_vpc_routers_9HY5WR:`**
    * [failed_plus_exceptions.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1466/tmp/MarvinLogs/test_vpc_routers_9HY5WR/failed_plus_exceptions.txt)
    * [results.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1466/tmp/MarvinLogs/test_vpc_routers_9HY5WR/results.txt)
    * [runinfo.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1466/tmp/MarvinLogs/test_vpc_routers_9HY5WR/runinfo.txt)
    
    
    Uploads will be available until `2016-07-04 02:00:00 +0200 CEST`
    
    *Comment created by [`upr comment`](https://github.com/cloudops/upr).*



---
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: CLOUDSTACK-9340: General DB Optimization

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

    https://github.com/apache/cloudstack/pull/1466#issuecomment-216876560
  
    Thanks @koushik-das. Looks like before 5.6 Mysql auto restricted length of InnoDB indexes to 255 characters without giving trouble if index length is not specified. For public_key index in ssh_keypairs table first 64 bytes is enough to make good index. @nvazquez let's change this index creation to 
    ALTER TABLE `cloud`.`ssh_keypairs` ADD INDEX `i_public_key` (`public_key`(64) ASC);


---
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: CLOUDSTACK-9340: General DB Optimization

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

    https://github.com/apache/cloudstack/pull/1466#issuecomment-208121230
  
    @rafaelweingartner 
    >>>>>>>>
     Here you change this table "ovs_tunnel_network", you dropped the primary key "id"; Will that remove only the index or the field too?
        Also, this change on database structures should be reflected on the pseudo "JPA" mapping that we use right?
        For instance, the class "com.cloud.network.ovs.dao.OvsTunnelNetworkVO" that represents the "ovs_tunnel_network" table, it is annotated as the "id" field being the id, and not the "ovs_tunnel_network"
    >>>>>>>>>
    
    We don't change any table structure but indexes so no changes in the mapping should  be required. For ovs_tunnel_network it was the only table in ACS DB that had primary key  not based on ID whereas ID was an unique index. In create-schema.sql
    CREATE TABLE `cloud`.`ovs_tunnel_network`(
      `id` bigint unsigned NOT NULL UNIQUE AUTO_INCREMENT,
      `from` bigint unsigned COMMENT 'from host id',
      `to` bigint unsigned COMMENT 'to host id',
      `network_id` bigint unsigned COMMENT 'network identifier',
      `key` int unsigned COMMENT 'gre key',
      `port_name` varchar(32) COMMENT 'in port on open vswitch',
      `state` varchar(16) default 'FAILED' COMMENT 'result of tunnel creatation',
      PRIMARY KEY(`from`, `to`, `network_id`)
    ) ENGINE=InnoDB DEFAULT CHARSET=utf8;
    
    What we are proposing is to swap them by making ID a primary key and creating a new unique index based on 3 columns `from`, `to` and `network_id`.
    
    >>>>>>>>>
    Here you remove duplicated primary keys.
        You are removing indexes and not primary keys per se, right?
        I see you removing two indexes, does that mean that the table has another one? A third one?
        Because if you remove the ID field index, if we use a select by that field, it would cause a full table scan, right?
    >>>>>>>>
    That's correct. We are removing indexes that are duplicate to the primary key. In some tables there  are 2 such indexes.
    
    >>>>>>>>>
     did you execute some evaluation to see which fields were being most used to create an index for them? 
        For instance, why did you create an index for the field called "type" of table "op_it_work"
    >>>>>>>>>
    Yes we did. Our prod DB has 1 mil+ VMs and volumes and 20K+ accounts. We started seeing degradation in simple list calls. We reviewed and analyzed all MySQL slow queries to see if any of them use full table scans. So far we found 7 cases where extra index could help. Altogether these changes reduced the load on Mysql by 30% (it doesn't spend time any more constantly doing full table scans) and improved some API calls by up to 200%. 
    
    >>>>>>>>>>
      This will remove from the projection the VMs already removed. Aren't they needed
    >>>>>>>>>
    We analyzed both the code and other DB tables. account_vmstats_view is only used in account_view and only in the join using these clauses
            LEFT JOIN `account_vmstats_view` `runningvm` ON (((`account`.`id` = `runningvm`.`account_id`)
                AND (`runningvm`.`state` = 'Running'))))
            LEFT JOIN `account_vmstats_view` `stoppedvm` ON (((`account`.`id` = `stoppedvm`.`account_id`)  AND (`stoppedvm`.`state` = 'Stopped'))))
    Since only Running and Stopped VMs used there is no need to bring Expunged one into the vmstats view. This changes alone improve listAccounts call retrieval from 11 sec to 3 ( x4 improvement).
    



---
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: CLOUDSTACK-9340: General DB Optimization

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

    https://github.com/apache/cloudstack/pull/1466#issuecomment-207498927
  
    Yes, this test failed, but I didn't see schema error this time
    
    === TestName: test_listVM_by_id_as_user_vmsfromotherdomain | Status : EXCEPTION ===
    ERROR


---
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: CLOUDSTACK-9340: General DB Optimization

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

    https://github.com/apache/cloudstack/pull/1466#issuecomment-207591089
  
    I saw travis failing with this earlier.  I will see if I can figure out what is going on.  Thx...


---
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: CLOUDSTACK-9340: General DB Optimization

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

    https://github.com/apache/cloudstack/pull/1466#issuecomment-215581686
  
    should I be running some sort of CI or validation for this change?  How do I validate this works and more importantly, not break anything?  I also need another LGTM.  Thx.  :)  @rhtyd have you had a chance to review this?


---
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: CLOUDSTACK-9340: General DB Optimization

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

    https://github.com/apache/cloudstack/pull/1466#issuecomment-216751340
  
    I am getting this error during deploy DB "Specified key was too long; max key length is 767 bytes". I have provided another inline comment in the code which is failing. Mysql version is 5.6.12.
    
    ========> Processing upgrade: com.cloud.upgrade.DatabaseUpgradeChecker
    [WARNING] 
    java.lang.reflect.InvocationTargetException
    	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
    	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    	at java.lang.reflect.Method.invoke(Method.java:606)
    	at org.codehaus.mojo.exec.ExecJavaMojo$1.run(ExecJavaMojo.java:297)
    	at java.lang.Thread.run(Thread.java:724)
    Caused by: com.cloud.utils.exception.CloudRuntimeException: Unable to upgrade the database
    	at com.cloud.upgrade.DatabaseUpgradeChecker.upgrade(DatabaseUpgradeChecker.java:394)
    	at com.cloud.upgrade.DatabaseUpgradeChecker.check(DatabaseUpgradeChecker.java:490)
    	at com.cloud.upgrade.DatabaseCreator.main(DatabaseCreator.java:217)
    	... 6 more
    Caused by: com.cloud.utils.exception.CloudRuntimeException: Unable to execute upgrade script: /Users/koushik/code/cloudstack-apache/cloudstack/developer/target/db/db/schema-481to490.sql
    	at com.cloud.upgrade.DatabaseUpgradeChecker.runScript(DatabaseUpgradeChecker.java:306)
    	at com.cloud.upgrade.DatabaseUpgradeChecker.upgrade(DatabaseUpgradeChecker.java:363)
    	... 8 more
    Caused by: com.mysql.jdbc.exceptions.jdbc4.MySQLSyntaxErrorException: Specified key was too long; max key length is 767 bytes
    	at com.cloud.utils.db.ScriptRunner.runScript(ScriptRunner.java:185)
    	at com.cloud.utils.db.ScriptRunner.runScript(ScriptRunner.java:87)
    	at com.cloud.upgrade.DatabaseUpgradeChecker.runScript(DatabaseUpgradeChecker.java:297)
    	... 9 more
    [INFO] ------------------------------------------------------------------------
    [INFO] BUILD FAILURE
    [INFO] ------------------------------------------------------------------------
    [INFO] Total time: 40.361 s
    [INFO] Finished at: 2016-05-04T11:07:42+05:30
    [INFO] Final Memory: 70M/263M
    [INFO] ------------------------------------------------------------------------
    [ERROR] Failed to execute goal org.codehaus.mojo:exec-maven-plugin:1.2.1:java (create-schema) on project cloud-developer: An exception occured while executing the Java class. null: InvocationTargetException: Unable to upgrade the database: Unable to execute upgrade script: /Users/koushik/code/cloudstack-apache/cloudstack/developer/target/db/db/schema-481to490.sql: Specified key was too long; max key length is 767 bytes -> [Help 1]
    [ERROR] 
    [ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
    [ERROR] Re-run Maven using the -X switch to enable full debug logging.
    [ERROR] 
    [ERROR] For more information about the errors and possible solutions, please read the following articles:
    [ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoExecutionException


---
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: CLOUDSTACK-9340: General DB Optimization

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

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


---
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: CLOUDSTACK-9340: General DB Optimization

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

    https://github.com/apache/cloudstack/pull/1466#issuecomment-207057213
  
    @nvazquez there is something wrong with one of your SQLs.
    
    >   Unable to upgrade the database: Unable to execute upgrade script: /home/travis/build/apache/cloudstack/developer/target/db/db/schema-481to490.sql: Incorrect table definition; there can be only one auto column and it must be defined as a key


---
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: CLOUDSTACK-9340: General DB Optimization

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

    https://github.com/apache/cloudstack/pull/1466#issuecomment-208285159
  
    Thanks @serg38,
    You answer helped a lot.
    I just do not fully understand the deletion of the indexes, for instance, you delete index “id” and “id_2” from table “domain_router”. Does that mean that the table has three (3) indexes?



---
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: CLOUDSTACK-9340: General DB Optimization

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

    https://github.com/apache/cloudstack/pull/1466#issuecomment-216896697
  
    @serg38, I just saw your communication with @koushik-das.  I will let him verify the fix you are working on and we will go from there.  Thanks...


---
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: CLOUDSTACK-9340: General DB Optimization

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

    https://github.com/apache/cloudstack/pull/1466#issuecomment-216893678
  
    The one failure above is unrelated to this fix and pops up periodically in my environment.  Above is a passing CI result...


---
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: CLOUDSTACK-9340: General DB Optimization

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

    https://github.com/apache/cloudstack/pull/1466#issuecomment-216226707
  
    @nvazquez thanks, can you rebase and squash changes to one commit
    
    LGTM, a CI and Travis run should confirm the fix.
    
    tag:mergeready


---
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: CLOUDSTACK-9340: General DB Optimization

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

    https://github.com/apache/cloudstack/pull/1466#issuecomment-207537254
  
    Just for the future reference.  If you want to kick off the rebuild of a PR, you can do a `push -f` which will trigger a new build even if the code did not change in the PR.  I am learning all these little hacks to make things easier...  :P


---
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: CLOUDSTACK-9340: General DB Optimization

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

    https://github.com/apache/cloudstack/pull/1466#issuecomment-217082822
  
    @serg38 LGTM. Verified on my test env.
    @swill This is ready for merge.


---
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: CLOUDSTACK-9340: General DB Optimization

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

    https://github.com/apache/cloudstack/pull/1466#issuecomment-214831953
  
    @rafaelweingartner @rhtyd @swill I did `push -f` tests start again, I'm getting an error on Travis test due to duplicate field name 'size' in `schema-40to410.sql`, but I added this changes in `schema-481to490.sql`, do you know why is it failing?
    
    `[ERROR] Failed to execute goal org.codehaus.mojo:exec-maven-plugin:1.2.1:java (create-schema-simulator) on project cloud-developer: An exception occured while executing the Java class. null: InvocationTargetException: Unable to upgrade the database: Unable to execute upgrade script: /home/travis/build/apache/cloudstack/developer/target/db/db/schema-40to410.sql: Duplicate column name 'size' -> [Help 1]`


---
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: CLOUDSTACK-9340: General DB Optimization

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

    https://github.com/apache/cloudstack/pull/1466#discussion_r59147080
  
    --- Diff: setup/db/db/schema-481to490.sql ---
    @@ -413,3 +413,89 @@ VIEW `user_vm_view` AS
     
     -- Add cluster.storage.operations.exclude property
     INSERT INTO `cloud`.`configuration` (`category`, `instance`, `component`, `name`, `description`, `default_value`, `updated`, `scope`, `is_dynamic`) VALUES ('Advanced', 'DEFAULT', 'CapacityManager', 'cluster.storage.operations.exclude', 'Exclude cluster from storage operations', 'false', now(), 'Cluster', '1');
    +
    +-- Added in CLOUDSTACK-9340: General DB optimization, 4 cases:
    +
    +----- 1) Incorrect PRIMARY key
    +ALTER TABLE `cloud`.`ovs_tunnel_network` 
    +DROP PRIMARY KEY,
    +ADD PRIMARY KEY (`id`),
    +DROP INDEX `id` ,
    +ADD UNIQUE INDEX `i_to_from_network_id` (`to` ASC, `from` ASC, `network_id` ASC);
    +
    +----- 2) Duplicate PRIMARY KEY
    +ALTER TABLE `cloud`.`user_vm` DROP INDEX `id_2` ,DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`domain_router` DROP INDEX `id_2` ,DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`vm_instance` DROP INDEX `id_2` ,DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`account_vlan_map` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`account_vnet_map` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`baremetal_rct` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`cluster` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`conditions` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`counter` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`data_center` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`dc_storage_network_ip_range` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`dedicated_resources` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`host_pod_ref` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`iam_group` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`iam_policy` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`iam_policy_permission` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`image_store_details` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`instance_group` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`netapp_lun` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`netapp_pool` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`netapp_volume` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`network_acl_item_cidrs` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`network_offerings` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`nic_secondary_ips` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`nics` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`op_ha_work` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`op_host` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`op_host_transfer` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`op_networks` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`op_nwgrp_work` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`op_vm_ruleset_log` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`op_vpc_distributed_router_sequence_no` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`pod_vlan_map` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`portable_ip_address` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`portable_ip_range` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`region` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`remote_access_vpn` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`snapshot_details` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`snapshots` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`storage_pool` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`storage_pool_details` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`storage_pool_work` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`user_ip_address` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`user_ipv6_address` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`user_statistics` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`version` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`vlan` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`vm_disk_statistics` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`vm_snapshot_details` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`vm_work_job` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`vpc_gateways` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`vpn_users` DROP INDEX `id` ;
    +
    +----- 3) Missing indexes (Add indexes to avoid full table scans)
    +ALTER TABLE `cloud`.`op_it_work` ADD INDEX `i_type_and_updated` (`type` ASC, `updated_at` ASC);
    +ALTER TABLE `cloud`.`vm_root_disk_tags` ADD INDEX `i_vm_id` (`vm_id` ASC);
    +ALTER TABLE `cloud`.`vm_compute_tags` ADD INDEX `i_vm_id` (`vm_id` ASC);
    +ALTER TABLE `cloud`.`vm_network_map` ADD INDEX `i_vm_id` (`vm_id` ASC);
    +ALTER TABLE `cloud`.`ssh_keypairs` ADD INDEX `i_public_key` (`public_key` ASC);
    +ALTER TABLE `cloud`.`user_vm_details` ADD INDEX `i_name_vm_id` (`vm_id` ASC, `name` ASC);
    +ALTER TABLE `cloud`.`instance_group` ADD INDEX `i_name` (`name` ASC);
    +
    +----- 4) Some views query (Change view to improve account retrieval speed) 
    --- End diff --
    
    This will remove from the projection the VMs already removed. Aren't they needed


---
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: CLOUDSTACK-9340: General DB Optimization

Posted by nvazquez <gi...@git.apache.org>.
GitHub user nvazquez reopened a pull request:

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

    CLOUDSTACK-9340: General DB Optimization

    ## Description
    In some production environments there were being experimented delays in most of the jobs. A search for DB optimization was taken and some deficiencies were discovered, we can group them in 4 groups:
    * Incorrect PRIMARY key
    * Duplicate PRIMARY KEY
    * Missing indexes (Add indexes to avoid full table scans)
    * Some views query (Change view to improve account retrieval speed)

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

    $ git pull https://github.com/nvazquez/cloudstack graldboptimization

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

    https://github.com/apache/cloudstack/pull/1466.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 #1466
    
----
commit fa0e61f183b361f4544dd2904da53380c1e12cfe
Author: nvazquez <ni...@gmail.com>
Date:   2016-04-08T17:47:50Z

    Dummy commit for testing travis

----


---
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: CLOUDSTACK-9340: General DB Optimization

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

    https://github.com/apache/cloudstack/pull/1466#issuecomment-215583632
  
    If you run your CI tests, it should detect if it breaks something. But more importantly would be to check if the performance was improved. But, I am sure @nvazquez and @serg38 have checked and re-checked that (given the work I have seen them doing here).


---
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: CLOUDSTACK-9340: General DB Optimization

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

    https://github.com/apache/cloudstack/pull/1466#issuecomment-215549970
  
    I confess that without looking the code, I thought the same as @nvazquez.
    First we would execute schema-XXXtoYYY.sql, schemaXXXtoYYY-cleanup.sql and then schema-XXX+1toYYY+1.sql, schemaXXX+1toYYY+1-cleanup.sql.
    It is good that we have clarified that ;)



---
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: CLOUDSTACK-9340: General DB Optimization

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

    https://github.com/apache/cloudstack/pull/1466#issuecomment-215546030
  
    @nvazquez, 
    What does that mean?
    
    First we are executing all of the `schema-XXXtoYYY.sql`, then all of the `schemaXXXtoYYY-cleanup.sql`? Instead of` schema-XXXtoYYY.sql, schemaXXXtoYYY-cleanup.sql` and then `schema-XXX+1toYYY+1.sql, schemaXXX+1toYYY+1-cleanup.sql`
    
    I have just looked at the code at "com.cloud.upgrade.DatabaseUpgradeChecker.upgrade(String, String)", that is exactly what happens.
    First we are executing all of the `schema-XXXtoYYY.sql`, then all of the `schemaXXXtoYYY-cleanup.sql`
    
    So, I am ok with your changes to create the "schema-410to420-cleanup.sql"
    
    So, I am ok with you moving some scripts to 



---
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: CLOUDSTACK-9340: General DB Optimization

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

    https://github.com/apache/cloudstack/pull/1466#issuecomment-207551688
  
    Oh, I understand.  Thanks...  :)


---
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: CLOUDSTACK-9340: General DB Optimization

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

    https://github.com/apache/cloudstack/pull/1466#issuecomment-208387324
  
    @rafaelweingartner. That's correct. 3 indexes: 1 - primary + 2 unique indexes, all based on ID column. Those 2 unique indexes are not needed and only consume disk space.


---
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: CLOUDSTACK-9340: General DB Optimization

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

    https://github.com/apache/cloudstack/pull/1466#issuecomment-207440848
  
    @nvazquez, 
    it is complain about a column called "size" at the "schema-40to410.sql"
    > schema-40to410.sql: Duplicate column name 'size'
    
    That is a little weird because you did not change anything there. It is looking like an environment problem. Maybe with a little luck and a push -f, the Jenkins job is executed somewhere else and that problem goes away?


---
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: CLOUDSTACK-9340: General DB Optimization

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

    https://github.com/apache/cloudstack/pull/1466#issuecomment-216258261
  
    Running CI now...  


---
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: CLOUDSTACK-9340: General DB Optimization

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

    https://github.com/apache/cloudstack/pull/1466#issuecomment-207562190
  
    You're welcome :) 
    Now Travis failed without including changes (only comment line). Do I try again after Jenkins finishes using push -f?


---
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: CLOUDSTACK-9340: General DB Optimization

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

    https://github.com/apache/cloudstack/pull/1466#issuecomment-207081102
  
    @rafaelweingartner I'll check this


---
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: CLOUDSTACK-9340: General DB Optimization

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

    https://github.com/apache/cloudstack/pull/1466#issuecomment-207454618
  
    It failed again, I can try pushing sql by parts to find out which part is failing


---
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: CLOUDSTACK-9340: General DB Optimization

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

    https://github.com/apache/cloudstack/pull/1466#issuecomment-215586085
  
    OK. I will just do a standard CI run then. Thx. 


---
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: CLOUDSTACK-9340: General DB Optimization

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

    https://github.com/apache/cloudstack/pull/1466#issuecomment-207824671
  
    @nvazquez can you explain how did you find out which keys (i.e indexes) to drop?


---
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: CLOUDSTACK-9340: General DB Optimization

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

    https://github.com/apache/cloudstack/pull/1466#issuecomment-208402795
  
    Thanks @serg38 @rafaelweingartner. I rebased master branch and pushed again


---
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: CLOUDSTACK-9340: General DB Optimization

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

    https://github.com/apache/cloudstack/pull/1466#issuecomment-216901961
  
    @koushik-das @swill @serg38 I modified index length for ssh_keypairs table


---
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: CLOUDSTACK-9340: General DB Optimization

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

    https://github.com/apache/cloudstack/pull/1466#issuecomment-207994438
  
    @bhaisaab Most of the indexes being dropped are duplicate. In create-schema.sql quite a lot of tables are created like this:
    CREATE TABLE `cloud`.`version` (
    `id` bigint unsigned NOT NULL UNIQUE AUTO_INCREMENT COMMENT 'id',
    `version` char(40) NOT NULL UNIQUE COMMENT 'version',
    `updated` datetime NOT NULL COMMENT 'Date this version table was updated',
    `step` char(32) NOT NULL COMMENT 'Step in the upgrade to this version',
    PRIMARY KEY (`id`),
    INDEX `i_version__version`(`version`)
    ) ENGINE=InnoDB DEFAULT CHARSET=utf8;
    
    When column ID is marked as UNIQUE MySQL creates an unique index ID. Then below a primary key ID is defined. Primary key in its nature already guarantees uniqueness so it is safe to drop index ID leaving only primary key to remain.
    There are more than 50 tables like this plus 3 more tables where unique index based on ID column was created twice. For example for domain_router table one duplicate index was created initially in create-schema.sql and another duplicate was generated in schema-442to450.sql when the following line was executed:
    ALTER TABLE `cloud`.`domain_router` MODIFY `id` bigint unsigned AUTO_INCREMENT UNIQUE NOT NULL;


---
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: CLOUDSTACK-9340: General DB Optimization

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

    https://github.com/apache/cloudstack/pull/1466#issuecomment-214856702
  
    @nvazquez I'm not sure, can you try to reproduce and fix it locally; test with MySQL 5.6 and 5.5 (both major versions)


---
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: CLOUDSTACK-9340: General DB Optimization

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

    https://github.com/apache/cloudstack/pull/1466#issuecomment-207534182
  
    are you still going to push this forward?


---
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: CLOUDSTACK-9340: General DB Optimization

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

    https://github.com/apache/cloudstack/pull/1466#issuecomment-207617920
  
    @swill @nvazquez I saw the failing job: https://travis-ci.org/apache/cloudstack/jobs/121766436
    It failed due to missing Marvin distribution:
    IOError: [Errno 2] No such file or directory: '/home/travis/build/apache/cloudstack/tools/marvin/dist/Marvin-*.tar.gz'
    
    The actual error seems to be hidden as only parts of mvn build output are shown in the output. It is likely that one of the unit tests have failed (likely one unit test related to kvm disk i/o where it checks some disk activity -- this has happened in my VM env few time though does not happen on baremetal). 
    
    I'll open a PR to increase verbosity of the Travis build to output which test/module failed.


---
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: CLOUDSTACK-9340: General DB Optimization

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

    https://github.com/apache/cloudstack/pull/1466#issuecomment-216251605
  
    Done, thanks @rhtyd @swill @rafaelweingartner 


---
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: CLOUDSTACK-9340: General DB Optimization

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

    https://github.com/apache/cloudstack/pull/1466#issuecomment-215548664
  
    @nvazquez that's by design -- all upgrade paths (migration code and the schema files) are run first and after that only the cleanup scripts are ran


---
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: CLOUDSTACK-9340: General DB Optimization

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

    https://github.com/apache/cloudstack/pull/1466#discussion_r61992331
  
    --- Diff: setup/db/db/schema-481to490.sql ---
    @@ -413,3 +413,26 @@ VIEW `user_vm_view` AS
     
     -- Add cluster.storage.operations.exclude property
     INSERT INTO `cloud`.`configuration` (`category`, `instance`, `component`, `name`, `description`, `default_value`, `updated`, `scope`, `is_dynamic`) VALUES ('Advanced', 'DEFAULT', 'CapacityManager', 'cluster.storage.operations.exclude', 'Exclude cluster from storage operations', 'false', now(), 'Cluster', '1');
    +
    +----- 3) Missing indexes (Add indexes to avoid full table scans)
    +ALTER TABLE `cloud`.`op_it_work` ADD INDEX `i_type_and_updated` (`type` ASC, `updated_at` ASC);
    +ALTER TABLE `cloud`.`vm_root_disk_tags` ADD INDEX `i_vm_id` (`vm_id` ASC);
    +ALTER TABLE `cloud`.`vm_compute_tags` ADD INDEX `i_vm_id` (`vm_id` ASC);
    +ALTER TABLE `cloud`.`vm_network_map` ADD INDEX `i_vm_id` (`vm_id` ASC);
    +ALTER TABLE `cloud`.`ssh_keypairs` ADD INDEX `i_public_key` (`public_key` ASC);
    --- End diff --
    
    The public_key field is varchar(5120) and creating an index on this is failing with error 
    Error Code: 1071. Specified key was too long; max key length is 767 bytes



---
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: CLOUDSTACK-9340: General DB Optimization

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

    https://github.com/apache/cloudstack/pull/1466#issuecomment-216910739
  
    Thanks @nvazquez did you test this against mysql 5.1, 5.6 and 5.7? The Travis run will check against MySQL-Server 5.5


---
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: CLOUDSTACK-9340: General DB Optimization

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

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


---
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: CLOUDSTACK-9340: General DB Optimization

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

    https://github.com/apache/cloudstack/pull/1466#issuecomment-218043423
  
    Thanks @koushik-das, @swill now that there are 2 LGTM can we have this 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: CLOUDSTACK-9340: General DB Optimization

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

    https://github.com/apache/cloudstack/pull/1466#issuecomment-215529568
  
    @rhtyd @rafaelweingartner I could discover the problem by deleting `cloud` and `cloud_usage` database, then `cloudstack-databases-setup` to create them blank and restarting CS to create schemas. Error was whis:
    ````
    2016-04-27 08:35:19,850 ERROR [utils.db.ScriptRunner] (main:null) Error executing: ALTER TABLE `cloud`.`remote_access_vpn` DROP INDEX `id`
    2016-04-27 08:35:19,851 ERROR [utils.db.ScriptRunner] (main:null) com.mysql.jdbc.exceptions.jdbc4.MySQLSyntaxErrorException: Incorrect table definition; there can be only one auto column and it must be defined as a key
    ````
    That shouldn't be a problem because in `schema-410to420-cleanup.sql` it was:
    ````
    ALTER TABLE `cloud`.`remote_access_vpn` DROP primary key;
    ALTER TABLE `cloud`.`remote_access_vpn` ADD primary key (`id`);
    ````
    However, it was noticed that `schemaXXXtoYYY-cleanup.sql` files hadn't been executed before the failure, so it was decided to move some sentences from `schema-481to490.sql` to `schema-481to490-cleanup.sql` so they can be executed after all `schemaXXXtoYYY.sql` files, it succeeded.


---
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: CLOUDSTACK-9340: General DB Optimization

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

    https://github.com/apache/cloudstack/pull/1466#issuecomment-207624970
  
    Thank you very much for the quick follow up on this, it is greatly appreciated.  :+1:  I will review the linked PR. 


---
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: CLOUDSTACK-9340: General DB Optimization

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

    https://github.com/apache/cloudstack/pull/1466#issuecomment-207472873
  
    @rafaelweingartner sorry I pushed force again, after the whole file failed I pushed a part of sql file and pushed it and started again


---
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: CLOUDSTACK-9340: General DB Optimization

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

    https://github.com/apache/cloudstack/pull/1466#issuecomment-215572026
  
    With this change PR passed Travis and Jenkins. @rafaelweingartner  and @rhtyd will you be able to give your blessing?


---
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: CLOUDSTACK-9340: General DB Optimization

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

    https://github.com/apache/cloudstack/pull/1466#issuecomment-216574386
  
    All checks have been passed after squashing. @swill did you CI run pass? If so can you merge it please?


---
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: CLOUDSTACK-9340: General DB Optimization

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

    https://github.com/apache/cloudstack/pull/1466#issuecomment-216967912
  
    @rhtyd  I just finished QA of changes on MySQL 5.1, 5.5, 5.6 and 5.7. No issues. 64byte index on ssh_keypairs table is created fine on all DB platforms.
    @koushik-das To be on the safe side can you re-test in your environment? 


---
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: CLOUDSTACK-9340: General DB Optimization

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

    https://github.com/apache/cloudstack/pull/1466#issuecomment-216576269
  
    I was having problems with this in my CI, but I am still trying to determine if it is my environment or not.  I will try again once I get master tested (since we fixed a merge conflict last night).


---
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: CLOUDSTACK-9340: General DB Optimization

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

    https://github.com/apache/cloudstack/pull/1466#discussion_r59146682
  
    --- Diff: setup/db/db/schema-481to490.sql ---
    @@ -413,3 +413,89 @@ VIEW `user_vm_view` AS
     
     -- Add cluster.storage.operations.exclude property
     INSERT INTO `cloud`.`configuration` (`category`, `instance`, `component`, `name`, `description`, `default_value`, `updated`, `scope`, `is_dynamic`) VALUES ('Advanced', 'DEFAULT', 'CapacityManager', 'cluster.storage.operations.exclude', 'Exclude cluster from storage operations', 'false', now(), 'Cluster', '1');
    +
    +-- Added in CLOUDSTACK-9340: General DB optimization, 4 cases:
    +
    +----- 1) Incorrect PRIMARY key
    +ALTER TABLE `cloud`.`ovs_tunnel_network` 
    --- End diff --
    
    Here you change this table "ovs_tunnel_network", you dropped the primary key "id"; Will that remove only the index or the field too?
    
    Also, this change on database structures should be reflected on the pseudo "JPA" mapping that we use right?
    
    For instance, the class "com.cloud.network.ovs.dao.OvsTunnelNetworkVO" that represents the "ovs_tunnel_network" table, it is annotated as the "id" field being the id, and not the "ovs_tunnel_network"


---
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: CLOUDSTACK-9340: General DB Optimization

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

    https://github.com/apache/cloudstack/pull/1466#issuecomment-207473461
  
    Now the error is different, right?


---
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: CLOUDSTACK-9340: General DB Optimization

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

    https://github.com/apache/cloudstack/pull/1466#issuecomment-215572438
  
    off course ;)
    I was only waiting for that
    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: CLOUDSTACK-9340: General DB Optimization

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

    https://github.com/apache/cloudstack/pull/1466#discussion_r59146919
  
    --- Diff: setup/db/db/schema-481to490.sql ---
    @@ -413,3 +413,89 @@ VIEW `user_vm_view` AS
     
     -- Add cluster.storage.operations.exclude property
     INSERT INTO `cloud`.`configuration` (`category`, `instance`, `component`, `name`, `description`, `default_value`, `updated`, `scope`, `is_dynamic`) VALUES ('Advanced', 'DEFAULT', 'CapacityManager', 'cluster.storage.operations.exclude', 'Exclude cluster from storage operations', 'false', now(), 'Cluster', '1');
    +
    +-- Added in CLOUDSTACK-9340: General DB optimization, 4 cases:
    +
    +----- 1) Incorrect PRIMARY key
    +ALTER TABLE `cloud`.`ovs_tunnel_network` 
    +DROP PRIMARY KEY,
    +ADD PRIMARY KEY (`id`),
    +DROP INDEX `id` ,
    +ADD UNIQUE INDEX `i_to_from_network_id` (`to` ASC, `from` ASC, `network_id` ASC);
    +
    +----- 2) Duplicate PRIMARY KEY
    +ALTER TABLE `cloud`.`user_vm` DROP INDEX `id_2` ,DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`domain_router` DROP INDEX `id_2` ,DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`vm_instance` DROP INDEX `id_2` ,DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`account_vlan_map` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`account_vnet_map` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`baremetal_rct` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`cluster` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`conditions` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`counter` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`data_center` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`dc_storage_network_ip_range` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`dedicated_resources` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`host_pod_ref` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`iam_group` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`iam_policy` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`iam_policy_permission` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`image_store_details` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`instance_group` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`netapp_lun` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`netapp_pool` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`netapp_volume` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`network_acl_item_cidrs` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`network_offerings` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`nic_secondary_ips` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`nics` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`op_ha_work` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`op_host` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`op_host_transfer` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`op_networks` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`op_nwgrp_work` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`op_vm_ruleset_log` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`op_vpc_distributed_router_sequence_no` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`pod_vlan_map` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`portable_ip_address` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`portable_ip_range` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`region` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`remote_access_vpn` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`snapshot_details` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`snapshots` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`storage_pool` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`storage_pool_details` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`storage_pool_work` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`user_ip_address` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`user_ipv6_address` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`user_statistics` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`version` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`vlan` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`vm_disk_statistics` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`vm_snapshot_details` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`vm_work_job` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`vpc_gateways` DROP INDEX `id` ;
    +ALTER TABLE `cloud`.`vpn_users` DROP INDEX `id` ;
    +
    +----- 3) Missing indexes (Add indexes to avoid full table scans)
    --- End diff --
    
    did you execute some evaluation to see which fields were being most used to create an index for them?
    
    For instance, why did you create an index for the field called "type" of table "op_it_work"


---
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: CLOUDSTACK-9340: General DB Optimization

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

    https://github.com/apache/cloudstack/pull/1466#issuecomment-214861439
  
    On Jenkins I saw an error that is due to environment misconfigurations:
    
    > [FATAL] Non-parseable settings /home/jenkins/.m2/settings.xml: unexpected character in markup % (position: START_TAG seen ...</username>\n     <password><%... @14:18)  @ /home/jenkins/.m2/settings.xml, line 14, column 18
    
    And then on Travis there is an error that is caused by a wrong table definition:
    
    > [ERROR] Failed to execute goal org.codehaus.mojo:exec-maven-plugin:1.2.1:java (create-schema) on project cloud-developer: An exception occured while executing the Java class. null: InvocationTargetException: Unable to upgrade the database: Unable to execute upgrade script: /home/travis/build/apache/cloudstack/developer/target/db/db/schema-481to490.sql: Incorrect table definition; there can be only one auto column and it must be defined as a key -> [Help 1]
    
    That seems to be something you changed in "schema-481to490.sql"


---
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: CLOUDSTACK-9340: General DB Optimization

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

    https://github.com/apache/cloudstack/pull/1466#issuecomment-207823632
  
    Thanks @rafaelweingartner, @swill and @bhaisaab for your help. I pushed changes again


---
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: CLOUDSTACK-9340: General DB Optimization

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

    https://github.com/apache/cloudstack/pull/1466#issuecomment-215548103
  
    @rafaelweingartner exactly, I meant to explain that I realized that first all `schema-XXXtoYYY.sql` were executed and after they finished, all `schema-XXXtoYYY-cleanup.sql` were executed, that's why I moved to cleanup sql :)


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