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

[nabarun nag <nn...@pivotal.io>]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52176/
-----------------------------------------------------------

(Updated Oct. 7, 2016, 10:01 p.m.)


Review request for geode, Barry Oglesby, Eric Shu, Jason Huynh, Dan Smith, and xiaojian zhou.


Repository: geode


Description
-------

Before my changes:
1. peek calls peekAhead to get the object from the sender queue to be put into the batch for dispatch.
2. peekAhead gets the current key for which it is able to get an object back by calling optimalGet.
3. It puts that current key into the peekedIds list and returns the object back to peek.
4. peek then tries to make a heapcopy and only if it is successful it will put the object into the dispatch batch.
5. Here is the issue, now conflation may kick in before peek is able to do heapcopy and that object is removed from the queue and hence the object will not be placed in the dispatch batch. However the the key representing that object in the queue still exist in the PeekedIDs list.
6. So now there is an extra key in the peekedIds list.
7. Batch is dispatched and now the ack is received, so things need to be removed from the sender queue.
8. remove() is called in a for loop with the count set to size of dispatch batch for which ack was received. 
9. The remove() operation picks up keys from peekedIds list sequentially and calls remove(key) on the queue.
10. Now since the batch size is smaller than the Ids peeked, element will still linger behind after all remove calls are completed.
11. so tests which wait for queues to be empty will hang forever.

Solution:
Since peek() doesnt have access to the current key but just the object and hence cannot remove it from peekedIDs list, we moved the check for heapcopy into peekAhead. 

So now only if a successful heap copy is made then only the key will be placed into the peekedIDs list.


Diffs (updated)
-----

  geode-core/src/main/java/org/apache/geode/internal/cache/wan/serial/SerialGatewaySenderQueue.java a8bb72d 
  geode-wan/src/test/java/org/apache/geode/internal/cache/wan/serial/SerialWANConflationDUnitTest.java PRE-CREATION 

Diff: https://reviews.apache.org/r/52176/diff/


Testing
-------

precheck


Thanks,

nabarun nag