You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by marcaurele <gi...@git.apache.org> on 2016/11/18 14:48:47 UTC

[GitHub] cloudstack pull request #1768: CLOUDSTACK 9601: Upgrade: change logic for up...

GitHub user marcaurele opened a pull request:

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

    CLOUDSTACK 9601: Upgrade: change logic for update path for files

    For going from version A to version D, it uses to run the SQL files in
    that order: A -> B -> C -> D -> A-cleanup -> B-cleanup -> C-cleanup ->
    D-cleanup. If you had upgraded each version separatively you would have
    run A -> A-cleanup -> B -> B-cleanup -> C -> C-cleanup -> D ->
    D-cleanup.
    This change the logic to follow the same path if you are jumping over
    versions.
    
    Signed-off-by: Marc-Aur�le Brothier <m...@brothier.org>

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

    $ git pull https://github.com/exoscale/cloudstack fix/CLOUDSTACK-9601

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

    https://github.com/apache/cloudstack/pull/1768.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 #1768
    
----
commit 66a839af4cf10ac65bd66cf4c7875c6fc76d6317
Author: Marc-Aur�le Brothier <m...@brothier.org>
Date:   2016-11-18T14:40:13Z

    Upgrade: change logic for update path for files
    
    For going from version A to version D, it uses to run the SQL files in
    that order: A -> B -> C -> D -> A-cleanup -> B-cleanup -> C-cleanup ->
    D-cleanup. If you had upgraded each version separatively you would have
    run A -> A-cleanup -> B -> B-cleanup -> C -> C-cleanup -> D ->
    D-cleanup.
    This change the logic to follow the same path if you are jumping over
    versions.
    
    Signed-off-by: Marc-Aur�le Brothier <m...@brothier.org>

commit 0dba4be398e0966eb506ef76d2b15bf88c8c0100
Author: Marc-Aur�le Brothier <m...@brothier.org>
Date:   2016-11-18T14:46:30Z

    Cleanup upgrade path from too old version
    
    Today, the schema is created as a version 4.0.0
    
    Signed-off-by: Marc-Aur�le Brothier <m...@brothier.org>

----


---
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 issue #1768: CLOUDSTACK 9601: Upgrade: change logic for update pa...

Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the issue:

    https://github.com/apache/cloudstack/pull/1768
  
    @BlueOrangUtan package


---
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 issue #1768: CLOUDSTACK 9601: Upgrade: change logic for update pa...

Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the issue:

    https://github.com/apache/cloudstack/pull/1768
  
    @rhtyd I don't see how your concern is more true for the new code by @marcaurele then it is true for old code. cleanup is still going to be executed after the scripts, is it? only directly after instead of waiting for unrelated upgrade scripts to complete as well. We never have and never can have full coverage of upgrades as every cloud is a different snowflake. So we'll need to fix problems after intitial release most of the time. That is no worse or better with this code it is just a little more predictable.


---
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 issue #1768: CLOUDSTACK 9601: Upgrade: change logic for update pa...

Posted by marcaurele <gi...@git.apache.org>.
Github user marcaurele commented on the issue:

    https://github.com/apache/cloudstack/pull/1768
  
    @rhtyd I don't see how I could write such a test case. My point is exactly what you're saying, the upgrade would be different today for anyone who comes from an older version, which I don't think you're currently testing. I want to avoid such situation, therefore I'm pushing this PR to stop having different upgrade paths.
    Can you be more explicit in what you think can become a problem compared to the current situation we're in today?


