[GitHub] [geode] albertogpz commented on pull request #6659: GEODE-9404: Do not log error message if sender is not configured.

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

albertogpz commented on pull request #6659:
URL: https://github.com/apache/geode/pull/6659#issuecomment-872009426


   > @pivotal-eshu
   > It looks like the Tx code is trying to assess Gateway sender requirement to support GEODE-7971 in TX code. Which doesn't seems right.
   > 
   > The "getLastTransactionEvent" should just return the last event in the transaction; currently it is doing it by "callbacks.get(callbacks.size() - 1);" which also seems to be error prone or inefficient.
   > In "firePendingCallbacks" it could have iterated over the list using size index:
   > for (int i=0; i<callbaack.size(); i++){
   > ee.getRegion().invokeTXCallbacks(EnumListenerEvent.AFTER_DESTROY, ee, true, (i+1) == size() /_isLastTrasaction_/);
   > }
   > 
   > Please correct me if i am missing anything.
   
   It was implemented that way to put the logic of getting the last transaction event in another class although it is as simple as getting the last element.
   Also, this logic could detect misconfigurations that can only be detected at runtime (second check in `getLastTransactionEvent()`), then the caller of getLastTransactionEvent() could detect it by means of the exception and set all the events as the last in the transaction. Why do this? because if not all events in the transaction go to the same senders (misconfiguration), that would imply that some senders will not get all the events for the transaction but could try if group-transaction-events is set to complete the transaction in the batch, by looking for the last event in the transaction which could never come (delaying the batch sending) or may come but not guaranteeing that the rest of the events for the transaction were sent in the batch (because they may not have reached the sender).
   
   The first check of `getLastTransactionEvent()` was added in order to avoid the cost of the second check if no sender had group-transaction-events set to true. In that case, it is irrelevant to return the last transaction event or null because it will not be used although, it could make more sense to return the last transaction event.


-- 
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