> On Aug. 7, 2014, 3:41 a.m., Bill Farner wrote: > > src/main/resources/org/apache/aurora/scheduler/storage/db/JobInstanceUpdateEventMapper.xml, > > line 28 > > <https://reviews.apache.org/r/24431/diff/1/?file=654336#file654336line28> > > > > If we're going to generate our own IDs, we should really let those be > > the foreign key references. However, i think now is a good time to revisit > > using the DB's generated ID. > > > > As far as i can tell, the current structure of JobUpdate works with > > this, since a JobUpdate is self-identifying. > > > > For objects that are not self-identifying, it looks like this would > > work: > > > > http://stackoverflow.com/questions/18507508/mybatis-how-to-get-the-auto-generated-key-of-an-insert-mysql > > > > In this case, insert() accepts Map<String, Object>, where one key > > points to the POJO, the other points to the returned autogenerated key. > > You would need this snippet below the insert statement: > > > > <selectKey resultType="long" order="AFTER" keyProperty="returnedId"> > > SELECT LAST_INSERT_ID() > > </selectKey> > > > > Not terribly elegant, but avoids broader exposure of this limitation. > > > > Bill Farner wrote: > I played around with this idea on top of your patch, and it does work as > expected. The exception, of course, is MERGE. In that case, you need a > small workaround: > > MERGE INTO table ( > id, > ... > } VALUES ( > <if test="id == 0"> > null > </if> > <if test="id != 0"> > #{id} > </if>, > )
I have actually made it work for task configs without a selectKey. I am passing a Result object into the insert() instead of Map. Will use this approach in the next CR. However, I am still concerned about it for updates. Relying on auto-generated keys requires stable IDs to guarantee correct relationships between tables. Since we re-populate the entire DB on failover, there are no hard guarantees the order will be preserved on re-insertion. Any shift in IDs on re-insertion into job_updates table would abandon/misplace records in event tables. Also, using pre-generated update IDs make tests simpler (i.e. no need to modify saved objects before asserting them against fetched ones). - Maxim ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24431/#review49860 ----------------------------------------------------------- On Aug. 7, 2014, 12:01 a.m., Maxim Khutornenko wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/24431/ > ----------------------------------------------------------- > > (Updated Aug. 7, 2014, 12:01 a.m.) > > > Review request for Aurora, David McLaughlin and Bill Farner. > > > Bugs: AURORA-612 > https://issues.apache.org/jira/browse/AURORA-612 > > > Repository: aurora > > > Description > ------- > > Adding SQL support for saving updates and fetching update details. > > The configuration and updateOnlyTheseInstances option support will come > separately. > > > Diffs > ----- > > src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java > 869590ed3c43c64936af42e60d82f5a6938475d3 > src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java > 5d6521813e40eed5cf7e6e0285a7400624c4c5b2 > src/main/java/org/apache/aurora/scheduler/storage/db/DbStorage.java > 8a573f7b04ecc37a19e3886f334f626af577839a > > src/main/java/org/apache/aurora/scheduler/storage/db/JobInstanceUpdateEventMapper.java > PRE-CREATION > src/main/java/org/apache/aurora/scheduler/storage/db/JobKeyMapper.java > 844714c08cdaf280c8862e933a95ab009bd9e464 > > src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java > PRE-CREATION > > src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateEventMapper.java > PRE-CREATION > src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateMapper.java > PRE-CREATION > src/main/java/org/apache/aurora/scheduler/storage/db/LockKeyMapper.java > 618d5b72fd3078ef59571b5f2e33ecce3eb7386c > > src/main/java/org/apache/aurora/scheduler/storage/db/typehandlers/JobUpdateActionTypeHandler.java > PRE-CREATION > > src/main/java/org/apache/aurora/scheduler/storage/db/typehandlers/JobUpdateStatusTypeHandler.java > PRE-CREATION > > src/main/java/org/apache/aurora/scheduler/storage/db/typehandlers/TypeHandlers.java > f014123348e2b229b2bb9be0dce424327ec0dfbe > > src/main/resources/org/apache/aurora/scheduler/storage/db/JobInstanceUpdateEventMapper.xml > PRE-CREATION > src/main/resources/org/apache/aurora/scheduler/storage/db/JobKeyMapper.xml > 3997f9833a7289a49e4f4972efc3f953674bd41b > > src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml > PRE-CREATION > > src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateEventMapper.xml > PRE-CREATION > > src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateMapper.xml > PRE-CREATION > src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql > eb2216ae22ae95370d0b5cd15dbb3d61fca79272 > > src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/24431/diff/ > > > Testing > ------- > > gradle -Pq build > > > Thanks, > > Maxim Khutornenko > >
