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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
