> On March 29, 2016, 9:25 a.m., Jonathan Hurley wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/orm/entities/BlueprintSettingsEntityPK.java,
> >  lines 32-38
> > <https://reviews.apache.org/r/45347/diff/3/?file=1317579#file1317579line32>
> >
> >     These can be unique, but don't need to be part of the PK.
> 
> Nahappan Somasundaram wrote:
>     Went through other similar tables and found that the composite PK has the 
> @Id attribute.

Yes; my point is don't use a composite PK.


> On March 29, 2016, 9:25 a.m., Jonathan Hurley wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/orm/entities/BlueprintSettingsEntityPK.java,
> >  lines 116-119
> > <https://reviews.apache.org/r/45347/diff/3/?file=1317579#file1317579line116>
> >
> >     I understand that blueprint names are unique, but at the same time, 
> > this doesn't seem like the right approach for a hashCode/equals 
> > implementation. It should be based off of the unique fields for this 
> > entity, not just the blueprint name.
> 
> Nahappan Somasundaram wrote:
>     It is based on both blueprintName and settingName which form a unique 
> combination:
>      return 31 * blueprintName.hashCode() + settingName.hashCode();
>      
>     For example in BlueprintConfigEntityPK.java where blueprint name and 
> config type form a unique combination:
>       @Override
>       public int hashCode() {
>         return 31 * blueprintName.hashCode() + type.hashCode();
>       }
>       
>     If there is a different way to generate the hash code, we can try that as 
> well.
> 
> Ajit Kumar wrote:
>     I like Objects.hash(...) api than manually computing.

I actualy mis-read this. But I still think you can have a better calculation 
which also uses the surrogate ID.


> On March 29, 2016, 9:25 a.m., Jonathan Hurley wrote:
> > ambari-server/src/main/resources/Ambari-DDL-Derby-CREATE.sql, line 428
> > <https://reviews.apache.org/r/45347/diff/3/?file=1317587#file1317587line428>
> >
> >     You're not consistent with the data type of this field; 3000 characters 
> > != LONGTEXT
> 
> Nahappan Somasundaram wrote:
>     Derby DB does not support TEXT data type, so using VARCHAR(3000). 
> Searched other tables which have TEXT fields: For example, the same 
> declaration is used in blueprint_config for config_data which is a TEXT 
> field. The setting table also uses VARCHAR(3000) for the content field which 
> is a TEXT field.
>     
>     If there is a way in Derby DB to specify TEXT data type natively, we can 
> use that.

Then why not use CLOB here? The point is you have a limit of 4GB in MySQL and a 
limit of 6000 bytes on Derby; that's quite a difference.


> On March 29, 2016, 9:25 a.m., Jonathan Hurley wrote:
> > ambari-server/src/main/resources/Ambari-DDL-Derby-CREATE.sql, line 798
> > <https://reviews.apache.org/r/45347/diff/3/?file=1317587#file1317587line798>
> >
> >     Include this inline with the table declaration. I really dislike this 
> > entire section since it's extremely hard to read. 
> >     
> >     This goes for the other SQL files as well.
> 
> Nahappan Somasundaram wrote:
>     The current style in the SQL files has this separate section for ALTER 
> TABLE. I'll open a separate JIRA to track fixing of ALTER TABLE for all the 
> tables.

The old style is a separate section; the new style is in-line since it's way 
more readable and makes diff's easier.


> On March 29, 2016, 9:25 a.m., Jonathan Hurley wrote:
> > ambari-server/src/main/resources/Ambari-DDL-MySQL-CREATE.sql, line 437
> > <https://reviews.apache.org/r/45347/diff/3/?file=1317588#file1317588line437>
> >
> >     Are you sure LONGTEXT is what you want here? LONGTEXT has performance 
> > implications over MEDIUMTEXT.
> 
> Nahappan Somasundaram wrote:
>     May be MEDIUMTEXT will work for the current requirements. But there is a 
> possibility for the user to specify a long list of components or services in 
> the auto start section under settings. Moreover, the settings section can be 
> used for specifying other information like admin settings later on.

