> On Oct. 25, 2012, 2:59 a.m., Alex Huang wrote: > > Nicely written unit test. You should add one to test what happens when the > > transaction is reused. For every bug we find, we should write a unit test > > for it. Also document in the unit test what you are testing for so other > > people who read the unit test understands why it is necessary. > > > > Thanks. > > > > Also I think the transition should happen before the txn closed for > > symmetry with the txn open and then transit to user managed.
Regarding ordering transitToAutoManagedConnection call before txn.close(), I am not quite convinced. Since txn.close() has some code to do rollback in removeUpTo subroutine, which is directly invoke _conn.rollback. If we invoke transitToAutoManagedConnection first, we may encounter null pointer exception. - Min ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/7726/#review12754 ----------------------------------------------------------- On Oct. 25, 2012, 12:02 a.m., Min Chen wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/7726/ > ----------------------------------------------------------- > > (Updated Oct. 25, 2012, 12:02 a.m.) > > > Review request for cloudstack and Alex Huang. > > > Description > ------- > > This patch is to fix this bug > https://issues.apache.org/jira/browse/CLOUDSTACK-409 > > > This addresses bug CLOUDSTACK-409. > > > Diffs > ----- > > server/src/com/cloud/api/ApiServlet.java 8a1d4de > server/src/com/cloud/api/response/ApiResponseSerializer.java 1429d14 > server/src/com/cloud/cluster/ClusterManagerImpl.java 4dbb16c > utils/src/com/cloud/utils/db/Transaction.java bcf7ae1 > utils/test/com/cloud/utils/db/DbTestDao.java PRE-CREATION > utils/test/com/cloud/utils/db/DbTestVO.java PRE-CREATION > utils/test/com/cloud/utils/db/TransactionTest.java PRE-CREATION > > Diff: https://reviews.apache.org/r/7726/diff/ > > > Testing > ------- > > Testing is done locally. > > > Thanks, > > Min Chen > >
