----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53503/#review155168 -----------------------------------------------------------
geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegion.java (line 3968) <https://reviews.apache.org/r/53503/#comment225002> I think your comment would be clearer if you said: // only transactions set allRetry to false geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegion.java (line 3974) <https://reviews.apache.org/r/53503/#comment225004> You should not set the cause to "pde" since it is an internal package that should not be thrown to customer code. It is probably worth at least a comment saying this else will currently never happen since all current constructors of PRLocallyDestroyedException set the cause to RegionDestroyedException. geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegion.java (line 4013) <https://reviews.apache.org/r/53503/#comment225011> I see other places in the code that handle RegionDestroyedException by throwing a TransactionDataNotColocatedException. This code does that for BucketNotFoundException. Why did you chose on a RegionDestroyedException to throw TransactionDataRebalancedException instead of TransactionDataNotColocatedException? geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegion.java (line 4020) <https://reviews.apache.org/r/53503/#comment225008> I think the following wording is clearer: // Make transaction fail so client can retry - Darrel Schneider On Nov. 4, 2016, 3:57 p.m., Eric Shu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/53503/ > ----------------------------------------------------------- > > (Updated Nov. 4, 2016, 3:57 p.m.) > > > Review request for geode, anilkumar gingade and Darrel Schneider. > > > Repository: geode > > > Description > ------- > > Throws TransactionDataRebalancedException when get call on a key in a bucket > that has been moved. > > > Diffs > ----- > > > geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegion.java > 8f67e25 > > geode-core/src/main/java/org/apache/geode/internal/cache/TXStateProxyImpl.java > a89cdc4 > > geode-core/src/test/java/org/apache/geode/internal/cache/execute/PRColocationDUnitTest.java > bedb1d4 > > geode-core/src/test/java/org/apache/geode/internal/cache/execute/PRTransactionDUnitTest.java > 37ea4e5 > > Diff: https://reviews.apache.org/r/53503/diff/ > > > Testing > ------- > > > Thanks, > > Eric Shu > >
