----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38269/#review98871 -----------------------------------------------------------
This looks good. Thank you for the contribution. Please also include Nate Cole and Jonathan Hurley in the code review. ambari-server/src/main/java/org/apache/ambari/server/orm/dao/ClusterDAO.java (line 190) <https://reviews.apache.org/r/38269/#comment155504> If this is no longer used, can remove it. ambari-server/src/main/java/org/apache/ambari/server/orm/entities/ClusterConfigEntity.java (line 54) <https://reviews.apache.org/r/38269/#comment155502> If this is no longer used, can remove it. ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClusterImpl.java (line 2845) <https://reviews.apache.org/r/38269/#comment155501> Can remove this. ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClusterImpl.java (line 2867) <https://reviews.apache.org/r/38269/#comment155505> Typo, has => have. Let's change the logging level to ERROR, and note that it was in the "applyLatestConfigurations" method. ambari-server/src/test/java/org/apache/ambari/server/orm/dao/ServiceConfigDAOTest.java (line 346) <https://reviews.apache.org/r/38269/#comment155507> Please add some doc outlining the situation this is testing. - Alejandro Fernandez On Sept. 10, 2015, 9:37 p.m., Di Li wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/38269/ > ----------------------------------------------------------- > > (Updated Sept. 10, 2015, 9:37 p.m.) > > > Review request for Ambari and Alejandro Fernandez. > > > Bugs: AMBARI-12949 > https://issues.apache.org/jira/browse/AMBARI-12949 > > > Repository: ambari > > > Description > ------- > > This seems to be an issue that happens when the "latest" configuration stored > in the "clusterconfig" table is for a conf group instead of the default > configurations. The named query > "ClusterConfigEntity.findLatestConfigsByStack" returns the latest > configuration to be set with "selected = 1" in the clusterconfigmapping > table. When the "latest" config is for a conf group, this record will not > have a corresponding entry in the clusterconfigmapping table. So the > clusterconfigmapping table is left with a type that does not have the > selected column set to 1. > > My proposed fix is the new named query > "ClusterConfigEntity.findLatestConfigsOfClusterConfigMappingEntriesByStack" > in ClusterConfigEntity.java, which searches the clusterconfig table for the > latest version of a type_name and version_tag specified in the > clusterconfigmapping table. This is different from the current > "ClusterConfigEntity.findLatestConfigsByStack" query, which simply pulls the > latest config of a type_name from the clusterconfig table regardless whether > the corresponding version_tag is in the clusterconfigmapping table or not. > > > Investigation: > > I used Oozie for debugging. > > When a user clicks the Downgrade button from the Finalize page. The request > eventually hits ClusterImpl.applyLatestConfigurations method where the Ambari > server updates the database to reverse back to the "latest" configuration of > each type for the older stack (HDP 2.2) > > The workflow goes as > 1. Set ALL entries in clusterconfigmapping table to have selected value set > to 0 > 2. Ambari server uses ClusterImpl.applyLatestConfigurations method to get the > "latest" configuration of each type for the older stack (HDP 2.2) > 3. Loop through all the entries in the clusterconfigmapping table, and set > the selected to 1 if this entry also happens to be in the "latest" > configuration returned in step 2. > > The ClusterImpl.applyLatestConfigurations method calls ClusterDAO.java > getLatestConfigurations to get the latest configuration for each type for the > older stack. The DAO class simply runs the following SQL query (cluster id > and stack id are the parameters) and passes the results back. > > SELECT clusterConfig1 FROM clusterconfig clusterConfig1 WHERE > clusterConfig1.cluster_id='2' AND clusterConfig1.create_timestamp = (SELECT > MAX(clusterConfig2.create_timestamp) FROM clusterconfig clusterConfig2 WHERE > clusterConfig2.cluster_id='2' AND clusterConfig2.stack_id= '3' AND > clusterConfig2.type_name = clusterConfig1.type_name); > > For downgrade, in our particular use case, the oozie-env configuration stored > in the clusterconfig table, when the stack id is the HDP 2.2 version, only > contains TWO conf, the original version1 and the one for the config group. > > The query above will return the entry corresponding to the oozie config > group, as it was the last configuration entry added to the clusterconfig > table for oozie-site with stack id = 3. However, because the config group > entry is NEVER stored into the clusterconfigmapping table, the logic in > ClusterImpl.applyLatestConfigurations method to re-select a version for a > type in the clusterconfigmapping will not be able to update the > clusterconfigmapping table. Thus no oozie-site is set with selected = 1 in > the clusterconfigmapping table. > > for( ClusterConfigEntity clusterConfigToMakeSelected : > clusterConfigsToMakeSelected ){ > LOG.info("Checking " + clusterConfigToMakeSelected.getType() + " with tag " + > clusterConfigToMakeSelected.getTag()); > for (ClusterConfigMappingEntity configMappingEntity : configMappingEntities) { > String tag = configMappingEntity.getTag(); > String type = configMappingEntity.getType(); > > if (clusterConfigToMakeSelected.getTag().equals(tag) > && clusterConfigToMakeSelected.getType().equals(type)) > { LOG.info(type + " with tag " + tag + " is selected"); > configMappingEntity.setSelected(1); } > > } > } > > > Diffs > ----- > > > ambari-server/src/main/java/org/apache/ambari/server/orm/dao/ClusterDAO.java > 6e55128 > > ambari-server/src/main/java/org/apache/ambari/server/orm/entities/ClusterConfigEntity.java > 899ffe8 > > ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClusterImpl.java > a48fb54 > > ambari-server/src/test/java/org/apache/ambari/server/orm/dao/ServiceConfigDAOTest.java > 4c186b5 > > Diff: https://reviews.apache.org/r/38269/diff/ > > > Testing > ------- > > unit test: added more test cases to ambari-server code > test on a rolling upgrade then downgrade. > > > Thanks, > > Di Li > >
