[
https://issues.apache.org/jira/browse/GEODE-8971?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17292825#comment-17292825
]
ASF GitHub Bot commented on GEODE-8971:
---------------------------------------
albertogpz commented on pull request #6052:
URL: https://github.com/apache/geode/pull/6052#issuecomment-787864365
> I've requested a few changes in-line and a couple general comments that
aren't requests for changes.
>
> A good rule is to always avoid raw types (`Predicate condition`) by
providing a type (`Predicate<Object> condition`).
>
> I really want to see us moving away from passing around impl classes like
`GatewaySenderEventImpl` if an interface like `GatewayQueueEvent` defines the
methods you need to call. If you need to call an impl method that's NOT on the
interface, then use the impl for now. We'll probably eventually create a new
internal interface like `InternalGatewayQueueEvent` if we find that we need to
pass around the impl to call non-interface-defined methods. This will help us
avoid concrete dependencies which then helps produce higher quality code and
unit tests.
>
> Looks like the existing `throw` statements for
`BucketRegionQueueUnavailableException` all have no message. It's always better
to provide a message, but it's ok as is since it's at least consistent with the
other lines that throw it.
> I've requested a few changes in-line and a couple general comments that
aren't requests for changes.
>
> A good rule is to always avoid raw types (`Predicate condition`) by
providing a type (`Predicate<Object> condition`).
>
> I really want to see us moving away from passing around impl classes like
`GatewaySenderEventImpl` if an interface like `GatewayQueueEvent` defines the
methods you need to call. If you need to call an impl method that's NOT on the
interface, then use the impl for now. We'll probably eventually create a new
internal interface like `InternalGatewayQueueEvent` if we find that we need to
pass around the impl to call non-interface-defined methods. This will help us
avoid concrete dependencies which then helps produce higher quality code and
unit tests.
>
> Looks like the existing `throw` statements for
`BucketRegionQueueUnavailableException` all have no message. It's always better
to provide a message, but it's ok as is since it's at least consistent with the
other lines that throw it.
I have added the isLastEventInTransaction() and getTransactionId() to the
GatewayQueueEvent interface. Do you see any drawback to it?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Batches with incomplete transactions when stopping the gateway sender
> ---------------------------------------------------------------------
>
> Key: GEODE-8971
> URL: https://issues.apache.org/jira/browse/GEODE-8971
> Project: Geode
> Issue Type: Improvement
> Components: wan
> Affects Versions: 1.14.0
> Reporter: Alberto Gomez
> Assignee: Alberto Gomez
> Priority: Major
> Labels: pull-request-available
>
> When the gateway sender is stopped there is a high probability that batches
> with incomplete transactions are sent even if group-transaction-events is
> enabled.
> The reason is that once the stop command reaches the gateway sender, it
> immediately stops queueing events, and this could happen in the middle of
> receiving events for the same transaction. If this is the case, some events
> for the transaction may have reached the queue right before the stop command
> was received and the rest of events for that transaction would not make it to
> the queue (they would be dropped) because they arrived right after the stop
> command was received at the gateway sender.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