---
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 issue #1768: CLOUDSTACK 9601: Upgrade: change logic for update pa...

Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the issue:

    https://github.com/apache/cloudstack/pull/1768
  
    @syed the -cleanup scripts are for removing data that had been migrated, not for temporary tables. also a use case for those may be if things are done partly in the migrate script and partly in the migrate class, though I can't think of a real instance of that use case.
    as for the ovs-tunnel code, we don't have to loose that, do we? SO even with people using it we can apply 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 #1768: CLOUDSTACK 9601: Upgrade: change logic for up...

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

    https://github.com/apache/cloudstack/pull/1768#discussion_r103394277
  
    --- Diff: engine/schema/src/com/cloud/upgrade/DatabaseUpgradeChecker.java ---
    @@ -424,27 +421,9 @@ protected void upgrade(CloudStackVersion dbVersion, CloudStackVersion currentVer
                     }
     
                     upgrade.performDataMigration(conn);
    -                boolean upgradeVersion = true;
    -
    -                if (upgrade.getUpgradedVersion().equals("2.1.8")) {
    --- End diff --
    
    I thought that anything older than 4.x should be removed. The simulator and DB schema create a structure based on 4.0.0 or 4.1.0 if I recall correctly.
    I initially added another commit (removed now) which was only leaving upgrade paths from version 4.0.0. That can be done later, and I think it would be a good cleanup.


---
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 issue #1768: CLOUDSTACK 9601: Upgrade: change logic for update pa...

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

    https://github.com/apache/cloudstack/pull/1768
  
    @marcaurele I appreciate the cleanup, it should have been like this from the beginning but since we've the workflow to execute all the upgrades first and then the cleanup. In `https://github.com/apache/cloudstack/blob/master/setup/db/db/schema-481to490-cleanup.sql` the views are executed at the end because certain columns are added by the java based upgrade path and views can only access them after the script/java-based-upgrade is complete. I think with this change future db upgrade paths need to written carefully.
    
    I've no objections with the overall goal to cleanup the sequence logic, though I have a concern that this might cause upgrade regressions in some environments as writing upgrade validation tests may not be possible. /cc @DaanHoogland @karuturi 


---
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 issue #1768: CLOUDSTACK 9601: Upgrade: change logic for update pa...

Posted by karuturi <gi...@git.apache.org>.
Github user karuturi commented on the issue:

    https://github.com/apache/cloudstack/pull/1768
  
    Actually, BVT is not going to verify this as this is db upgrade related and travis would have tested it. 
    I am merging 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 issue #1768: CLOUDSTACK 9601: Upgrade: change logic for update pa...

Posted by borisstoyanov <gi...@git.apache.org>.
Github user borisstoyanov commented on the issue:

    https://github.com/apache/cloudstack/pull/1768
  
    @blueorangutan package


---
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 issue #1768: CLOUDSTACK 9601: Upgrade: change logic for update pa...

Posted by marcaurele <gi...@git.apache.org>.
Github user marcaurele commented on the issue:

    https://github.com/apache/cloudstack/pull/1768
  
    @rhtyd Changing the sequence is only idempotent if you have been upgrading to each new versions step by step. If you jumped other versions, then the path is different for each original version. This fix wants to avoid such a difference.


---
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 issue #1768: CLOUDSTACK 9601: Upgrade: change logic for update pa...

Posted by marcaurele <gi...@git.apache.org>.
Github user marcaurele commented on the issue:

    https://github.com/apache/cloudstack/pull/1768
  
    I looked at the code inside `Upgrade481to490.java` and I cannot understand why the table alterations have been made inside the Java class instead of the SQL file. Because now we cannot move the code from the `-cleanup` without changing that class too, other the column `role_id` won't be present when creating the `account_view`.
    To make the change transparent, I'll have to change those too. I'll push another commit for 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 issue #1768: CLOUDSTACK 9601: Upgrade: change logic for update pa...

Posted by blueorangutan <gi...@git.apache.org>.
Github user blueorangutan commented on the issue:

    https://github.com/apache/cloudstack/pull/1768
  
    @borisstoyanov a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.


---
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 #1768: CLOUDSTACK 9601: Upgrade: change logic for up...

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

    https://github.com/apache/cloudstack/pull/1768#discussion_r103388263
  
    --- Diff: engine/schema/src/com/cloud/upgrade/DatabaseUpgradeChecker.java ---
    @@ -424,27 +421,9 @@ protected void upgrade(CloudStackVersion dbVersion, CloudStackVersion currentVer
                     }
     
                     upgrade.performDataMigration(conn);
    -                boolean upgradeVersion = true;
    -
    -                if (upgrade.getUpgradedVersion().equals("2.1.8")) {
    --- End diff --
    
    I'm not sure about the implications, but since 2.x is a very old version -- we need to put in our release notes that we won't support upgrades from very old 2.x releases. We can in that case even remove all 2.x upgrade paths.


---
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 issue #1768: CLOUDSTACK 9601: Upgrade: change logic for update pa...

Posted by syed <gi...@git.apache.org>.
Github user syed commented on the issue:

    https://github.com/apache/cloudstack/pull/1768
  
    I agree with @DaanHoogland , as @marcaurele mentioned, the only file we need to worry about is `schema-481to490-cleanup.sql` the rest of them are either empty or change the configuration where the order of execution doesn't really matter. Looking at it briefly it looks like it modifies `ovs_tunnel_network` table which is used to manage OVS tunnels when using non-vxlan isolation. I've never seen anyone use this in production. Would be useful if we can find someone who has used this before. Or if no one is using, it is LGTM from me 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 issue #1768: CLOUDSTACK 9601: Upgrade: change logic for update pa...

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

    https://github.com/apache/cloudstack/pull/1768
  
    @marcaurele I tend to agree with @rhtydthat it could break some of the upgrades. If I get it right with your proposed changes, upgrade scripts become obsolete since all the changes can be done in upgrade scripts.



---
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 issue #1768: CLOUDSTACK 9601: Upgrade: change logic for update pa...

Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the issue:

    https://github.com/apache/cloudstack/pull/1768
  
    btw @marcaurele @rhtyd idempotent will only be possible if we encode it. It is not encoded in older upgrades. Unless we clean those upgrades up we will have some issues.


---
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 issue #1768: CLOUDSTACK 9601: Upgrade: change logic for update pa...

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

    https://github.com/apache/cloudstack/pull/1768
  
    @marcaurele can we have a unit test to confirm that this does not break upgrade for older systems, also by changing the order for all older versions, the way someone would upgrade from say 4.3 to 4.10, will be different from someone who upgraded from 4.3->4.9->4.10 -- this may have side effects.


---
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 issue #1768: CLOUDSTACK 9601: Upgrade: change logic for update pa...

Posted by marcaurele <gi...@git.apache.org>.
Github user marcaurele commented on the issue:

    https://github.com/apache/cloudstack/pull/1768
  
    The second commit can be removed if the cleanup is not wanted.


---
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 issue #1768: CLOUDSTACK 9601: Upgrade: change logic for update pa...

Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the issue:

    https://github.com/apache/cloudstack/pull/1768
  
    [1768.network.results.txt](https://github.com/apache/cloudstack/files/805988/1768.network.results.txt)
    [1768.vpc_routers.results.txt](https://github.com/apache/cloudstack/files/805989/1768.vpc_routers.results.txt)
    
    just for form @karuturi . I see failures in network but can not relate these to 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 #1768: CLOUDSTACK 9601: Upgrade: change logic for up...

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

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


---
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 issue #1768: CLOUDSTACK 9601: Upgrade: change logic for update pa...

Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the issue:

    https://github.com/apache/cloudstack/pull/1768
  
    @blueorangutan test


---
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 issue #1768: CLOUDSTACK 9601: Upgrade: change logic for update pa...

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

    https://github.com/apache/cloudstack/pull/1768
  
    This may break db upgrade, as the cleanup paths are run at the end. Can you prove if changing the sequence will be idempotent? 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 issue #1768: CLOUDSTACK 9601: Upgrade: change logic for update pa...

Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the issue:

    https://github.com/apache/cloudstack/pull/1768
  
    @BlueOrangUtan help


---
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 issue #1768: CLOUDSTACK 9601: Upgrade: change logic for update pa...

Posted by remibergsma <gi...@git.apache.org>.
Github user remibergsma commented on the issue:

    https://github.com/apache/cloudstack/pull/1768
  
    @marcaurele Nice improvement, thanks! Tested it and works as you described. 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 issue #1768: CLOUDSTACK 9601: Upgrade: change logic for update pa...

Posted by marcaurele <gi...@git.apache.org>.
Github user marcaurele commented on the issue:

    https://github.com/apache/cloudstack/pull/1768
  
    @serg38 Not sure to understand what you mean with:
    > If I get it right with your proposed changes, upgrade scripts become obsolete since all the changes can be done in upgrade scripts.
    
    You meant: *If I get it right with your proposed changes, upgrade **cleanup** scripts become obsolete since all the changes can be done in upgrade scripts.* ?


---
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 issue #1768: CLOUDSTACK 9601: Upgrade: change logic for update pa...

Posted by marcaurele <gi...@git.apache.org>.
Github user marcaurele commented on the issue:

    https://github.com/apache/cloudstack/pull/1768
  
    Forget my last comment, the files can remains as they are.


---
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 issue #1768: CLOUDSTACK 9601: Upgrade: change logic for update pa...

Posted by blueorangutan <gi...@git.apache.org>.
Github user blueorangutan commented on the issue:

    https://github.com/apache/cloudstack/pull/1768
  
    Packaging result: \u2714centos6 \u2714centos7 \u2714debian. JID-532


---
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 issue #1768: CLOUDSTACK 9601: Upgrade: change logic for update pa...

Posted by karuturi <gi...@git.apache.org>.
Github user karuturi commented on the issue:

    https://github.com/apache/cloudstack/pull/1768
  
    Thanks everyone. merging this 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 issue #1768: CLOUDSTACK 9601: Upgrade: change logic for update pa...

Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the issue:

    https://github.com/apache/cloudstack/pull/1768
  
    LGTM but
    I hope you appreciate the concerns @marcaurele , on the other hand let's fix what breaks and make sure the 4.9 to 4.10 works with this. We can always advice to upgrade to 4.9 before going up.


---
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 issue #1768: CLOUDSTACK 9601: Upgrade: change logic for update pa...

Posted by blueorangutan <gi...@git.apache.org>.
Github user blueorangutan commented on the issue:

    https://github.com/apache/cloudstack/pull/1768
  
    @DaanHoogland a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests


---
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 issue #1768: CLOUDSTACK 9601: Upgrade: change logic for update pa...

Posted by marcaurele <gi...@git.apache.org>.
Github user marcaurele commented on the issue:

    https://github.com/apache/cloudstack/pull/1768
  
    I'll try to make my point clearer with a better use case. Let say you were running version ACS 4.4.2 and wish to upgrade to 4.7.1. After installing the 4.7.1, when ACS starts for the first time you will execute SQL scripts in that order (case A):
    ```
    schema-442to450.sql       -----> schema-442to450-cleanup.sql
          |                  |             |
          v                  |             v
    schema-450to451.sql      |       schema-450to451-cleanup.sql
          |                  |             |
          v                  |             v
    schema-451to452.sql      |       schema-451to452-cleanup.sql
          |                  |             |
          v                  |             v
    schema-452to460.sql      |       schema-452to460-cleanup.sql
          |                  |             |
          v                  |             v
    schema-460to461.sql      |       schema-460to461-cleanup.sql
          |                  |             |
          v                  |             v
    schema-461to470.sql      |       schema-461to470-cleanup.sql
          |                  |             |
          v                  |             v
    schema-470to471.sql >----        schema-470to471-cleanup.sql
    ```
    
    But if you would have updated to each versions, one after the other, you would have run those scripts in that order (case B):
    ```
    schema-442to450.sql -----> schema-442to450-cleanup.sql
                                     |
           --------------------------
          |
          v
    schema-450to451.sql -----> schema-450to451-cleanup.sql
                                     |
           --------------------------
          |
          v
    schema-451to452.sql -----> schema-451to452-cleanup.sql
                                     |
           --------------------------
          |
          v
    schema-452to460.sql -----> schema-452to460-cleanup.sql
                                     |
           --------------------------
          |
          v
    schema-460to461.sql -----> schema-460to461-cleanup.sql
                                     |
           --------------------------
          |
          v
    schema-461to470.sql -----> schema-461to470-cleanup.sql
                                     |
           --------------------------
          |
          v
    schema-470to471.sql -----> schema-470to471-cleanup.sql
    ```
    
    Since **case B** is that most developer would expect when fixing bugs and doing changes, but **case A** is the most common case of production upgrade, I wanted to correct the algorithm so that everyone will follow the same route (case B).
    
    Most `-cleanup.sql` scripts are either empty or only updating the `configuration` table, so it's safe. There is only one possible problematic script: https://github.com/apache/cloudstack/blob/master/setup/db/db/schema-481to490-cleanup.sql today. This one does change views, which IMO was a mistake to put in the cleanup script file, it should have gone into `schema-481to490.sql` (@rhtyd ?). Leaving the mechanism as it is today would leave people with a possible bug while upgrading from any version prior to 4.9.0 *if* any future SQL script was to change the views modified inside `schema-481to490-cleanup.sql` because of scenario case A. Did I lost people there?
    
    Any comment @remibergsma @DaanHoogland @syed @nvazquez ?


---
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 issue #1768: CLOUDSTACK 9601: Upgrade: change logic for update pa...

Posted by marcaurele <gi...@git.apache.org>.
Github user marcaurele commented on the issue:

    https://github.com/apache/cloudstack/pull/1768
  
    As a reminder for `-cleanup` SQL scripts:
    > We rarely need cleanup scripts, only when some field/value is required during the data migration and has to be dropped later on.
    
    https://cwiki.apache.org/confluence/display/CLOUDSTACK/DB+Upgrade+in+CloudStack


---
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 issue #1768: CLOUDSTACK 9601: Upgrade: change logic for update pa...

Posted by marcaurele <gi...@git.apache.org>.
Github user marcaurele commented on the issue:

    https://github.com/apache/cloudstack/pull/1768
  
    @syed I would prefer to move what's inside `schema-481to490-cleanup.sql` to the end of the file `schema-481to490.sql` as it would have been the way of processing during an update from 481 to 490.
    If I get the go about the move, I'll do it too in this 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 issue #1768: CLOUDSTACK 9601: Upgrade: change logic for update pa...

Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the issue:

    https://github.com/apache/cloudstack/pull/1768
  
    @blueorangutan test


---
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 issue #1768: CLOUDSTACK 9601: Upgrade: change logic for update pa...

Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the issue:

    https://github.com/apache/cloudstack/pull/1768
  
    @karuturi I think this should go in. Even being a bit of a risk, it will potentially reduce the support hours on 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 issue #1768: CLOUDSTACK 9601: Upgrade: change logic for update pa...

Posted by karuturi <gi...@git.apache.org>.
Github user karuturi commented on the issue:

    https://github.com/apache/cloudstack/pull/1768
  
    @DaanHoogland Can you post test results?



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