Except that in some cases you use a max of 6000 bytes, so MEDIUMTEXT is still 
more than you need. I really, really hope you're not storing more than 16MB in 
this field. I think LONGTEXT is overkill here.


- Jonathan


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


On March 29, 2016, 1:47 p.m., Nahappan Somasundaram wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45347/
> -----------------------------------------------------------
> 
> (Updated March 29, 2016, 1:47 p.m.)
> 
> 
> Review request for Ambari, Ajit Kumar, Jonathan Hurley, Nate Cole, Sumit 
> Mohanty, Sebastian Toader, and Sid Wagle.
> 
> 
> Bugs: AMBARI-15592
>     https://issues.apache.org/jira/browse/AMBARI-15592
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> AMBARI-15592: Auto-start services - support blueprint deployment.
> 
> ** Issue: **
> Define a JSON structure to specify settings for auto start in a blueprint and 
> use that information to enable or disable auto start for components during 
> deployment.
> 
> ** Changes **
> Added a new section in the blueprint called "settings" which is a collection 
> of various setting names to collection of properties. For auto start, the 
> following setting names are used:
> *recovery_settings*: {"recovery_enabled" : "true" } specifies that auto start 
> should be enabled for all components. Can also be specified within a 
> collection, to support a uniform schema.
> *service_settings*: Collection of service names and the recovery values. 
> [{"name":"HDFS", "recovery_enabled":"false"},{...}] overrides cluster level 
> recovery settings.
> *component_settings*: Collection of component names and the corresponding 
> recovery values.  [{"name":"HDFS_CLIENT", "recovery_enabled":"true"},{...}]. 
> Overrides both service settings and recovery settings.
> 
> 
> Diffs
> -----
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/orm/entities/BlueprintEntity.java
>  8578d6beca91bf411d0c3dafeaee42d2dd23caea 
>   
> ambari-server/src/main/java/org/apache/ambari/server/orm/entities/BlueprintSettingsEntity.java
>  PRE-CREATION 
>   
> ambari-server/src/main/java/org/apache/ambari/server/orm/entities/BlueprintSettingsEntityPK.java
>  PRE-CREATION 
>   
> ambari-server/src/main/java/org/apache/ambari/server/topology/AmbariContext.java
>  a5e2fa1c7a2adaadc8bbe9de15b913cd9a7ca456 
>   
> ambari-server/src/main/java/org/apache/ambari/server/topology/Blueprint.java 
> 11311dba0f1174248d24276a41837d7284e41607 
>   
> ambari-server/src/main/java/org/apache/ambari/server/topology/BlueprintFactory.java
>  cca28ca1529d6bccdf7a870dab7c317c1a334b1d 
>   
> ambari-server/src/main/java/org/apache/ambari/server/topology/BlueprintImpl.java
>  bea036421c64b70b4bceffcbd0d13c26020a7ed1 
>   ambari-server/src/main/java/org/apache/ambari/server/topology/Settings.java 
> PRE-CREATION 
>   
> ambari-server/src/main/java/org/apache/ambari/server/topology/SettingsFactory.java
>  PRE-CREATION 
>   
> ambari-server/src/main/java/org/apache/ambari/server/upgrade/UpgradeCatalog240.java
>  38a3614ae3543ae1de0ea0bfd4191951c6d5c5ff 
>   ambari-server/src/main/resources/Ambari-DDL-Derby-CREATE.sql 
> f8c97ca11040bff414626189591d86d4781be478 
>   ambari-server/src/main/resources/Ambari-DDL-MySQL-CREATE.sql 
> b306c0ab7687c6fe05c818acefa2d063e5b111ed 
>   ambari-server/src/main/resources/Ambari-DDL-Oracle-CREATE.sql 
> 37744f8bc729e374a1db76f6509a6e85bffb37d0 
>   ambari-server/src/main/resources/Ambari-DDL-Postgres-CREATE.sql 
> eba1745d8c7a3e1b677cb8b8fc33d9ae09d0cf68 
>   ambari-server/src/main/resources/Ambari-DDL-Postgres-EMBEDDED-CREATE.sql 
> cad0a39d824fdbf03b1ec6cd2a99dec79fa916d2 
>   ambari-server/src/main/resources/Ambari-DDL-SQLAnywhere-CREATE.sql 
> 346af50961fa1b2a41bd0a81121a08ec8ed70a2f 
>   ambari-server/src/main/resources/Ambari-DDL-SQLServer-CREATE.sql 
> e238a7665dff5a52426be9e334e4a48f53c7dbee 
>   ambari-server/src/main/resources/META-INF/persistence.xml 
> 513035f5baf6eacfcc69507069d311418546a09c 
>   ambari-server/src/main/resources/properties.json 
> 01c15f2b1c3564c37e8203ec70f2d2b812135b13 
>   
> ambari-server/src/test/java/org/apache/ambari/server/orm/entities/BlueprintEntityTest.java
>  c660d19aee1727fd4734c07c0e5130f5bbe3c3cf 
>   
> ambari-server/src/test/java/org/apache/ambari/server/orm/entities/BlueprintSettingsEntityPKTest.java
>  PRE-CREATION 
>   
> ambari-server/src/test/java/org/apache/ambari/server/orm/entities/BlueprintSettingsEntityTest.java
>  PRE-CREATION 
>   
> ambari-server/src/test/java/org/apache/ambari/server/topology/SettingsFactoryTest.java
>  PRE-CREATION 
>   
> ambari-server/src/test/java/org/apache/ambari/server/topology/SettingsTest.java
>  PRE-CREATION 
>   
> ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog240Test.java
>  7b2d797486061a1377622806f7680176fa8ad458 
> 
> Diff: https://reviews.apache.org/r/45347/diff/
> 
> 
> Testing
> -------
> 
> ** 1. mvn clean install **
> 
> [INFO] 
> ------------------------------------------------------------------------
> [INFO] Reactor Summary:
> [INFO]
> [INFO] Ambari Main ....................................... SUCCESS [6.402s]
> [INFO] Apache Ambari Project POM ......................... SUCCESS [0.039s]
> [INFO] Ambari Web ........................................ SUCCESS [24.133s]
> [INFO] Ambari Views ...................................... SUCCESS [1.280s]
> [INFO] Ambari Admin View ................................. SUCCESS [5.725s]
> [INFO] ambari-metrics .................................... SUCCESS [0.346s]
> [INFO] Ambari Metrics Common ............................. SUCCESS [0.459s]
> [INFO] Ambari Metrics Hadoop Sink ........................ SUCCESS [1.077s]
> [INFO] Ambari Metrics Flume Sink ......................... SUCCESS [0.574s]
> [INFO] Ambari Metrics Kafka Sink ......................... SUCCESS [0.582s]
> [INFO] Ambari Metrics Storm Sink ......................... SUCCESS [1.459s]
> [INFO] Ambari Metrics Collector .......................... SUCCESS [6.518s]
> [INFO] Ambari Metrics Monitor ............................ SUCCESS [2.164s]
> [INFO] Ambari Metrics Grafana ............................ SUCCESS [0.847s]
> [INFO] Ambari Metrics Assembly ........................... SUCCESS [1:15.164s]
> [INFO] Ambari Server ..................................... SUCCESS [2:20.006s]
> [INFO] Ambari Functional Tests ........................... SUCCESS [1.010s]
> [INFO] Ambari Agent ...................................... SUCCESS [22.882s]
> [INFO] Ambari Client ..................................... SUCCESS [0.043s]
> [INFO] Ambari Python Client .............................. SUCCESS [0.839s]
> [INFO] Ambari Groovy Client .............................. SUCCESS [2.054s]
> [INFO] Ambari Shell ...................................... SUCCESS [0.040s]
> [INFO] Ambari Python Shell ............................... SUCCESS [0.676s]
> [INFO] Ambari Groovy Shell ............................... SUCCESS [1.182s]
> [INFO] 
> ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] 
> ------------------------------------------------------------------------
> [INFO] Total time: 4:56.150s
> [INFO] Finished at: Fri Mar 25 17:24:57 PDT 2016
> [INFO] Final Memory: 279M/1202M
> [INFO] 
> ------------------------------------------------------------------------
> 
> ** 2. mvn test -DskipPythonTests 
> -Dtest=BlueprintEntityTest,BlueprintSettingsEntityPKTest,BlueprintSettingsEntityTest,SettingsFactoryTest,SettingsTest
>  **
> -------------------------------------------------------
>  T E S T S
> -------------------------------------------------------
> Picked up _JAVA_OPTIONS: -Xmx2048m -XX:MaxPermSize=512m 
> -Djava.awt.headless=true
> Running org.apache.ambari.server.orm.entities.BlueprintEntityTest
> Tests run: 5, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.001 sec - 
> in org.apache.ambari.server.orm.entities.BlueprintEntityTest
> Picked up _JAVA_OPTIONS: -Xmx2048m -XX:MaxPermSize=512m 
> -Djava.awt.headless=true
> Running org.apache.ambari.server.orm.entities.BlueprintSettingsEntityPKTest
> Tests run: 3, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.009 sec - 
> in org.apache.ambari.server.orm.entities.BlueprintSettingsEntityPKTest
> Picked up _JAVA_OPTIONS: -Xmx2048m -XX:MaxPermSize=512m 
> -Djava.awt.headless=true
> Running org.apache.ambari.server.orm.entities.BlueprintSettingsEntityTest
> Tests run: 4, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.014 sec - 
> in org.apache.ambari.server.orm.entities.BlueprintSettingsEntityTest
> Picked up _JAVA_OPTIONS: -Xmx2048m -XX:MaxPermSize=512m 
> -Djava.awt.headless=true
> Running org.apache.ambari.server.topology.SettingsFactoryTest
> Tests run: 2, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.009 sec - 
> in org.apache.ambari.server.topology.SettingsFactoryTest
> Picked up _JAVA_OPTIONS: -Xmx2048m -XX:MaxPermSize=512m 
> -Djava.awt.headless=true
> Running org.apache.ambari.server.topology.SettingsTest
> Tests run: 2, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.007 sec - 
> in org.apache.ambari.server.topology.SettingsTest
> 
> Results :
> 
> Tests run: 16, Failures: 0, Errors: 0, Skipped: 0
> 
> [INFO] Rat check: Summary of files. Unapproved: 0 unknown: 0 generated: 0 
> approved: 4290 licence.
> [INFO]
> [INFO] --- exec-maven-plugin:1.2.1:exec (python-test) @ ambari-server ---
> [INFO] skipping execute as per configuraion
> [INFO] 
> ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] 
> ------------------------------------------------------------------------
> [INFO] Total time: 28.801s
> [INFO] Finished at: Fri Mar 25 17:32:53 PDT 2016
> [INFO] Final Memory: 55M/1014M
> [INFO] 
> ------------------------------------------------------------------------
> 
> 
> ** mvn test -DskipPythonTests -Dtest=UpgradeCatalog240Test **
> 
> 
> -------------------------------------------------------
>  T E S T S
> -------------------------------------------------------
> Picked up _JAVA_OPTIONS: -Xmx2048m -XX:MaxPermSize=512m 
> -Djava.awt.headless=true
> Running org.apache.ambari.server.upgrade.UpgradeCatalog240Test
> Tests run: 4, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 9.649 sec - 
> in org.apache.ambari.server.upgrade.UpgradeCatalog240Test
> 
> Results :
> 
> Tests run: 4, Failures: 0, Errors: 0, Skipped: 0
> 
> [INFO] 
> ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] 
> ------------------------------------------------------------------------
> [INFO] Total time: 33.620s
> [INFO] Finished at: Sun Mar 27 14:49:50 PDT 2016
> [INFO] Final Memory: 55M/1013M
> [INFO] 
> ------------------------------------------------------------------------
> 
> ** 3. Manual tests **
> 
> Deployed a 2-node VM and replaced the ambari-server JAR on the server node 
> and made a blueprint based deployment:
> 
> [root@c6403 /]# curl -u admin:admin -H "X-Requested-By: ambari" -X POST -d 
> @/vagrant/blueprints1.json http://localhost:8080/api/v1/blueprints/bp1
> 
> [root@c6403 /]# curl -u admin:admin -H "X-Requested-By: ambari" -X POST -d 
> @/vagrant/hg1.json http://localhost:8080/api/v1/clusters/c1
> {
>   "href" : "http://localhost:8080/api/v1/clusters/c1/requests/1";,
>   "Requests" : {
>     "id" : 1,
>     "status" : "Accepted"
>   }
> }
> 
> ** Blueprint "settings" **
> 
>   "settings" : [
>     {
>     "recovery_settings" : [{
>       "recovery_enabled" : "true"
>      }
>     ]},
>     {
>     "service_settings" : [
>         {
>         "name" : "HDFS",
>         "recovery_enabled" : "false"
>         },
>         {
>         "name" : "TEZ",
>         "recovery_enabled" : "false"
>         }
>     ]},
>     {
>     "component_settings" : [
>     {
>       "name" : "DATANODE",
>       "recovery_enabled" : "true"
>     }
>     ]
>     }
>   ]
>   
> ** Database: blueprint_settings table **
> 
> ambari=> select * from blueprint_settings;
>  blueprint_name |    setting_name    |                                      
> setting_data
> ----------------+--------------------+----------------------------------------------------------------------------------------
>  bp1            | component_settings | 
> [{"recovery_enabled":"true","name":"DATANODE"}]
>  bp1            | recovery_settings  | [{"recovery_enabled":"true"}]
>  bp1            | service_settings   | 
> [{"recovery_enabled":"false","name":"HDFS"},{"recovery_enabled":"false","name":"TEZ"}]
> (3 rows)
> 
> ** Database: servicecomponentdesiredstate table **
> 
> ambari=> select * from servicecomponentdesiredstate;
>  id |   component_name    | cluster_id | desired_stack_id | desired_version | 
> desired_state |  service_name  | recovery_enabled
> 
> ----+---------------------+------------+------------------+-----------------+---------------+----------------+-----------------
>   3 | TEZ_CLIENT          |          2 |                7 | UNKNOWN         | 
> INSTALLED     | TEZ            |                0
>   5 | MAPREDUCE2_CLIENT   |          2 |                7 | UNKNOWN         | 
> INSTALLED     | MAPREDUCE2     |                1
>  12 | HDFS_CLIENT         |          2 |                7 | UNKNOWN         | 
> INSTALLED     | HDFS           |                0
>  13 | YARN_CLIENT         |          2 |                7 | UNKNOWN         | 
> INSTALLED     | YARN           |                1
>  15 | ZOOKEEPER_CLIENT    |          2 |                7 | UNKNOWN         | 
> INSTALLED     | ZOOKEEPER      |                1
>   1 | DATANODE            |          2 |                7 | UNKNOWN         | 
> STARTED       | HDFS           |                1
>   2 | METRICS_COLLECTOR   |          2 |                7 | UNKNOWN         | 
> STARTED       | AMBARI_METRICS |                1
>   4 | HISTORYSERVER       |          2 |                7 | UNKNOWN         | 
> STARTED       | MAPREDUCE2     |                1
>   6 | ZOOKEEPER_SERVER    |          2 |                7 | UNKNOWN         | 
> STARTED       | ZOOKEEPER      |                1
>   7 | APP_TIMELINE_SERVER |          2 |                7 | UNKNOWN         | 
> STARTED       | YARN           |                1
>   8 | METRICS_MONITOR     |          2 |                7 | UNKNOWN         | 
> STARTED       | AMBARI_METRICS |                1
>   9 | NAMENODE            |          2 |                7 | UNKNOWN         | 
> STARTED       | HDFS           |                0
>  10 | SECONDARY_NAMENODE  |          2 |                7 | UNKNOWN         | 
> STARTED       | HDFS           |                0
>  11 | RESOURCEMANAGER     |          2 |                7 | UNKNOWN         | 
> STARTED       | YARN           |                1
>  14 | METRICS_GRAFANA     |          2 |                7 | UNKNOWN         | 
> STARTED       | AMBARI_METRICS |                1
>  16 | NODEMANAGER         |          2 |                7 | UNKNOWN         | 
> STARTED       | YARN           |                1
> (16 rows)
> 
> 
> Thanks,
> 
> Nahappan Somasundaram
> 
>

Reply via email to