> On Feb. 9, 2016, 6:40 p.m., Alejandro Fernandez wrote: > > ambari-server/src/main/java/org/apache/ambari/server/upgrade/UpgradeCatalog240.java, > > line 130 > > <https://reviews.apache.org/r/42838/diff/1/?file=1231923#file1231923line130> > > > > This should be idempotent. If anything fails in this file, we should be > > able to retry. > > Please add a check that the table doesn't already exist. > > Before inserting those 2 rows, can check if they already exist. > > Nahappan Somasundaram wrote: > SchemaUpgradeHelper::main() or at least > SchemaUpgradeHelper::executeUpgrade() should be transactional. If anything > fails, we should be able to leave the DB in the state it was found, instead > of a partially upgraded schema, which will break the system if the upgrade > is not run again. It is cumbersome to verify each upgrade for partial > execution before re-running the upgrade. > > If an upgrade was successful, re-running the upgrade should exit with a > message. I suppose this is already happening. > > Ajit Kumar wrote: > I agree with Nahappan here. Ideal way to deal with the problem is make > sure db changes are transactional. I looked at other UpgradeCatalog*.java and > there is no validation during table creation if table already exists. If > current implementation of UpgradeCatalog can leave DB in inconsistent/partial > upgrade state, we should handle that as a separate task.
The changes are not transactional. This is just good practice, check if the table and records exist before attempting to insert them. Also, if another unrelated change fails as part of the upgrade and this change for Settings was already done and the upgrade must be re-ran, we don't want it to fail. This is a very critical part of Ambari. - Alejandro ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42838/#review118429 ----------------------------------------------------------- On Feb. 4, 2016, 12:36 a.m., Ajit Kumar wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/42838/ > ----------------------------------------------------------- > > (Updated Feb. 4, 2016, 12:36 a.m.) > > > Review request for Ambari, Jayush Luniya, Nahappan Somasundaram, and Sumit > Mohanty. > > > Bugs: AMBARI-14912 > https://issues.apache.org/jira/browse/AMBARI-14912 > > > Repository: ambari > > > Description > ------- > > Update upgrade catalog 240 and add db schema changes for settings table. > > > Diffs > ----- > > > ambari-server/src/main/java/org/apache/ambari/server/upgrade/UpgradeCatalog240.java > 3414388b9a738a4fc7cc5d0f484fe553cce840ee > > ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog240Test.java > d1d68f24871422936321ad0c940499fc1ef4cbec > > Diff: https://reviews.apache.org/r/42838/diff/ > > > Testing > ------- > > Unit testing. Currently my local trunk build is breaking even w/o my changes. > I'll run mvn clean install when it is fixed. > > > Thanks, > > Ajit Kumar > >
