Bill commented on a change in pull request #6362:
URL: https://github.com/apache/geode/pull/6362#discussion_r623266221
##########
File path:
geode-core/src/main/java/org/apache/geode/internal/cache/event/DistributedEventTracker.java
##########
@@ -356,22 +363,58 @@ public boolean hasSeenEvent(EventID eventID,
InternalCacheEvent tagHolder) {
if (evh.isRemoved() || evh.getLastSequenceNumber() <
eventID.getSequenceID()) {
return false;
}
- // log at fine because partitioned regions can send event multiple times
- // during normal operation during bucket region initialization
- if (logger.isTraceEnabled(LogMarker.DISTRIBUTION_BRIDGE_SERVER_VERBOSE))
{
- logger.trace(LogMarker.DISTRIBUTION_BRIDGE_SERVER_VERBOSE,
- "Cache encountered replay of event with ID {}. Highest recorded
for this source is {}",
- eventID, evh.getLastSequenceNumber());
+ if (shouldLogPreviouslySeenEvent(tagHolder, evh)) {
+ logger.info(EVENT_HAS_PREVIOUSLY_BEEN_SEEN, region.getName(),
+ tagHolder == null ? "unknown" : ((EntryEventImpl)
tagHolder).getKey(),
+ tagHolder == null ? "unknown" : tagHolder.getOperation(),
eventID.expensiveToString(),
+ evh.getLastSequenceNumber());
}
// bug #44956 - recover version tag for duplicate event
if (evh.getLastSequenceNumber() == eventID.getSequenceID() && tagHolder
!= null
&& evh.getVersionTag() != null) {
((EntryEventImpl) tagHolder).setVersionTag(evh.getVersionTag());
}
+
+ // Increment the previously seen events statistic
+ region.getCachePerfStats().incPreviouslySeenEvents();
+
return true;
}
}
+ private boolean shouldLogPreviouslySeenEvent(InternalCacheEvent event,
+ EventSequenceNumberHolder evh) {
+ boolean shouldLogSeenEvent = true;
+ String message = null;
+ if (event != null && ((EntryEventImpl) event).isPossibleDuplicate()) {
+ // Ignore the previously seen event if it is a possible duplicate
+ message = "possible duplicate";
+ shouldLogSeenEvent = false;
+ } else if (region instanceof BucketRegion) {
+ BucketRegion br = (BucketRegion) region;
+ if (br.hasLowRedundancy()) {
+ // Ignore the previously seen event while the bucket has low redundancy
+ message = "low redundancy";
+ shouldLogSeenEvent = false;
+ } else if (br.getPartitionedRegion().areRecoveriesInProgress()) {
+ // Ignore the previously seen event while recoveries are in progress
+ message = "recoveries in progress";
+ shouldLogSeenEvent = false;
+ }
+ }
+ if (!shouldLogSeenEvent && logger.isDebugEnabled()) {
+ if (event == null) {
+ logger.debug("Ignoring previously seen event due to {}", message);
+ } else {
+ logger.debug(
+ "Ignoring previously seen event due to {} for region={};
operation={}; key={}; eventId={}; highestSequenceNumberSeen={}",
+ message, region.getName(), ((EntryEventImpl) event).getKey(),
event.getOperation(),
+ event.getEventId().expensiveToString(),
evh.getLastSequenceNumber());
Review comment:
It seems odd that a method whose purpose is to decide whether or not to
log something, is itself logging something (as a side-effect). Wouldn't it make
more sense to put all the logging in the caller?
The effect is (as I read it) we are logging something either way (whether we
`shouldLogSeenEvent` or not).
--
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:
[email protected]
