DonalEvans commented on a change in pull request #6474:
URL: https://github.com/apache/geode/pull/6474#discussion_r639129906
##########
File path:
geode-core/src/main/java/org/apache/geode/internal/cache/BucketRegionQueue.java
##########
@@ -203,8 +206,7 @@ private void destroyFailedBatchRemovalMessageKeys() {
@Override
public void beforeAcquiringPrimaryState() {
- Iterator<Object> itr = eventSeqNumDeque.iterator();
- markEventsAsDuplicate(itr);
+ markAsDuplicate.addAll(Arrays.asList(eventSeqNumDeque.toArray()));
Review comment:
I think that this can just be
```
markAsDuplicate.addAll(eventSeqNumDeque);
```
##########
File path:
geode-core/src/main/java/org/apache/geode/internal/cache/BucketRegionQueue.java
##########
@@ -203,8 +206,7 @@ private void destroyFailedBatchRemovalMessageKeys() {
@Override
public void beforeAcquiringPrimaryState() {
- Iterator<Object> itr = eventSeqNumDeque.iterator();
- markEventsAsDuplicate(itr);
Review comment:
With this change, this method is no longer used, and so can be removed.
However, some debug-level logging is lost as a result, so maybe that should be
moved to where you're now calling `setPossibleDuplicate()`.
##########
File path:
geode-core/src/main/java/org/apache/geode/internal/cache/BucketRegionQueue.java
##########
@@ -423,8 +428,14 @@ public Object peek() {
}
key = this.eventSeqNumDeque.peekFirst();
if (key != null) {
+ boolean setDuplicate = markAsDuplicate.remove(key);
+
object = optimalGet(key);
Review comment:
The old `markEventsAsDuplicate()` method used `getNoLRU()` to retrieve
the object and determine if `setPossibleDuplicate()` should be called on it.
Will using `optimalGet()` here result in the same behaviour?
##########
File path:
geode-core/src/test/java/org/apache/geode/internal/cache/BucketRegionQueueJUnitTest.java
##########
@@ -199,4 +256,34 @@ GatewaySenderEventImpl createMockGatewaySenderEvent(Object
key, TransactionId tI
when(event.getKey()).thenReturn(key);
return event;
}
+
+ private GatewaySenderEventImpl createGatewaySenderEvent(LocalRegion lr,
Operation operation,
+ Object key, Object value, long threadId, long sequenceId)
+ throws Exception {
+ when(lr.getKeyInfo(key, value, null)).thenReturn(new KeyInfo(key, null,
null));
+ when(lr.getTXId()).thenReturn(null);
+
+
+
Review comment:
Some unnecessary line breaks here.
##########
File path:
geode-core/src/main/java/org/apache/geode/internal/cache/BucketRegionQueue.java
##########
@@ -668,4 +679,27 @@ public boolean isReadyForPeek() {
&& !this.eventSeqNumDeque.isEmpty() && getBucketAdvisor().isPrimary();
}
+ List<Object> getQueueList() {
+ getInitializationLock().readLock().lock();
+ try {
+ if (this.getPartitionedRegion().isDestroyed()) {
+ throw new BucketRegionQueueUnavailableException();
+ }
+ List<Object> elementsInQueue = new ArrayList<>();
+ Iterator<Object> it = this.eventSeqNumDeque.iterator();
+ while (it.hasNext()) {
+ Object key = it.next();
+ Object event = optimalGet(key);
+
+ if (!(event instanceof InternalGatewayQueueEvent)) {
+ continue;
+ }
+ elementsInQueue.add(event);
+ }
+ return elementsInQueue;
Review comment:
Purely personal preference, but this can be simplified significantly
using streams:
```
return eventSeqNumDeque.stream()
.map(this::optimalGet)
.filter(o -> o instanceof InternalGatewayQueueEvent)
.collect(Collectors.toList());
```
##########
File path:
geode-core/src/test/java/org/apache/geode/internal/cache/BucketRegionQueueJUnitTest.java
##########
@@ -199,4 +256,34 @@ GatewaySenderEventImpl createMockGatewaySenderEvent(Object
key, TransactionId tI
when(event.getKey()).thenReturn(key);
return event;
}
+
+ private GatewaySenderEventImpl createGatewaySenderEvent(LocalRegion lr,
Operation operation,
+ Object key, Object value, long threadId, long sequenceId)
+ throws Exception {
+ when(lr.getKeyInfo(key, value, null)).thenReturn(new KeyInfo(key, null,
null));
+ when(lr.getTXId()).thenReturn(null);
+
+
+
+ EntryEventImpl eei = EntryEventImpl.create(lr, operation, key, value,
null, false, null);
+ eei.setEventId(new EventID(new byte[16], threadId, sequenceId));
+ GatewaySenderEventImpl gsei =
+ new GatewaySenderEventImpl(getEnumListenerEvent(operation), eei, null,
true, false);
+ return gsei;
Review comment:
This can be simplified to
```
return new GatewaySenderEventImpl(getEnumListenerEvent(operation), eei,
null, true, false);
```
##########
File path:
geode-core/src/main/java/org/apache/geode/internal/cache/BucketRegionQueue.java
##########
@@ -668,4 +679,27 @@ public boolean isReadyForPeek() {
&& !this.eventSeqNumDeque.isEmpty() && getBucketAdvisor().isPrimary();
}
+ List<Object> getQueueList() {
Review comment:
This method should probably be marked as `@VisibleForTesting` and maybe
renamed to make it clear that it's a test helped method.
--
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]
