----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54488/#review158365 -----------------------------------------------------------
ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClusterImpl.java (lines 3084 - 3105) <https://reviews.apache.org/r/54488/#comment229128> ReviewBoard doesn't make it easy to see what changed. Essentially we're taking the in-memory ClusterConfigMappings and finding the most recent. Then we're iterating over that list, setting them as selected. I build a Map of the entries so it's easier to pick them out later (prevents a double-for loop). Because the entity references are the same in the different collections, we can modify them directly. One question you might ask is why we can't use JPQL for this. See my comment below; it would be a lot more work b/c not all DBs support a nested SELECT with a compound IN clause. - Jonathan Hurley On Dec. 7, 2016, 1:30 p.m., Jonathan Hurley wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/54488/ > ----------------------------------------------------------- > > (Updated Dec. 7, 2016, 1:30 p.m.) > > > Review request for Ambari, Alejandro Fernandez and Nate Cole. > > > Bugs: AMBARI-19130 > https://issues.apache.org/jira/browse/AMBARI-19130 > > > Repository: ambari > > > Description > ------- > > During a downgrade which crosses major stack versions (such as from HDP 2.y > to HDP 2.x), Ambari attempts to set the latest configurations from HDP 2.x as > "current". However, after the downgrade succeeds, the database fails with the > following consistency check: > > {code} > ERROR - You have config(s), in cluster c1, that is(are) selected more than > once in clusterconfigmapping table: > ranger-storm-plugin-properties,mapred-env,webhcat-env,hive-exec-log4j,hive-log4j,webhcat-log4j,storm-env,yarn-log4j,hcat-env > {code} > > The problem is actually not with {{clusterconfig}} but with the state of the > {{clusterconfigmapping}} table _before_ upgrade. Apparently, it is possible > to have multiple mappings for the same config. We match on the config type > and tag (such as hdfs-site / version1234566789). There should never, EVER be > duplicate mappings for the same config and version tag. > > The current downgrade code doesn't "break" after finding its first match: > ``` > for(ClusterConfigMappingEntity configMappingEntity: configMappingEntities){ > String type = configMappingEntity.getType(); > String tag = configMappingEntity.getTag(); > > for (ClusterConfigMappingEntity latest : latestConfigMappingByStack) { > String latestType = latest.getType(); > String latestTag = latest.getTag(); > > // find the latest config of a given mapping entity > if (StringUtils.equals(type, latestType) && StringUtils.equals(tag, > latestTag)) { > LOG.info("{} with version tag {} is selected for stack {}", type, tag, > stackId.toString()); > configMappingEntity.setSelected(1); > } > } > } > ``` > > I'm not sure that breaking will work here since we don't know the ordering. > We could order the results and then break on the first match. In any event, > it's a problem. But it's only a problem for the latest version of a config > since that's what we're going to try to make selected. > > STR: > # Install a cluster, and instrument the {{clusterconfigmapping}} table to > have multiple mappings for a given type (with only 1 currently selected). > # Upgrade and then downgrade > > > Diffs > ----- > > > ambari-server/src/main/java/org/apache/ambari/server/orm/entities/ClusterConfigMappingEntity.java > 04c6030 > > ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClusterImpl.java > 7bf24ce > > ambari-server/src/test/java/org/apache/ambari/server/orm/dao/ServiceConfigDAOTest.java > 2388c11 > > ambari-server/src/test/java/org/apache/ambari/server/state/cluster/ClusterTest.java > 90a3d02 > > Diff: https://reviews.apache.org/r/54488/diff/ > > > Testing > ------- > > mvn clean test > > > Thanks, > > Jonathan Hurley > >
