----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38269/#review99227 -----------------------------------------------------------
ambari-server/src/main/java/org/apache/ambari/server/orm/entities/ClusterConfigEntity.java (line 54) <https://reviews.apache.org/r/38269/#comment156163> Instead of double a triple nested SELECT, would it instead make sense to just iterate over the clusterocnfigmapping listings and take the latest of each? So instead of using the clusterconfig table, we're using the clusterconfigmapping table to "apply" the latest configs? ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClusterImpl.java (line 2837) <https://reviews.apache.org/r/38269/#comment156159> Don't use + when logging statements as it constructs a string. You can use {} as a placeholder instead. ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClusterImpl.java (line 2841) <https://reviews.apache.org/r/38269/#comment156161> Don't use + when logging statements as it constructs a string. You can use {} as a placeholder instead. ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClusterImpl.java (line 2850) <https://reviews.apache.org/r/38269/#comment156160> Don't use + when logging statements as it constructs a string. You can use {} as a placeholder instead. ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClusterImpl.java (line 2857) <https://reviews.apache.org/r/38269/#comment156162> Don't use + when logging statements as it constructs a string. You can use {} as a placeholder instead. - Jonathan Hurley On Sept. 15, 2015, 5:12 p.m., Di Li wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/38269/ > ----------------------------------------------------------- > > (Updated Sept. 15, 2015, 5:12 p.m.) > > > Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, and Nate > Cole. > > > 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 > >
