[GitHub] [geode] albertogpz commented on a change in pull request #6663: GEODE-9408: Avoid duplicate events sent by Serial Gateway Sender when…

[GitBox <gi...@apache.org>]

albertogpz commented on a change in pull request #6663:
URL: https://github.com/apache/geode/pull/6663#discussion_r681977195



##########
File path: geode-core/src/main/java/org/apache/geode/internal/cache/wan/parallel/ParallelGatewaySenderQueue.java
##########
@@ -1404,13 +1404,19 @@ void peekEventsFromIncompleteTransactions(List<GatewaySenderEventImpl> batch,
         }
       }
       if (incompleteTransactionIdsInBatch.size() == 0 ||
-          retries++ == GET_TRANSACTION_EVENTS_FROM_QUEUE_RETRIES) {
+          retries >= sender.getRetriesToGetTransactionEventsFromQueue()) {
         break;
       }
+      retries++;

Review comment:
       @agingade I have given a bit of thought about using wait and notify instead of sleep but I have come to the conclusion that it is not a good idea in this case.
   According to your suggestion, instead of `sleep(time)` we could call `wait(timeout)` and then `notify()` whenever an event is added to the queue. This way, every event added to the queue will awake the thread that must look for events in the queue to complete transactions inside a batch.
   The problem is that the event added to the queue that will provoke the awakening of the waiting thread may not be the one required to complete the transaction so the thread could have been awaken in vain.
   If a lot of events are entering in the queue at a high rate, it might be that, a lot of awakes will be required before the events to complete the transaction enter the queue. This would require to increase the value of the retries. But this is dangerous as the WAN replication rate could greatly decrease.
   
   The original implementation of this feature did not contemplate any sleeps. It just retried several times the search in the queue to retrieve events to complete transactions when they were not found, assuming that the events for a transaction should arrive to the queue very close in time. I observed that sometimes the retries were done so quickly that even if the number of retries was high, still some batches had to be sent with incomplete transactions and that is why I changed the behavior in this PR to wait a little before retrying by means of sleep.
   
   If you do not agree with my conclusions, I am open to discuss them with you by e-mail, chat or videoconference.
   




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org