You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@geode.apache.org by GitBox <gi...@apache.org> on 2021/07/29 01:53:06 UTC
[GitHub] [geode] agingade commented on a change in pull request #6663: GEODE-9408: Avoid duplicate events sent by Serial Gateway Sender when…
agingade commented on a change in pull request #6663:
URL: https://github.com/apache/geode/pull/6663#discussion_r678747476
##########
File path: geode-core/src/main/java/org/apache/geode/internal/cache/wan/AbstractGatewaySender.java
##########
@@ -710,7 +718,12 @@ public void setGatewayEventFilters(List<GatewayEventFilter> filters) {
} else {
this.eventFilters = Collections.unmodifiableList(filters);
}
- };
+ }
+
+ @Override
+ public void setRetriesToGetTransactionEventsFromQueue(int retries) {
+ this.retriesToGetTransactionEventsFromQueue = retries;
Review comment:
There is effort not to use "this" in the geode code; if you like you can cleanup/refactor use of "this" in this class.
##########
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:
I am assuming this is a best effort to find missing transaction events that are getting added to the Queue. If thats the case, can it be done based on wait/notify on Queue? Right now the default wait is for 1ms; but in case if someone sets it longer than wait/notify will be efficient.
Also why is the default 1ms; isn't it too small...
##########
File path: geode-core/src/main/java/org/apache/geode/cache/wan/GatewaySender.java
##########
@@ -174,7 +174,15 @@
*/
int GET_TRANSACTION_EVENTS_FROM_QUEUE_RETRIES =
Integer.getInteger(GeodeGlossary.GEMFIRE_PREFIX + "get-transaction-events-from-queue-retries",
- 10);
+ 3);
+ /**
+ * Milliseconds to wait before retrying to get events for a transaction from the
+ * gateway sender queue when group-transaction-events is true.
+ */
+ int GET_TRANSACTION_EVENTS_FROM_QUEUE_WAIT_TIME_MS =
Review comment:
Please use SystemPropertyHelper for setting system properties....
E.g.:
SystemPropertyHelper
.getProductIntegerProperty(SystemPropertyHelper.EVICTION_SCAN_THRESHOLD_PERCENT);
Look for use of VICTION_SCAN_THRESHOLD_PERCENT.
##########
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++;
+ try {
+ Thread.sleep(GET_TRANSACTION_EVENTS_FROM_QUEUE_WAIT_TIME_MS);
+ } catch (InterruptedException e) {
+ Thread.currentThread().interrupt();
+ }
}
if (incompleteTransactionIdsInBatch.size() > 0) {
- logger.warn("Not able to retrieve all events for transactions: {} after {} retries",
- incompleteTransactionIdsInBatch, retries);
+ logger.warn("Not able to retrieve all events for transactions: {} after {} retries of {}ms" +
+ incompleteTransactionIdsInBatch, retries, GET_TRANSACTION_EVENTS_FROM_QUEUE_WAIT_TIME_MS);
Review comment:
Curious, in the code other than logging nothing is done to handle in-complete-trasactions; is it OK to send the events with incomplete transactions...
##########
File path: geode-core/src/main/java/org/apache/geode/cache/wan/GatewaySender.java
##########
@@ -500,4 +516,14 @@
*/
void setGatewayEventFilters(List<GatewayEventFilter> filters);
+ /**
+ * Set the number of retries to get transaction events
+ * for this GatewaySender when GroupTransactionEvents
+ * is set.
+ *
+ * @since Geode 1.15
Review comment:
Not sure this is practiced anymore; if you want to use make it consistent across other methods.
##########
File path: geode-core/src/main/java/org/apache/geode/cache/wan/GatewaySender.java
##########
@@ -174,7 +174,15 @@
*/
int GET_TRANSACTION_EVENTS_FROM_QUEUE_RETRIES =
Integer.getInteger(GeodeGlossary.GEMFIRE_PREFIX + "get-transaction-events-from-queue-retries",
- 10);
+ 3);
+ /**
+ * Milliseconds to wait before retrying to get events for a transaction from the
+ * gateway sender queue when group-transaction-events is true.
+ */
+ int GET_TRANSACTION_EVENTS_FROM_QUEUE_WAIT_TIME_MS =
+ Integer.getInteger(
+ GeodeGlossary.GEMFIRE_PREFIX + "get-transaction-events-from-queue-wait-time-ms",
Review comment:
Please use SystemPropertyHelper for setting system properties....
E.g.:
SystemPropertyHelper
.getProductIntegerProperty(SystemPropertyHelper.EVICTION_SCAN_THRESHOLD_PERCENT);
Look for use of VICTION_SCAN_THRESHOLD_PERCENT.
--
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