Re: Review Request 56859: Add Upgrade Logic For Removal of clusterconfigmapping

2017-02-21 Thread Jonathan Hurley


> On Feb. 21, 2017, 1:06 p.m., Alejandro Fernandez wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/upgrade/UpgradeCatalog300.java,
> >  line 213
> > 
> >
> > Sould first check !dbAccessor.tableHasColumn(..., ...)

addColumn is idempotent:
```
  public void addColumn(String tableName, DBColumnInfo columnInfo) throws 
SQLException {
if (tableHasColumn(tableName, columnInfo.getName())) {
  return;
}
```


> On Feb. 21, 2017, 1:06 p.m., Alejandro Fernandez wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/upgrade/UpgradeCatalog300.java,
> >  line 239
> > 
> >
> > Does the WHERE clause also need to check that selected_timestamp is 
> > NULL or = ''?

No, because selected_timestamp is a new column; the existing CCM table this is 
reading from doesn't have it.


> On Feb. 21, 2017, 1:06 p.m., Alejandro Fernandez wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/upgrade/UpgradeCatalog300.java,
> >  line 259
> > 
> >
> > The result set already has selected=1, instead, it should only set 
> > "selected_timestamp".

This is setting it on the updated columns of the clusterconfig table; it 
doesn't have them set yet. Only the clusterconfigmapping table did. This is 
basically transferring the data from the old mapping table into the new columns 
of the clusterconfig table.


- Jonathan


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56859/#review166196
---


On Feb. 20, 2017, 5:10 p.m., Jonathan Hurley wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56859/
> ---
> 
> (Updated Feb. 20, 2017, 5:10 p.m.)
> 
> 
> Review request for Ambari, Dmitro Lisnichenko, Nate Cole, Robert Levas, and 
> Sid Wagle.
> 
> 
> Bugs: AMBARI-20055
> https://issues.apache.org/jira/browse/AMBARI-20055
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> The Ambari upgrade scripts should now perform the following operations:
> 
> - Adding the following columns / constraints to the {{clusterconfig}} table:
> -- {{selected SMALLINT NOT NULL DEFAULT 0}} - whether or not the 
> configuration is currently selected
> -- {{selected_timestamp BIGINT NOT NULL DEFAULT 0}} - the last time that the 
> configuration was marked as {{selected}}
> 
> - Finding all {{selected}} mappings in {{clusterconfigmapping}} and marked 
> their associated entry in {{clusterconfig}} as {{selected}}
> 
> - Removal of the {{clusterconfigmapping}} table.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/orm/helpers/dbms/GenericDbmsHelper.java
>  36fab83 
>   
> ambari-server/src/main/java/org/apache/ambari/server/upgrade/UpgradeCatalog300.java
>  0267a5e 
>   
> ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog300Test.java
>  ec001ec 
> 
> Diff: https://reviews.apache.org/r/56859/diff/
> 
> 
> Testing
> ---
> 
> - Installed Ambari 2.5 with a basic cluster. 
> - Added new configuration versions
> - Reverted a few configurations
> - Created configuration groups with overrides
> - Performed an upgrade to verify new configs are correct
> 
> mvn clean test
> 
> [INFO] 
> 
> [INFO] BUILD SUCCESS
> [INFO] 
> 
> [INFO] Total time: 20:12 min
> [INFO] Finished at: 2017-02-20T17:09:46-05:00
> [INFO] Final Memory: 66M/718M
> [INFO] 
> 
> 
> 
> Thanks,
> 
> Jonathan Hurley
> 
>



Re: Review Request 56859: Add Upgrade Logic For Removal of clusterconfigmapping

2017-02-21 Thread Alejandro Fernandez

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56859/#review166196
---




ambari-server/src/main/java/org/apache/ambari/server/upgrade/UpgradeCatalog300.java
 (line 213)


Sould first check !dbAccessor.tableHasColumn(..., ...)



ambari-server/src/main/java/org/apache/ambari/server/upgrade/UpgradeCatalog300.java
 (line 239)


Does the WHERE clause also need to check that selected_timestamp is NULL or 
= ''?



ambari-server/src/main/java/org/apache/ambari/server/upgrade/UpgradeCatalog300.java
 (line 259)


The result set already has selected=1, instead, it should only set 
"selected_timestamp".


- Alejandro Fernandez


On Feb. 20, 2017, 10:10 p.m., Jonathan Hurley wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56859/
> ---
> 
> (Updated Feb. 20, 2017, 10:10 p.m.)
> 
> 
> Review request for Ambari, Dmitro Lisnichenko, Nate Cole, Robert Levas, and 
> Sid Wagle.
> 
> 
> Bugs: AMBARI-20055
> https://issues.apache.org/jira/browse/AMBARI-20055
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> The Ambari upgrade scripts should now perform the following operations:
> 
> - Adding the following columns / constraints to the {{clusterconfig}} table:
> -- {{selected SMALLINT NOT NULL DEFAULT 0}} - whether or not the 
> configuration is currently selected
> -- {{selected_timestamp BIGINT NOT NULL DEFAULT 0}} - the last time that the 
> configuration was marked as {{selected}}
> 
> - Finding all {{selected}} mappings in {{clusterconfigmapping}} and marked 
> their associated entry in {{clusterconfig}} as {{selected}}
> 
> - Removal of the {{clusterconfigmapping}} table.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/orm/helpers/dbms/GenericDbmsHelper.java
>  36fab83 
>   
> ambari-server/src/main/java/org/apache/ambari/server/upgrade/UpgradeCatalog300.java
>  0267a5e 
>   
> ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog300Test.java
>  ec001ec 
> 
> Diff: https://reviews.apache.org/r/56859/diff/
> 
> 
> Testing
> ---
> 
> - Installed Ambari 2.5 with a basic cluster. 
> - Added new configuration versions
> - Reverted a few configurations
> - Created configuration groups with overrides
> - Performed an upgrade to verify new configs are correct
> 
> mvn clean test
> 
> [INFO] 
> 
> [INFO] BUILD SUCCESS
> [INFO] 
> 
> [INFO] Total time: 20:12 min
> [INFO] Finished at: 2017-02-20T17:09:46-05:00
> [INFO] Final Memory: 66M/718M
> [INFO] 
> 
> 
> 
> Thanks,
> 
> Jonathan Hurley
> 
>



