Re: Review Request 52176: GEODE-1926: Modification of peedAhead() function check if heapCopy is successful before adding the key to peekedIds

2016-09-24 Thread xiaojian zhou
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52176/#review150325 --- Ship it! A good fix with very clean description. - xiaojian

Re: Review Request 52176: GEODE-1926: Modification of peedAhead() function check if heapCopy is successful before adding the key to peekedIds

2016-09-23 Thread Dan Smith
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52176/#review150271 --- Ship it! Ship It! - Dan Smith On Sept. 23, 2016, 9:58 p.m.,

Re: Review Request 52176: GEODE-1926: Modification of peedAhead() function check if heapCopy is successful before adding the key to peekedIds

2016-09-23 Thread Dan Smith
> On Sept. 22, 2016, 11:02 p.m., Dan Smith wrote: > > geode-core/src/main/java/org/apache/geode/internal/cache/wan/serial/SerialGatewaySenderQueue.java, > > line 796 > > > > > > Minor nit - can't you just write

Re: Review Request 52176: GEODE-1926: Modification of peedAhead() function check if heapCopy is successful before adding the key to peekedIds

2016-09-23 Thread nabarun nag
> On Sept. 22, 2016, 11:02 p.m., Dan Smith wrote: > > geode-core/src/main/java/org/apache/geode/internal/cache/wan/serial/SerialGatewaySenderQueue.java, > > line 796 > > > > > > Minor nit - can't you just write

Re: Review Request 52176: GEODE-1926: Modification of peedAhead() function check if heapCopy is successful before adding the key to peekedIds

2016-09-23 Thread nabarun nag
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52176/#review150244 --- I was able to find paralleWANConflationDUnitTest but no

Re: Review Request 52176: GEODE-1926: Modification of peedAhead() function check if heapCopy is successful before adding the key to peekedIds

2016-09-23 Thread nabarun nag
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52176/ --- (Updated Sept. 23, 2016, 9:58 p.m.) Review request for geode, Barry Oglesby,

Re: Review Request 52176: GEODE-1926: Modification of peedAhead() function check if heapCopy is successful before adding the key to peekedIds

2016-09-23 Thread Dan Smith
> On Sept. 22, 2016, 11:02 p.m., Dan Smith wrote: > > geode-core/src/main/java/org/apache/geode/internal/cache/wan/serial/SerialGatewaySenderQueue.java, > > line 796 > > > > > > Minor nit - can't you just write

Re: Review Request 52176: GEODE-1926: Modification of peedAhead() function check if heapCopy is successful before adding the key to peekedIds

2016-09-23 Thread nabarun nag
> On Sept. 23, 2016, 8:29 p.m., Jason Huynh wrote: > > geode-core/src/main/java/org/apache/geode/internal/cache/wan/serial/SerialGatewaySenderQueue.java, > > line 796 > > > > > > +1 on the less confusing part

Re: Review Request 52176: GEODE-1926: Modification of peedAhead() function check if heapCopy is successful before adding the key to peekedIds

2016-09-23 Thread nabarun nag
> On Sept. 22, 2016, 11:02 p.m., Dan Smith wrote: > > Looks like a good fix for a really complicated issue! > > > > I had a couple of minor nitpicks, below. Also, is there a reason why you > > did not remove the code to > > ((GatewaySenderEventImpl)object).makeHeapCopyIfOffHeap() in > >

Re: Review Request 52176: GEODE-1926: Modification of peedAhead() function check if heapCopy is successful before adding the key to peekedIds

2016-09-23 Thread Jason Huynh
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52176/#review150201 --- Not sure what an appropriate test would be for this, maybe create