Re: Review Request 56859: Add Upgrade Logic For Removal of clusterconfigmapping

2017-02-21 Thread Robert Levas

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56859/#review166185
---


Ship it!




Ship It!

- Robert Levas


On Feb. 20, 2017, 5:10 p.m., Jonathan Hurley wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56859/
> ---
> 
> (Updated Feb. 20, 2017, 5:10 p.m.)
> 
> 
> Review request for Ambari, Dmitro Lisnichenko, Nate Cole, Robert Levas, and 
> Sid Wagle.
> 
> 
> Bugs: AMBARI-20055
> https://issues.apache.org/jira/browse/AMBARI-20055
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> The Ambari upgrade scripts should now perform the following operations:
> 
> - Adding the following columns / constraints to the {{clusterconfig}} table:
> -- {{selected SMALLINT NOT NULL DEFAULT 0}} - whether or not the 
> configuration is currently selected
> -- {{selected_timestamp BIGINT NOT NULL DEFAULT 0}} - the last time that the 
> configuration was marked as {{selected}}
> 
> - Finding all {{selected}} mappings in {{clusterconfigmapping}} and marked 
> their associated entry in {{clusterconfig}} as {{selected}}
> 
> - Removal of the {{clusterconfigmapping}} table.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/orm/helpers/dbms/GenericDbmsHelper.java
>  36fab83 
>   
> ambari-server/src/main/java/org/apache/ambari/server/upgrade/UpgradeCatalog300.java
>  0267a5e 
>   
> ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog300Test.java
>  ec001ec 
> 
> Diff: https://reviews.apache.org/r/56859/diff/
> 
> 
> Testing
> ---
> 
> - Installed Ambari 2.5 with a basic cluster. 
> - Added new configuration versions
> - Reverted a few configurations
> - Created configuration groups with overrides
> - Performed an upgrade to verify new configs are correct
> 
> mvn clean test
> 
> [INFO] 
> 
> [INFO] BUILD SUCCESS
> [INFO] 
> 
> [INFO] Total time: 20:12 min
> [INFO] Finished at: 2017-02-20T17:09:46-05:00
> [INFO] Final Memory: 66M/718M
> [INFO] 
> 
> 
> 
> Thanks,
> 
> Jonathan Hurley
> 
>



Re: Review Request 56859: Add Upgrade Logic For Removal of clusterconfigmapping

2017-02-21 Thread Dmitro Lisnichenko

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56859/#review166136
---


Ship it!




Ship It!

- Dmitro Lisnichenko


On Feb. 21, 2017, 12:10 a.m., Jonathan Hurley wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56859/
> ---
> 
> (Updated Feb. 21, 2017, 12:10 a.m.)
> 
> 
> Review request for Ambari, Dmitro Lisnichenko, Nate Cole, Robert Levas, and 
> Sid Wagle.
> 
> 
> Bugs: AMBARI-20055
> https://issues.apache.org/jira/browse/AMBARI-20055
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> The Ambari upgrade scripts should now perform the following operations:
> 
> - Adding the following columns / constraints to the {{clusterconfig}} table:
> -- {{selected SMALLINT NOT NULL DEFAULT 0}} - whether or not the 
> configuration is currently selected
> -- {{selected_timestamp BIGINT NOT NULL DEFAULT 0}} - the last time that the 
> configuration was marked as {{selected}}
> 
> - Finding all {{selected}} mappings in {{clusterconfigmapping}} and marked 
> their associated entry in {{clusterconfig}} as {{selected}}
> 
> - Removal of the {{clusterconfigmapping}} table.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/orm/helpers/dbms/GenericDbmsHelper.java
>  36fab83 
>   
> ambari-server/src/main/java/org/apache/ambari/server/upgrade/UpgradeCatalog300.java
>  0267a5e 
>   
> ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog300Test.java
>  ec001ec 
> 
> Diff: https://reviews.apache.org/r/56859/diff/
> 
> 
> Testing
> ---
> 
> - Installed Ambari 2.5 with a basic cluster. 
> - Added new configuration versions
> - Reverted a few configurations
> - Created configuration groups with overrides
> - Performed an upgrade to verify new configs are correct
> 
> mvn clean test
> 
> [INFO] 
> 
> [INFO] BUILD SUCCESS
> [INFO] 
> 
> [INFO] Total time: 20:12 min
> [INFO] Finished at: 2017-02-20T17:09:46-05:00
> [INFO] Final Memory: 66M/718M
> [INFO] 
> 
> 
> 
> Thanks,
> 
> Jonathan Hurley
> 
>