[jira] [Commented] (GEODE-8971) Batches with incomplete transactions when stopping the gateway sender

2021-03-09 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/GEODE-8971?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17298575#comment-17298575
 ] 

ASF GitHub Bot commented on GEODE-8971:
---

albertogpz commented on a change in pull request #6052:
URL: https://github.com/apache/geode/pull/6052#discussion_r591130988



##
File path: 
geode-wan/src/distributedTest/java/org/apache/geode/internal/cache/wan/parallel/ParallelWANStatsDUnitTest.java
##
@@ -472,6 +473,200 @@ public void 
testPRParallelPropagationWithGroupTransactionEventsSendsBatchesWithC
 
   }
 
+  @Test
+  public void 
testPRParallelPropagationWithGroupTransactionEventsDoesNotSendBatchesWithIncompleteTransactionsIfGatewaySenderIsStoppedWhileReceivingTrafficAndLaterStarted()
+  throws InterruptedException {
+Integer lnPort = vm0.invoke(() -> 
WANTestBase.createFirstLocatorWithDSId(1));
+Integer nyPort = vm1.invoke(() -> WANTestBase.createFirstRemoteLocator(2, 
lnPort));
+
+createCacheInVMs(nyPort, vm2);
+createReceiverCustomerOrderShipmentPR(vm2);
+createReceiverInVMs(vm2);
+
+createCacheInVMs(lnPort, vm4, vm5);
+createSenderCustomerOrderShipmentPRs(vm4);
+createSenderCustomerOrderShipmentPRs(vm5);
+
+int batchSize = 10;
+vm4.invoke(
+() -> WANTestBase.createSender("ln", 2, true, 100, batchSize, false, 
true, null, false,
+true));
+vm5.invoke(
+() -> WANTestBase.createSender("ln", 2, true, 100, batchSize, false, 
true, null, false,
+true));
+
+int customers = 4;
+int transactionsPerCustomer = 100;
+// Each transaction will contain one order plus the following shipments
+int shipmentsPerTransaction = batchSize;
+AsyncInvocation inv1 = asyncExecuteCustomerTransactions(vm4, 
customers,
+transactionsPerCustomer, shipmentsPerTransaction);
+
+// wait for some batches to be distributed and then stop the sender
+vm4.invoke(() -> await()
+.until(() -> WANTestBase.getSenderStats("ln", -1).get(4) > 0));
+
+stopSenderInVMsAsync("ln", vm4, vm5);
+
+// Wait for customer transactions to finish
+inv1.await();
+int orderEntries = transactionsPerCustomer * customers;
+int shipmentEntries = orderEntries * 10;
+vm4.invoke(() -> WANTestBase.validateRegionSize(orderRegionName, 
orderEntries));
+vm5.invoke(() -> WANTestBase.validateRegionSize(orderRegionName, 
orderEntries));
+vm4.invoke(() -> WANTestBase.validateRegionSize(shipmentRegionName, 
shipmentEntries));
+vm5.invoke(() -> WANTestBase.validateRegionSize(shipmentRegionName, 
shipmentEntries));
+
+checkOnlyCompleteTransactionsAreReplicatedAfterSenderStopped(
+shipmentsPerTransaction);
+
+// Start sender to validate that queued events do not contain incomplete 
transactions after
+// restart
+startSenderInVMsAsync("ln", vm4, vm5);
+
+
checkOnlyCompleteTransactionsAreReplicatedWithSenderRestarted(shipmentsPerTransaction);
+  }
+
+  @Test
+  public void 
testPRParallelPropagationWithGroupTransactionEventsDoesNotSendBatchesWithIncompleteTransactionsIfGatewaySenderIsStoppedWhileReceivingTrafficAndLaterStartedReceiverStopped()
+  throws InterruptedException {
+Integer lnPort = vm0.invoke(() -> 
WANTestBase.createFirstLocatorWithDSId(1));
+Integer nyPort = vm1.invoke(() -> WANTestBase.createFirstRemoteLocator(2, 
lnPort));
+
+createCacheInVMs(nyPort, vm2);
+createReceiverCustomerOrderShipmentPR(vm2);
+createReceiverInVMs(vm2);
+vm2.invoke(WANTestBase::stopReceivers);
+
+createCacheInVMs(lnPort, vm4, vm5);
+createSenderCustomerOrderShipmentPRs(vm4);
+createSenderCustomerOrderShipmentPRs(vm5);
+
+int batchSize = 10;
+vm4.invoke(() -> WANTestBase.createSender("ln", 2, true, 100, 10, false, 
true, null, false,
+true));
+vm5.invoke(() -> WANTestBase.createSender("ln", 2, true, 100, 10, false, 
true, null, false,
+true));
+
+int customers = 4;
+int transactionsPerCustomer = 100;
+// Each transaction will contain one order plus the following shipments
+int shipmentsPerTransaction = batchSize;
+
+AsyncInvocation inv1 = asyncExecuteCustomerTransactions(vm4, 
customers,
+transactionsPerCustomer, shipmentsPerTransaction);
+
+// wait for some batches to be redistributed and then stop the sender
+vm4.invoke(() -> await()
+.until(() -> WANTestBase.getSenderStats("ln", -1).get(5) > 0));
+
+stopSenderInVMsAsync("ln", vm4, vm5);
+
+// Wait for the customer transactions to finish
+inv1.await();
+int orderEntries = transactionsPerCustomer * customers;
+int shipmentEntries = orderEntries * 10;
+vm4.invoke(() -> WANTestBase.validateRegionSize(orderRegionName, 
orderEntries));
+vm5.invoke(() -> WANTestBase.validateRegionSize(orderRegionName, 
orderEntries));
+vm4.invoke(() -> WANTestBase.validateRegionSize(shipmentRegionName, 

[jira] [Commented] (GEODE-8971) Batches with incomplete transactions when stopping the gateway sender

2021-03-09 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/GEODE-8971?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17298570#comment-17298570
 ] 

ASF GitHub Bot commented on GEODE-8971:
---

albertogpz commented on a change in pull request #6052:
URL: https://github.com/apache/geode/pull/6052#discussion_r591130076



##
File path: 
geode-wan/src/distributedTest/java/org/apache/geode/internal/cache/wan/parallel/ParallelWANStatsDUnitTest.java
##
@@ -472,6 +473,200 @@ public void 
testPRParallelPropagationWithGroupTransactionEventsSendsBatchesWithC
 
   }
 
+  @Test
+  public void 
testPRParallelPropagationWithGroupTransactionEventsDoesNotSendBatchesWithIncompleteTransactionsIfGatewaySenderIsStoppedWhileReceivingTrafficAndLaterStarted()
+  throws InterruptedException {
+Integer lnPort = vm0.invoke(() -> 
WANTestBase.createFirstLocatorWithDSId(1));
+Integer nyPort = vm1.invoke(() -> WANTestBase.createFirstRemoteLocator(2, 
lnPort));
+
+createCacheInVMs(nyPort, vm2);
+createReceiverCustomerOrderShipmentPR(vm2);
+createReceiverInVMs(vm2);
+
+createCacheInVMs(lnPort, vm4, vm5);
+createSenderCustomerOrderShipmentPRs(vm4);
+createSenderCustomerOrderShipmentPRs(vm5);
+
+int batchSize = 10;
+vm4.invoke(
+() -> WANTestBase.createSender("ln", 2, true, 100, batchSize, false, 
true, null, false,
+true));
+vm5.invoke(
+() -> WANTestBase.createSender("ln", 2, true, 100, batchSize, false, 
true, null, false,
+true));
+
+int customers = 4;
+int transactionsPerCustomer = 100;
+// Each transaction will contain one order plus the following shipments
+int shipmentsPerTransaction = batchSize;
+AsyncInvocation inv1 = asyncExecuteCustomerTransactions(vm4, 
customers,
+transactionsPerCustomer, shipmentsPerTransaction);
+
+// wait for some batches to be distributed and then stop the sender
+vm4.invoke(() -> await()
+.until(() -> WANTestBase.getSenderStats("ln", -1).get(4) > 0));
+
+stopSenderInVMsAsync("ln", vm4, vm5);
+
+// Wait for customer transactions to finish
+inv1.await();
+int orderEntries = transactionsPerCustomer * customers;
+int shipmentEntries = orderEntries * 10;

Review comment:
   Totally agree





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)


[jira] [Resolved] (GEODE-8761) Add ServerConnection threads to ThreadMonitoring Service

2021-03-09 Thread Owen Nichols (Jira)


 [ 
https://issues.apache.org/jira/browse/GEODE-8761?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Owen Nichols resolved GEODE-8761.
-
Resolution: Fixed

> Add ServerConnection threads to ThreadMonitoring Service
> 
>
> Key: GEODE-8761
> URL: https://issues.apache.org/jira/browse/GEODE-8761
> Project: Geode
>  Issue Type: Improvement
>  Components: regions
>Affects Versions: 1.9.2, 1.10.0, 1.11.0, 1.12.0, 1.12.1, 1.13.0, 1.13.1, 
> 1.14.0
>Reporter: Anilkumar Gingade
>Assignee: Darrel Schneider
>Priority: Major
>  Labels: GeodeOperationAPI, blocks-1.14.0​, pull-request-available
> Fix For: 1.14.0, 1.15.0
>
>
> In Geode the Thread Monitoring Service (TMS) allows to monitor a thread to 
> see if a thread is stuck doing particular operation; it provides thread dump 
> of stuck thread after configured time. This ticket is to add ServerConnection 
> threads to be monitored by TMS.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (GEODE-6143) PowerMock should not be used in Geode tests

2021-03-09 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/GEODE-6143?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17298478#comment-17298478
 ] 

ASF GitHub Bot commented on GEODE-6143:
---

moleske opened a new pull request #6107:
URL: https://github.com/apache/geode/pull/6107


   Inspired by @kirklund's email about not using PowerMockito, I saw there were 
some easy ones to change in 30 minutes, so here's a PR
   
   Thank you for submitting a contribution to Apache Geode.
   
   In order to streamline the review of the contribution we ask you
   to ensure the following steps have been taken:
   
   ### For all changes:
   - [x] Is there a JIRA ticket associated with this PR? Is it referenced in 
the commit message?
   
   - [x] Has your PR been rebased against the latest commit within the target 
branch (typically `develop`)?
   
   - [ ] Is your initial contribution a single, squashed commit?
   It is two commits to show the actual change and some cleanup.  The person 
who merges may squash if they like
   - [x] Does `gradlew build` run cleanly?
   
   - [x] Have you written or updated unit tests to verify your changes?
   
   - [x] If adding new dependencies to the code, are these dependencies 
licensed in a way that is compatible for inclusion under [ASF 
2.0](http://www.apache.org/legal/resolved.html#category-a)?
   
   ### Note:
   Please ensure that once the PR is submitted, check Concourse for build 
issues and
   submit an update to your PR as soon as possible. If you need help, please 
send an
   email to d...@geode.apache.org.
   



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


> PowerMock should not be used in Geode tests
> ---
>
> Key: GEODE-6143
> URL: https://issues.apache.org/jira/browse/GEODE-6143
> Project: Geode
>  Issue Type: Bug
>  Components: build, tests
>Reporter: Kirk Lund
>Assignee: Ryan McMahon
>Priority: Major
>  Labels: pull-request-available
>  Time Spent: 13h 10m
>  Remaining Estimate: 0h
>
> At least one usage of PowerMock in our unit tests is leaking and causing 
> other tests to fail with the following stack.
> No further tests should be written using PowerMock and all tests that 
> currently use it need to be identified and changed (along with product code 
> refactoring) to not use PowerMock.
> We will then remove PowerMock as a dependency from Geode.
> {noformat}
> 2018-12-04 18:11:36,258 Distributed system shutdown hook ERROR Could not 
> reconfigure JMX java.lang.LinkageError: loader constraint violation: loader 
> (instance of org/powermock/core/classloader/MockClassLoader) previously 
> initiated loading for a different type with name 
> "javax/management/MBeanServer"
>   at java.lang.ClassLoader.defineClass1(Native Method)
>   at java.lang.ClassLoader.defineClass(ClassLoader.java:763)
>   at 
> org.powermock.core.classloader.MockClassLoader.loadUnmockedClass(MockClassLoader.java:262)
>   at 
> org.powermock.core.classloader.MockClassLoader.loadModifiedClass(MockClassLoader.java:206)
>   at 
> org.powermock.core.classloader.DeferSupportingClassLoader.loadClass1(DeferSupportingClassLoader.java:89)
>   at 
> org.powermock.core.classloader.DeferSupportingClassLoader.loadClass(DeferSupportingClassLoader.java:79)
>   at java.lang.ClassLoader.loadClass(ClassLoader.java:357)
>   at 
> org.apache.logging.log4j.core.jmx.Server.unregisterAllMatching(Server.java:337)
>   at 
> org.apache.logging.log4j.core.jmx.Server.unregisterLoggerContext(Server.java:261)
>   at 
> org.apache.logging.log4j.core.jmx.Server.reregisterMBeansAfterReconfigure(Server.java:165)
>   at 
> org.apache.logging.log4j.core.jmx.Server.reregisterMBeansAfterReconfigure(Server.java:141)
>   at 
> org.apache.logging.log4j.core.LoggerContext.setConfiguration(LoggerContext.java:558)
>   at 
> org.apache.logging.log4j.core.LoggerContext.reconfigure(LoggerContext.java:619)
>   at 
> org.apache.logging.log4j.core.LoggerContext.reconfigure(LoggerContext.java:636)
>   at 
> org.apache.logging.log4j.core.LoggerContext.start(LoggerContext.java:231)
>   at 
> org.apache.logging.log4j.core.impl.Log4jContextFactory.getContext(Log4jContextFactory.java:243)
>   at 
> org.apache.logging.log4j.core.impl.Log4jContextFactory.getContext(Log4jContextFactory.java:45)
>   at org.apache.logging.log4j.LogManager.getContext(LogManager.java:174)
>   at org.apache.logging.log4j.LogManager.getLogger(LogManager.java:661)
>   at 
> 

[jira] [Commented] (GEODE-9016) NullPointerException during PutAll with CQ LOCAL_DESTROY event

2021-03-09 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/GEODE-9016?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17298471#comment-17298471
 ] 

ASF GitHub Bot commented on GEODE-9016:
---

DonalEvans commented on a change in pull request #6104:
URL: https://github.com/apache/geode/pull/6104#discussion_r590881622



##
File path: 
geode-core/src/main/java/org/apache/geode/internal/cache/DistributedCacheOperation.java
##
@@ -716,7 +716,7 @@ protected void _distribute() {
* This is similar to 
CacheClientNotifier.removeDestroyTokensFromCqResultKeys() where the
* destroyed events for local CQs are handled.
*/
-  private void removeDestroyTokensFromCqResultKeys(FilterRoutingInfo 
filterRouting) {
+  protected void removeDestroyTokensFromCqResultKeys(FilterRoutingInfo 
filterRouting) {

Review comment:
   This method (and the overridden version in DistributedPutAllOperation) 
can be package-private instead of protected. It's best to keep visibility as 
restricted as possible.

##
File path: 
geode-cq/src/distributedTest/java/org/apache/geode/cache/query/cq/dunit/PartitionedRegionCqQueryDUnitTest.java
##
@@ -120,6 +123,60 @@
 
   private static int bridgeServerPort;
 
+  @Test
+  public void testPutAllWithCQLocalDestroy() throws Exception {

Review comment:
   This method never throws an `Exception` so this is not necessary.

##
File path: 
geode-core/src/main/java/org/apache/geode/internal/cache/DistributedPutAllOperation.java
##
@@ -814,6 +818,46 @@ protected FilterRoutingInfo getRecipientFilterRouting(Set 
cacheOpRecipients) {
 return consolidated;
   }
 
+  @Override
+  protected void removeDestroyTokensFromCqResultKeys(FilterRoutingInfo 
filterRouting) {
+for (InternalDistributedMember m : filterRouting.getMembers()) {
+  FilterInfo filterInfo = filterRouting.getFilterInfo(m);
+  if (filterInfo.getCQs() == null) {
+continue;
+  }
+
+  CacheDistributionAdvisor.CacheProfile cf =
+  (CacheDistributionAdvisor.CacheProfile) ((Bucket) 
getRegion()).getPartitionedRegion()
+  .getCacheDistributionAdvisor().getProfile(m);
+
+  if (cf == null || cf.filterProfile == null || 
cf.filterProfile.isLocalProfile()
+  || cf.filterProfile.getCqMap().isEmpty()) {
+continue;
+  }
+
+  for (Object value : cf.filterProfile.getCqMap().values()) {
+ServerCQ cq = (ServerCQ) value;
+
+for (Map.Entry e : filterInfo.getCQs().entrySet()) {
+  Long cqID = e.getKey();
+  // For the CQs satisfying the event with destroy CQEvent, remove
+  // the entry from CQ cache.
+  for (int i = 0; i < this.putAllData.length; i++) {

Review comment:
   Until this line, the code in this method and in 
`DistributedCacheOperation.removeDestroyTokensFromCqResultKeys
   ()` is identical. Would it be possible to extract the duplicated code to a 
method in `DistributedCacheOperation` that both classes could use, rather than 
duplicating 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


> NullPointerException during PutAll with CQ LOCAL_DESTROY event
> --
>
> Key: GEODE-9016
> URL: https://issues.apache.org/jira/browse/GEODE-9016
> Project: Geode
>  Issue Type: Bug
>Reporter: Jianxia Chen
>Assignee: Jianxia Chen
>Priority: Major
>  Labels: pull-request-available
>
> It is possible that PutAll operation hits a NPE when CQ LOCAL_DESTROY event 
> is generated.
> {code:java}
> java.lang.NullPointerException
> at 
> java.util.concurrent.ConcurrentHashMap.get(ConcurrentHashMap.java:936)
> at 
> org.apache.geode.cache.query.cq.internal.ServerCQResultsCachePartitionRegionImpl.remove(ServerCQResultsCachePartitionRegionImpl.java:69)
> at 
> org.apache.geode.cache.query.cq.internal.ServerCQImpl.removeFromCqResultKeys(ServerCQImpl.java:297)
> at 
> org.apache.geode.internal.cache.DistributedCacheOperation.removeDestroyTokensFromCqResultKeys(DistributedCacheOperation.java:743)
> at 
> org.apache.geode.internal.cache.DistributedCacheOperation._distribute(DistributedCacheOperation.java:693)
> at 
> org.apache.geode.internal.cache.DistributedCacheOperation.startOperation(DistributedCacheOperation.java:277)
> at 
> org.apache.geode.internal.cache.DistributedRegion.postPutAllSend(DistributedRegion.java:3304)
> at 
> org.apache.geode.internal.cache.LocalRegionDataView.postPutAll(LocalRegionDataView.java:358)
> at 
> 

[jira] [Commented] (GEODE-8971) Batches with incomplete transactions when stopping the gateway sender

2021-03-09 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/GEODE-8971?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17298459#comment-17298459
 ] 

ASF GitHub Bot commented on GEODE-8971:
---

DonalEvans commented on a change in pull request #6052:
URL: https://github.com/apache/geode/pull/6052#discussion_r590864147



##
File path: 
geode-wan/src/distributedTest/java/org/apache/geode/internal/cache/wan/serial/SerialWANStatsDUnitTest.java
##
@@ -348,7 +349,201 @@ public void 
testReplicatedSerialPropagationWithBatchRedistWithGroupTransactionEv
   }
 
   @Test
-  public void testReplicatedSerialPropagationWithMultipleDispatchers() throws 
Exception {
+  public void 
testReplicatedSerialPropagationWithGroupTransactionEventsDoesNotSendBatchesWithIncompleteTransactionsIfGatewaySenderIsStoppedWhileReceivingTrafficAndLaterStarted()
+  throws InterruptedException {
+Integer lnPort = vm0.invoke(() -> 
WANTestBase.createFirstLocatorWithDSId(1));
+Integer nyPort = vm1.invoke(() -> WANTestBase.createFirstRemoteLocator(2, 
lnPort));
+
+String regionName = testName + "_RR";
+
+createCacheInVMs(nyPort, vm2);
+createReceiverInVMs(vm2);
+vm2.invoke(() -> WANTestBase.createReplicatedRegion(regionName, null, 
isOffHeap()));
+
+createCacheInVMs(lnPort, vm4, vm5);
+vm4.invoke(() -> WANTestBase.createReplicatedRegion(regionName, "ln", 
isOffHeap()));
+vm5.invoke(() -> WANTestBase.createReplicatedRegion(regionName, "ln", 
isOffHeap()));
+
+boolean groupTransactionEvents = true;
+int batchSize = 10;
+vm4.invoke(
+() -> WANTestBase.createSender("ln", 2, false, 100, batchSize, false, 
true, null, false,
+groupTransactionEvents));
+vm5.invoke(
+() -> WANTestBase.createSender("ln", 2, false, 100, batchSize, false, 
true, null, false,
+groupTransactionEvents));
+
+int eventsPerTransaction = batchSize + 1;
+// The number of entries must be big enough so that not all entries
+// are replicated before the sender is stopped and also divisible by 
eventsPerTransaction
+int entries = 2200;
+// Execute some transactions
+AsyncInvocation inv1 =
+asyncExecuteTransactions(regionName, eventsPerTransaction, entries);
+
+// wait for batches to be distributed and then stop the sender
+vm4.invoke(() -> await()
+.until(() -> WANTestBase.getSenderStats("ln", -1).get(4) > 0));
+
+// These exceptions are ignored here because it could happen that when an 
event
+// is to be handled, the sender is stopped. The sender, when stopped, 
shuts down
+// the thread pool that would handle the event and this could provoke the 
exception.
+addIgnoredException("Exception occurred in CacheListener");
+addIgnoredException(RejectedExecutionException.class);
+
+// Stop the sender
+stopSenderInVMsAsync("ln", vm4, vm5);
+
+// Wait for transactions to finish
+inv1.await();
+vm4.invoke(() -> WANTestBase.validateRegionSize(regionName, entries));
+vm5.invoke(() -> WANTestBase.validateRegionSize(regionName, entries));
+
+// Check
+checkOnlyCompleteTransactionsAreReplicatedAfterSenderStopped(regionName,
+eventsPerTransaction);
+
+// Start the sender
+startSenderInVMsAsync("ln", vm4, vm5);
+
+// Check
+checkOnlyCompleteTransactionsAreReplicatedAfterSenderRestarted(regionName,
+eventsPerTransaction);
+  }
+
+  @Test
+  public void 
testReplicatedSerialPropagationWithGroupTransactionEventsDoesNotSendBatchesWithIncompleteTransactionsIfGatewaySenderIsStoppedWhileReceivingTrafficAndLaterStartedReceiverStopped()
+  throws InterruptedException {
+Integer lnPort = vm0.invoke(() -> 
WANTestBase.createFirstLocatorWithDSId(1));
+Integer nyPort = vm1.invoke(() -> WANTestBase.createFirstRemoteLocator(2, 
lnPort));
+
+String regionName = testName + "_RR";
+
+createCacheInVMs(nyPort, vm2);
+createReceiverInVMs(vm2);
+vm2.invoke(() -> WANTestBase.createReplicatedRegion(regionName, null, 
isOffHeap()));
+vm2.invoke(WANTestBase::stopReceivers);
+
+createCacheInVMs(lnPort, vm4, vm5);
+vm4.invoke(() -> WANTestBase.createReplicatedRegion(regionName, "ln", 
isOffHeap()));
+vm5.invoke(() -> WANTestBase.createReplicatedRegion(regionName, "ln", 
isOffHeap()));
+
+boolean groupTransactionEvents = true;
+int batchSize = 10;
+vm4.invoke(
+() -> WANTestBase.createSender("ln", 2, false, 100, batchSize, false, 
true, null, false,
+groupTransactionEvents));
+vm5.invoke(
+() -> WANTestBase.createSender("ln", 2, false, 100, batchSize, false, 
true, null, false,
+groupTransactionEvents));
+
+int eventsPerTransaction = batchSize + 1;
+// The number of entries must be big enough so that not all entries
+// are replicated before the sender is stopped and also divisible by 
eventsPerTransaction
+int entries = 2200;

[jira] [Commented] (GEODE-8971) Batches with incomplete transactions when stopping the gateway sender

2021-03-09 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/GEODE-8971?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17298448#comment-17298448
 ] 

ASF GitHub Bot commented on GEODE-8971:
---

DonalEvans commented on a change in pull request #6052:
URL: https://github.com/apache/geode/pull/6052#discussion_r590846518



##
File path: 
geode-wan/src/distributedTest/java/org/apache/geode/internal/cache/wan/parallel/ParallelWANStatsDUnitTest.java
##
@@ -472,6 +473,200 @@ public void 
testPRParallelPropagationWithGroupTransactionEventsSendsBatchesWithC
 
   }
 
+  @Test
+  public void 
testPRParallelPropagationWithGroupTransactionEventsDoesNotSendBatchesWithIncompleteTransactionsIfGatewaySenderIsStoppedWhileReceivingTrafficAndLaterStarted()
+  throws InterruptedException {
+Integer lnPort = vm0.invoke(() -> 
WANTestBase.createFirstLocatorWithDSId(1));
+Integer nyPort = vm1.invoke(() -> WANTestBase.createFirstRemoteLocator(2, 
lnPort));
+
+createCacheInVMs(nyPort, vm2);
+createReceiverCustomerOrderShipmentPR(vm2);
+createReceiverInVMs(vm2);
+
+createCacheInVMs(lnPort, vm4, vm5);
+createSenderCustomerOrderShipmentPRs(vm4);
+createSenderCustomerOrderShipmentPRs(vm5);
+
+int batchSize = 10;
+vm4.invoke(
+() -> WANTestBase.createSender("ln", 2, true, 100, batchSize, false, 
true, null, false,
+true));
+vm5.invoke(
+() -> WANTestBase.createSender("ln", 2, true, 100, batchSize, false, 
true, null, false,
+true));
+
+int customers = 4;
+int transactionsPerCustomer = 100;
+// Each transaction will contain one order plus the following shipments
+int shipmentsPerTransaction = batchSize;
+AsyncInvocation inv1 = asyncExecuteCustomerTransactions(vm4, 
customers,
+transactionsPerCustomer, shipmentsPerTransaction);
+
+// wait for some batches to be distributed and then stop the sender
+vm4.invoke(() -> await()
+.until(() -> WANTestBase.getSenderStats("ln", -1).get(4) > 0));
+
+stopSenderInVMsAsync("ln", vm4, vm5);
+
+// Wait for customer transactions to finish
+inv1.await();
+int orderEntries = transactionsPerCustomer * customers;
+int shipmentEntries = orderEntries * 10;

Review comment:
   I think this could be `int shipmentEntries = orderEntries * 
shipmentsPerTransaction;` for additional clarity.

##
File path: 
geode-wan/src/distributedTest/java/org/apache/geode/internal/cache/wan/parallel/ParallelWANStatsDUnitTest.java
##
@@ -472,6 +473,200 @@ public void 
testPRParallelPropagationWithGroupTransactionEventsSendsBatchesWithC
 
   }
 
+  @Test
+  public void 
testPRParallelPropagationWithGroupTransactionEventsDoesNotSendBatchesWithIncompleteTransactionsIfGatewaySenderIsStoppedWhileReceivingTrafficAndLaterStarted()
+  throws InterruptedException {
+Integer lnPort = vm0.invoke(() -> 
WANTestBase.createFirstLocatorWithDSId(1));
+Integer nyPort = vm1.invoke(() -> WANTestBase.createFirstRemoteLocator(2, 
lnPort));
+
+createCacheInVMs(nyPort, vm2);
+createReceiverCustomerOrderShipmentPR(vm2);
+createReceiverInVMs(vm2);
+
+createCacheInVMs(lnPort, vm4, vm5);
+createSenderCustomerOrderShipmentPRs(vm4);
+createSenderCustomerOrderShipmentPRs(vm5);
+
+int batchSize = 10;
+vm4.invoke(
+() -> WANTestBase.createSender("ln", 2, true, 100, batchSize, false, 
true, null, false,
+true));
+vm5.invoke(
+() -> WANTestBase.createSender("ln", 2, true, 100, batchSize, false, 
true, null, false,
+true));
+
+int customers = 4;
+int transactionsPerCustomer = 100;
+// Each transaction will contain one order plus the following shipments
+int shipmentsPerTransaction = batchSize;
+AsyncInvocation inv1 = asyncExecuteCustomerTransactions(vm4, 
customers,
+transactionsPerCustomer, shipmentsPerTransaction);
+
+// wait for some batches to be distributed and then stop the sender
+vm4.invoke(() -> await()
+.until(() -> WANTestBase.getSenderStats("ln", -1).get(4) > 0));
+
+stopSenderInVMsAsync("ln", vm4, vm5);
+
+// Wait for customer transactions to finish
+inv1.await();
+int orderEntries = transactionsPerCustomer * customers;
+int shipmentEntries = orderEntries * 10;
+vm4.invoke(() -> WANTestBase.validateRegionSize(orderRegionName, 
orderEntries));
+vm5.invoke(() -> WANTestBase.validateRegionSize(orderRegionName, 
orderEntries));
+vm4.invoke(() -> WANTestBase.validateRegionSize(shipmentRegionName, 
shipmentEntries));
+vm5.invoke(() -> WANTestBase.validateRegionSize(shipmentRegionName, 
shipmentEntries));
+
+checkOnlyCompleteTransactionsAreReplicatedAfterSenderStopped(
+shipmentsPerTransaction);
+
+// Start sender to validate that queued events do not contain incomplete 
transactions after
+// restart
+startSenderInVMsAsync("ln", vm4, vm5);

[jira] [Commented] (GEODE-8679) Replace Native logger with 3rd party solution

2021-03-09 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/GEODE-8679?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17298447#comment-17298447
 ] 

ASF GitHub Bot commented on GEODE-8679:
---

pivotal-jbarrett commented on a change in pull request #761:
URL: https://github.com/apache/geode-native/pull/761#discussion_r590850026



##
File path: CMakeLists.txt
##
@@ -285,8 +285,9 @@ elseif(CMAKE_CXX_COMPILER_ID MATCHES "MSVC")
   set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} /ignore:4099")
   set(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} /ignore:4099")
 
-  # Enables multiprocess compiles
-  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /MP")
+  # /MP Enables multiprocess compiles
+  # SPDLOG_NO_TLS disables use of tls code which clashes with managed build on 
Windows
+  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /MP /DSPDLOG_NO_TLS")

Review comment:
   This sort for definition specific to a dependency should be defined in 
that dependency.

##
File path: cppcache/integration-test/CMakeLists.txt
##
@@ -30,6 +30,7 @@ target_link_libraries(test-cppcache-utils
   PUBLIC
 apache-geode
 framework
+SPDLog::SPDLog

Review comment:
   Why are the integration test dependent on the `SPDLog` library? It's use 
should be hidden at the integration level. The library should be statically 
compiled into the `apache-geode` library.

##
File path: cppcache/src/Log.cpp
##
@@ -107,334 +78,280 @@ void Log::init(LogLevel level, const char* logFileName, 
int32_t logFileLimit,
   init(level, logFileNameString, logFileLimit, logDiskSpaceLimit);
 }
 
-void Log::rollLogFile() {
-  if (g_log) {
-fclose(g_log);
-g_log = nullptr;
-  }
-
-  auto rollFileName =
-  (g_fullpath.parent_path() /
-   (g_fullpath.stem().string() + "-" + std::to_string(g_rollIndex) +
-g_fullpath.extension().string()))
-  .string();
-  try {
-auto rollFile = boost::filesystem::path(rollFileName);
-boost::filesystem::rename(g_fullpath, rollFile);
-g_rollFiles[g_rollIndex] = rollFile;
-g_rollIndex++;
-  } catch (const boost::filesystem::filesystem_error&) {
-throw IllegalStateException("Failed to roll log file");
-  }
-}
-
-void Log::removeOldestRolledLogFile() {
-  if (g_rollFiles.size()) {
-auto index = g_rollFiles.begin()->first;
-auto fileToRemove = g_rollFiles.begin()->second;
-auto fileSize = boost::filesystem::file_size(fileToRemove);
-boost::filesystem::remove(fileToRemove);
-g_rollFiles.erase(index);
-g_spaceUsed -= fileSize;
-  } else {
-throw IllegalStateException(
-"Failed to free sufficient disk space for logs");
-  }
-}
-
-void Log::calculateUsedDiskSpace() {
-  g_spaceUsed = 0;
-  if (boost::filesystem::exists(g_fullpath)) {
-g_spaceUsed = boost::filesystem::file_size(g_fullpath);
-for (auto const& item : g_rollFiles) {
-  g_spaceUsed += boost::filesystem::file_size(item.second);
-}
-  }
-}
+std::string Log::logLineFormat() {
+  std::stringstream format;
+  const size_t MINBUFSIZE = 128;
+  auto now = std::chrono::system_clock::now();
+  auto secs = std::chrono::system_clock::to_time_t(now);
+  auto tm_val = apache::geode::util::chrono::localtime(secs);
 
-void Log::buildRollFileMapping() {
-  const auto filterstring = g_fullpath.stem().string() + "-(\\d+)\\.log$";
-  const std::regex my_filter(filterstring);
-
-  g_rollFiles.clear();
-
-  boost::filesystem::directory_iterator end_itr;
-  for (boost::filesystem::directory_iterator i(
-   g_fullpath.parent_path().string());
-   i != end_itr; ++i) {
-if (boost::filesystem::is_regular_file(i->status())) {
-  std::string filename = i->path().filename().string();
-  std::regex testPattern(filterstring);
-  std::match_results testMatches;
-  if (std::regex_search(std::string::const_iterator(filename.begin()),
-filename.cend(), testMatches, testPattern)) {
-auto index = std::atoi(
-std::string(testMatches[1].first, testMatches[1].second).c_str());
-g_rollFiles[index] = i->path();
-  }
-}
-  }
-}
+  format << "[%l %Y/%m/%d %H:%M:%S.%f " << std::put_time(_val, "%Z  ")
+ << boost::asio::ip::host_name() << ":%P %t] %v";
 
-void Log::setRollFileIndex() {
-  g_rollIndex = 0;
-  if (g_rollFiles.size()) {
-g_rollIndex = g_rollFiles.rbegin()->first + 1;
-  }
+  return format.str();
 }
 
-void Log::setSizeLimits(int32_t logFileLimit, int64_t logDiskSpaceLimit) {
+void Log::setSizeLimits(int32_t logFileLimit, int64_t logDiskSpaceLimit,
+int32_t& adjustedFileLimit,
+int64_t& adjustedDiskLimit) {
   validateSizeLimits(logFileLimit, logDiskSpaceLimit);
 
   // Default to 10MB file limit and 1GB disk limit
   if (logFileLimit == 0 && logDiskSpaceLimit == 0) {
-g_fileSizeLimit = 10 * __1M__;
-g_diskSpaceLimit = 1000 * __1M__;
+

[jira] [Commented] (GEODE-8925) Migrate old integration test testThinClientPRSingleHop to new test framework

2021-03-09 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/GEODE-8925?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17298442#comment-17298442
 ] 

ASF GitHub Bot commented on GEODE-8925:
---

pivotal-jbarrett commented on a change in pull request #760:
URL: https://github.com/apache/geode-native/pull/760#discussion_r590848984



##
File path: cppcache/src/TcrMessage.cpp
##
@@ -1530,7 +1530,7 @@ void TcrMessage::handleByteArrayResponse(
   }
   m_metadata->push_back(bucketServerLocations);
 }
-LOGFINER("Metadata size is %", m_metadata->size());
+LOGFINER("Metadata size is %d", m_metadata->size());

Review comment:
   I believe the `%zu` is what you want for `size_t` values.

##
File path: cppcache/integration/test/PartitionRegionOpsTest.cpp
##
@@ -168,97 +186,67 @@ void 
putPartitionedRegionWithRedundancyServerGoesDown(bool singleHop) {
 
   int ENTRIES = 30;
 
-  putEntries(region, ENTRIES, 0);
-
-  cluster.getServers()[1].stop();
-
-  putEntries(region, ENTRIES, 1);
-
-  cluster.getServers()[1].start();
-
-  putEntries(region, ENTRIES, 2);
-}
+  putEntries(region, ENTRIES, 0, isAllOp);
 
-void getPartitionedRegionWithRedundancyServerGoesDown(bool singleHop) {
-  Cluster cluster{LocatorCount{1}, ServerCount{2}};
-  cluster.start();
-  cluster.getGfsh()
-  .create()
-  .region()
-  .withName("region")
-  .withType("PARTITION")
-  .withRedundantCopies("1")
-  .execute();
-
-  auto cache = createCache();
-  auto pool = createPool(cluster, cache, singleHop);
-  auto region = setupRegion(cache, pool);
-
-  int ENTRIES = 30;
-
-  putEntries(region, ENTRIES, 0);
-
-  getEntries(region, ENTRIES);
+  getEntries(region, ENTRIES, isAllOp);
 
   cluster.getServers()[1].stop();
 
-  getEntries(region, ENTRIES);
+  getEntries(region, ENTRIES, isAllOp);
 
   cluster.getServers()[1].start();
 
-  getEntries(region, ENTRIES);
+  getEntries(region, ENTRIES, isAllOp);
 }
 
 /**
  * In this test case we verify that in a partition region with redundancy
- * when one server goes down, all gets are still served.
+ * when one server goes down, all puts and gets are still served.
  * Single-hop is enabled in the client.
  * It can be observed in the logs that when one of the server goes down
  * the bucketServerLocations for that server are removed from the
  * client metadata.
  */
 TEST(PartitionRegionOpsTest,
- getPartitionedRegionWithRedundancyServerGoesDownSingleHop) {
+ putgetPartitionedRegionWithRedundancyServerGoesDownSingleHop) {

Review comment:
   An appropriately encompassing test class name with appropriately 
descriptive test names should cover it. The problem with the old style was it 
preferred keyboard conservation over readability and maintainability. I would 
rather see duplicate code with one minor change than a method with several 
boolean flags.





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


> Migrate old integration test testThinClientPRSingleHop to new test framework
> 
>
> Key: GEODE-8925
> URL: https://issues.apache.org/jira/browse/GEODE-8925
> Project: Geode
>  Issue Type: Sub-task
>  Components: native client
>Reporter: Michael Martell
>Priority: Major
>  Labels: pull-request-available
>
> The following singleHop tests are marked Flaky. This ticket is to port these 
> to the new framework.
>  * testThinClientPRSingleHop
>  * testThinClientPutAllPRSingleHop



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (GEODE-8925) Migrate old integration test testThinClientPRSingleHop to new test framework

2021-03-09 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/GEODE-8925?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17298436#comment-17298436
 ] 

ASF GitHub Bot commented on GEODE-8925:
---

mmartell commented on a change in pull request #760:
URL: https://github.com/apache/geode-native/pull/760#discussion_r590837766



##
File path: cppcache/integration/test/PartitionRegionOpsTest.cpp
##
@@ -89,19 +89,36 @@ std::shared_ptr setupRegion(Cache& cache,
 }
 
 void putEntries(std::shared_ptr region, int numEntries,
-int offsetForValue) {
+int offsetForValue, bool isAllOp) {

Review comment:
   Yah, I think this aligns with echobravopapa's suggestions above. Please 
have a look at my refactor suggestion above and let me know if that works.





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


> Migrate old integration test testThinClientPRSingleHop to new test framework
> 
>
> Key: GEODE-8925
> URL: https://issues.apache.org/jira/browse/GEODE-8925
> Project: Geode
>  Issue Type: Sub-task
>  Components: native client
>Reporter: Michael Martell
>Priority: Major
>  Labels: pull-request-available
>
> The following singleHop tests are marked Flaky. This ticket is to port these 
> to the new framework.
>  * testThinClientPRSingleHop
>  * testThinClientPutAllPRSingleHop



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (GEODE-8925) Migrate old integration test testThinClientPRSingleHop to new test framework

2021-03-09 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/GEODE-8925?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17298435#comment-17298435
 ] 

ASF GitHub Bot commented on GEODE-8925:
---

mmartell commented on a change in pull request #760:
URL: https://github.com/apache/geode-native/pull/760#discussion_r590836522



##
File path: cppcache/integration/test/PartitionRegionOpsTest.cpp
##
@@ -168,97 +186,67 @@ void 
putPartitionedRegionWithRedundancyServerGoesDown(bool singleHop) {
 
   int ENTRIES = 30;
 
-  putEntries(region, ENTRIES, 0);
-
-  cluster.getServers()[1].stop();
-
-  putEntries(region, ENTRIES, 1);
-
-  cluster.getServers()[1].start();
-
-  putEntries(region, ENTRIES, 2);
-}
+  putEntries(region, ENTRIES, 0, isAllOp);
 
-void getPartitionedRegionWithRedundancyServerGoesDown(bool singleHop) {
-  Cluster cluster{LocatorCount{1}, ServerCount{2}};
-  cluster.start();
-  cluster.getGfsh()
-  .create()
-  .region()
-  .withName("region")
-  .withType("PARTITION")
-  .withRedundantCopies("1")
-  .execute();
-
-  auto cache = createCache();
-  auto pool = createPool(cluster, cache, singleHop);
-  auto region = setupRegion(cache, pool);
-
-  int ENTRIES = 30;
-
-  putEntries(region, ENTRIES, 0);
-
-  getEntries(region, ENTRIES);
+  getEntries(region, ENTRIES, isAllOp);
 
   cluster.getServers()[1].stop();
 
-  getEntries(region, ENTRIES);
+  getEntries(region, ENTRIES, isAllOp);
 
   cluster.getServers()[1].start();
 
-  getEntries(region, ENTRIES);
+  getEntries(region, ENTRIES, isAllOp);
 }
 
 /**
  * In this test case we verify that in a partition region with redundancy
- * when one server goes down, all gets are still served.
+ * when one server goes down, all puts and gets are still served.
  * Single-hop is enabled in the client.
  * It can be observed in the logs that when one of the server goes down
  * the bucketServerLocations for that server are removed from the
  * client metadata.
  */
 TEST(PartitionRegionOpsTest,
- getPartitionedRegionWithRedundancyServerGoesDownSingleHop) {
+ putgetPartitionedRegionWithRedundancyServerGoesDownSingleHop) {

Review comment:
   Good call. Since I didn't write this test (or review it), I didn't want 
to trash it too badly. I think we can do away with many of the comments, which 
are mostly just duplicated for the four tests, in favor of using better test 
names. How do you feel about just breaking the single function with parameters 
(putgetPartitionedRegionWithRedundancyServerGoesDown(singleHop, useAll)) into 
the four separate aptly named functions with no parameters:
   
   putgetHA
   putgetHAWithSingleHop
   putAllgetAllHA
   putAllGetAllHASingleHop
   
   I think the HA implies failover so we could nuke the comments.





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


> Migrate old integration test testThinClientPRSingleHop to new test framework
> 
>
> Key: GEODE-8925
> URL: https://issues.apache.org/jira/browse/GEODE-8925
> Project: Geode
>  Issue Type: Sub-task
>  Components: native client
>Reporter: Michael Martell
>Priority: Major
>  Labels: pull-request-available
>
> The following singleHop tests are marked Flaky. This ticket is to port these 
> to the new framework.
>  * testThinClientPRSingleHop
>  * testThinClientPutAllPRSingleHop



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (GEODE-8925) Migrate old integration test testThinClientPRSingleHop to new test framework

2021-03-09 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/GEODE-8925?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17298433#comment-17298433
 ] 

ASF GitHub Bot commented on GEODE-8925:
---

pdxcodemonkey commented on a change in pull request #760:
URL: https://github.com/apache/geode-native/pull/760#discussion_r590835496



##
File path: cppcache/integration/test/PartitionRegionOpsTest.cpp
##
@@ -89,19 +89,36 @@ std::shared_ptr setupRegion(Cache& cache,
 }
 
 void putEntries(std::shared_ptr region, int numEntries,
-int offsetForValue) {
+int offsetForValue, bool isAllOp) {

Review comment:
   Boolean parameters generally are a code smell 
(https://medium.com/@amlcurran/clean-code-the-curse-of-a-boolean-parameter-c237a830b7a3),
 and I don't think these are doing you any favors.  Have a look at refactoring 
to remove isAllOp altogether, I think it'll be better.





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


> Migrate old integration test testThinClientPRSingleHop to new test framework
> 
>
> Key: GEODE-8925
> URL: https://issues.apache.org/jira/browse/GEODE-8925
> Project: Geode
>  Issue Type: Sub-task
>  Components: native client
>Reporter: Michael Martell
>Priority: Major
>  Labels: pull-request-available
>
> The following singleHop tests are marked Flaky. This ticket is to port these 
> to the new framework.
>  * testThinClientPRSingleHop
>  * testThinClientPutAllPRSingleHop



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (GEODE-8679) Replace Native logger with 3rd party solution

2021-03-09 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/GEODE-8679?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17298428#comment-17298428
 ] 

ASF GitHub Bot commented on GEODE-8679:
---

pdxcodemonkey opened a new pull request #761:
URL: https://github.com/apache/geode-native/pull/761


   



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


> Replace Native logger with 3rd party solution
> -
>
> Key: GEODE-8679
> URL: https://issues.apache.org/jira/browse/GEODE-8679
> Project: Geode
>  Issue Type: Improvement
>  Components: native client
>Reporter: Blake Bender
>Priority: Major
>
> As a native client developer, I would prefer to only maintain code that 
> actually implements core features of a Geode client.  A logging subsystem is 
> definitely not core Geode functionality.  Additionally, the existing 
> geode-native logger does not properly support the `loig-file-size-limit` and 
> `log-disk-space-limit` properties.  Rather than attempt to fix the existing 
> logging code to properly implement these, we should switch to a 3rd-party 
> solution (spdlog or similar) that already has these features available and 
> working.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Updated] (GEODE-8679) Replace Native logger with 3rd party solution

2021-03-09 Thread ASF GitHub Bot (Jira)


 [ 
https://issues.apache.org/jira/browse/GEODE-8679?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

ASF GitHub Bot updated GEODE-8679:
--
Labels: pull-request-available  (was: )

> Replace Native logger with 3rd party solution
> -
>
> Key: GEODE-8679
> URL: https://issues.apache.org/jira/browse/GEODE-8679
> Project: Geode
>  Issue Type: Improvement
>  Components: native client
>Reporter: Blake Bender
>Priority: Major
>  Labels: pull-request-available
>
> As a native client developer, I would prefer to only maintain code that 
> actually implements core features of a Geode client.  A logging subsystem is 
> definitely not core Geode functionality.  Additionally, the existing 
> geode-native logger does not properly support the `loig-file-size-limit` and 
> `log-disk-space-limit` properties.  Rather than attempt to fix the existing 
> logging code to properly implement these, we should switch to a 3rd-party 
> solution (spdlog or similar) that already has these features available and 
> working.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (GEODE-8925) Migrate old integration test testThinClientPRSingleHop to new test framework

2021-03-09 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/GEODE-8925?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17298422#comment-17298422
 ] 

ASF GitHub Bot commented on GEODE-8925:
---

mmartell commented on a change in pull request #760:
URL: https://github.com/apache/geode-native/pull/760#discussion_r590819249



##
File path: cppcache/integration/test/PartitionRegionOpsTest.cpp
##
@@ -89,19 +89,36 @@ std::shared_ptr setupRegion(Cache& cache,
 }
 
 void putEntries(std::shared_ptr region, int numEntries,
-int offsetForValue) {
+int offsetForValue, bool isAllOp) {

Review comment:
   I agree isAllOp is confusing. I don't think doAllOperations is any 
better. I think this is much better:
   
   - putEntries(..., bool usePutAll)
   - getEntries(..., bool useGetAll)





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


> Migrate old integration test testThinClientPRSingleHop to new test framework
> 
>
> Key: GEODE-8925
> URL: https://issues.apache.org/jira/browse/GEODE-8925
> Project: Geode
>  Issue Type: Sub-task
>  Components: native client
>Reporter: Michael Martell
>Priority: Major
>  Labels: pull-request-available
>
> The following singleHop tests are marked Flaky. This ticket is to port these 
> to the new framework.
>  * testThinClientPRSingleHop
>  * testThinClientPutAllPRSingleHop



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (GEODE-8761) Add ServerConnection threads to ThreadMonitoring Service

2021-03-09 Thread ASF subversion and git services (Jira)


[ 
https://issues.apache.org/jira/browse/GEODE-8761?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17298420#comment-17298420
 ] 

ASF subversion and git services commented on GEODE-8761:


Commit 2bb7e34ab1ef3a5c0965b907ca46c7ea38dde378 in geode's branch 
refs/heads/support/1.14 from Darrel Schneider
[ https://gitbox.apache.org/repos/asf?p=geode.git;h=2bb7e34 ]

GEODE-8761: detect stuck server connection threads (#6094) (#6105)

(cherry picked from commit df7b5b167472d87d8d262515533e7c80d7306e67)

> Add ServerConnection threads to ThreadMonitoring Service
> 
>
> Key: GEODE-8761
> URL: https://issues.apache.org/jira/browse/GEODE-8761
> Project: Geode
>  Issue Type: Improvement
>  Components: regions
>Affects Versions: 1.9.2, 1.10.0, 1.11.0, 1.12.0, 1.12.1, 1.13.0, 1.13.1, 
> 1.14.0
>Reporter: Anilkumar Gingade
>Assignee: Darrel Schneider
>Priority: Major
>  Labels: GeodeOperationAPI, blocks-1.14.0​, pull-request-available
> Fix For: 1.14.0, 1.15.0
>
>
> In Geode the Thread Monitoring Service (TMS) allows to monitor a thread to 
> see if a thread is stuck doing particular operation; it provides thread dump 
> of stuck thread after configured time. This ticket is to add ServerConnection 
> threads to be monitored by TMS.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Updated] (GEODE-8761) Add ServerConnection threads to ThreadMonitoring Service

2021-03-09 Thread Darrel Schneider (Jira)


 [ 
https://issues.apache.org/jira/browse/GEODE-8761?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Darrel Schneider updated GEODE-8761:

Fix Version/s: 1.14.0

> Add ServerConnection threads to ThreadMonitoring Service
> 
>
> Key: GEODE-8761
> URL: https://issues.apache.org/jira/browse/GEODE-8761
> Project: Geode
>  Issue Type: Improvement
>  Components: regions
>Affects Versions: 1.9.2, 1.10.0, 1.11.0, 1.12.0, 1.12.1, 1.13.0, 1.13.1, 
> 1.14.0
>Reporter: Anilkumar Gingade
>Assignee: Darrel Schneider
>Priority: Major
>  Labels: GeodeOperationAPI, blocks-1.14.0​, pull-request-available
> Fix For: 1.14.0, 1.15.0
>
>
> In Geode the Thread Monitoring Service (TMS) allows to monitor a thread to 
> see if a thread is stuck doing particular operation; it provides thread dump 
> of stuck thread after configured time. This ticket is to add ServerConnection 
> threads to be monitored by TMS.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (GEODE-8761) Add ServerConnection threads to ThreadMonitoring Service

2021-03-09 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/GEODE-8761?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17298419#comment-17298419
 ] 

ASF GitHub Bot commented on GEODE-8761:
---

dschneider-pivotal merged pull request #6105:
URL: https://github.com/apache/geode/pull/6105


   



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


> Add ServerConnection threads to ThreadMonitoring Service
> 
>
> Key: GEODE-8761
> URL: https://issues.apache.org/jira/browse/GEODE-8761
> Project: Geode
>  Issue Type: Improvement
>  Components: regions
>Affects Versions: 1.9.2, 1.10.0, 1.11.0, 1.12.0, 1.12.1, 1.13.0, 1.13.1, 
> 1.14.0
>Reporter: Anilkumar Gingade
>Assignee: Darrel Schneider
>Priority: Major
>  Labels: GeodeOperationAPI, blocks-1.14.0​, pull-request-available
> Fix For: 1.15.0
>
>
> In Geode the Thread Monitoring Service (TMS) allows to monitor a thread to 
> see if a thread is stuck doing particular operation; it provides thread dump 
> of stuck thread after configured time. This ticket is to add ServerConnection 
> threads to be monitored by TMS.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (GEODE-8925) Migrate old integration test testThinClientPRSingleHop to new test framework

2021-03-09 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/GEODE-8925?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17298417#comment-17298417
 ] 

ASF GitHub Bot commented on GEODE-8925:
---

mmartell commented on a change in pull request #760:
URL: https://github.com/apache/geode-native/pull/760#discussion_r590812844



##
File path: cppcache/src/TcrMessage.cpp
##
@@ -1530,7 +1530,7 @@ void TcrMessage::handleByteArrayResponse(
   }
   m_metadata->push_back(bucketServerLocations);
 }
-LOGFINER("Metadata size is %", m_metadata->size());

Review comment:
   Yah, it just prints as a blank. I needed to fix it to help with my test 
analysis.





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


> Migrate old integration test testThinClientPRSingleHop to new test framework
> 
>
> Key: GEODE-8925
> URL: https://issues.apache.org/jira/browse/GEODE-8925
> Project: Geode
>  Issue Type: Sub-task
>  Components: native client
>Reporter: Michael Martell
>Priority: Major
>  Labels: pull-request-available
>
> The following singleHop tests are marked Flaky. This ticket is to port these 
> to the new framework.
>  * testThinClientPRSingleHop
>  * testThinClientPutAllPRSingleHop



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (GEODE-9004) Issues with queries targeting a map field

2021-03-09 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/GEODE-9004?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17298407#comment-17298407
 ] 

ASF GitHub Bot commented on GEODE-9004:
---

DonalEvans commented on a change in pull request #6096:
URL: https://github.com/apache/geode/pull/6096#discussion_r590790779



##
File path: 
geode-core/src/integrationTest/java/org/apache/geode/cache/query/internal/index/MapRangeIndexMaintenanceJUnitTest.java
##
@@ -341,6 +341,155 @@ public void 
testNullMapValuesInIndexOnLocalRegionForCompactMap() throws Exceptio
 assertEquals(1, result.size());
   }
 
+  @Test
+  public void testQueriesForValueInMapFieldWithoutIndex() throws Exception {
+region =
+
CacheUtils.getCache().createRegionFactory(RegionShortcut.REPLICATE).create("portfolio");
+qs = CacheUtils.getQueryService();
+testQueriesForValueInMapField(region, qs);
+  }
+
+  @Test
+  public void testQueriesForValueInMapFieldWithMapIndexWithOneKey() throws 
Exception {
+region =
+
CacheUtils.getCache().createRegionFactory(RegionShortcut.REPLICATE).create("portfolio");
+qs = CacheUtils.getQueryService();
+
+keyIndex1 = qs.createIndex(INDEX_NAME, "positions['SUN']", SEPARATOR + 
"portfolio ");
+assertTrue(keyIndex1 instanceof CompactMapRangeIndex);
+testQueriesForValueInMapField(region, qs);
+
+long keys = ((CompactMapRangeIndex) 
keyIndex1).internalIndexStats.getNumberOfKeys();
+long mapIndexKeys =
+((CompactMapRangeIndex) 
keyIndex1).internalIndexStats.getNumberOfMapIndexKeys();
+long values =
+((CompactMapRangeIndex) 
keyIndex1).internalIndexStats.getNumberOfValues();
+assertEquals(3, keys);
+assertEquals(1, mapIndexKeys);
+assertEquals(3, values);
+  }
+
+  @Test
+  public void testQueriesForValueInMapFieldWithMapIndexWithSeveralKeys() 
throws Exception {
+region =
+
CacheUtils.getCache().createRegionFactory(RegionShortcut.REPLICATE).create("portfolio");
+qs = CacheUtils.getQueryService();
+
+keyIndex1 =
+qs.createIndex(INDEX_NAME, "positions['SUN', 'ERICSSON']", SEPARATOR + 
"portfolio ");
+assertTrue(keyIndex1 instanceof CompactMapRangeIndex);
+testQueriesForValueInMapField(region, qs);
+
+long keys = ((CompactMapRangeIndex) 
keyIndex1).internalIndexStats.getNumberOfKeys();
+long mapIndexKeys =
+((CompactMapRangeIndex) 
keyIndex1).internalIndexStats.getNumberOfMapIndexKeys();
+long values =
+((CompactMapRangeIndex) 
keyIndex1).internalIndexStats.getNumberOfValues();
+assertEquals(3, keys);
+assertEquals(1, mapIndexKeys);
+assertEquals(3, values);
+  }
+
+  @Test
+  public void testQueriesForValueInMapFieldWithMapIndexWithStar() throws 
Exception {
+region =
+
CacheUtils.getCache().createRegionFactory(RegionShortcut.REPLICATE).create("portfolio");
+qs = CacheUtils.getQueryService();
+
+keyIndex1 = qs.createIndex(INDEX_NAME, "positions[*]", SEPARATOR + 
"portfolio ");
+assertTrue(keyIndex1 instanceof CompactMapRangeIndex);
+testQueriesForValueInMapField(region, qs);
+
+long keys = ((CompactMapRangeIndex) 
keyIndex1).internalIndexStats.getNumberOfKeys();
+long mapIndexKeys =
+((CompactMapRangeIndex) 
keyIndex1).internalIndexStats.getNumberOfMapIndexKeys();
+long values =
+((CompactMapRangeIndex) 
keyIndex1).internalIndexStats.getNumberOfValues();
+assertEquals(5, keys);
+assertEquals(4, mapIndexKeys);
+assertEquals(5, values);
+  }
+
+  public void testQueriesForValueInMapField(Region region, QueryService qs) 
throws Exception {
+// Empty map
+Portfolio p = new Portfolio(1, 1);
+p.positions = new HashMap();

Review comment:
   It looks like this use of the raw class instead of the parameterized one 
got missed in the last set of changes.





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


> Issues with queries targeting a map field
> -
>
> Key: GEODE-9004
> URL: https://issues.apache.org/jira/browse/GEODE-9004
> Project: Geode
>  Issue Type: Bug
>  Components: querying
>Reporter: Alberto Gomez
>Assignee: Alberto Gomez
>Priority: Major
>  Labels: pull-request-available
>
> When defining indexes on a map field several issues have been found:
>  * If the index is defined for one specific key in the map, e.g. 
> positions['SUN'], then an index entry is created for every entry, no matter 
> if the entry contains the 'SUN' key in its map or not. This makes the index 
> take a lot of memory 

[jira] [Commented] (GEODE-9004) Issues with queries targeting a map field

2021-03-09 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/GEODE-9004?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17298404#comment-17298404
 ] 

ASF GitHub Bot commented on GEODE-9004:
---

DonalEvans commented on pull request #6096:
URL: https://github.com/apache/geode/pull/6096#issuecomment-794554193


   The StressNewTest failures you're seeing in pre-checkin might be fixed if 
you rebase your branch on develop. They look very similar to the failures that 
were being caused by GEODE-8905, which has since been reverted.



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


> Issues with queries targeting a map field
> -
>
> Key: GEODE-9004
> URL: https://issues.apache.org/jira/browse/GEODE-9004
> Project: Geode
>  Issue Type: Bug
>  Components: querying
>Reporter: Alberto Gomez
>Assignee: Alberto Gomez
>Priority: Major
>  Labels: pull-request-available
>
> When defining indexes on a map field several issues have been found:
>  * If the index is defined for one specific key in the map, e.g. 
> positions['SUN'], then an index entry is created for every entry, no matter 
> if the entry contains the 'SUN' key in its map or not. This makes the index 
> take a lot of memory (unnecessarily?).
>  * If the index is specified for more than one key in the map or for all 
> ('*'), then queries in which the "where" contains a != condition for the map, 
> e.g. "p.positions['SUN'] != '3'" return less values than those returned when 
> the query is run without the index.
>  * If the index is specified for more than one key in the map or for all 
> ('*'), then queries in which the "where" contains a "= null" condition for 
> the map, e.g. "p.positions['SUN'] = null" return less values than those 
> returned when the query is run without the index.
>  * If the index is defined for one specific key in the map, e.g. 
> positions['SUN'], queries in which the "where" contains a "!=" condition for 
> the map or a " = null" condition sometimes return less values than those 
> returned when the query is run without the index.
> Apart from the above, looking at the indexes documentation, it seems that Map 
> indexes are only those indexes for which more than one key or '*' is 
> specified for the Map. But if just one key is specified for the Map in the 
> index, then the index is not a Map index but a range index. This should be 
> clarified.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (GEODE-8925) Migrate old integration test testThinClientPRSingleHop to new test framework

2021-03-09 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/GEODE-8925?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17298402#comment-17298402
 ] 

ASF GitHub Bot commented on GEODE-8925:
---

echobravopapa commented on a change in pull request #760:
URL: https://github.com/apache/geode-native/pull/760#discussion_r590762097



##
File path: cppcache/src/TcrMessage.cpp
##
@@ -1530,7 +1530,7 @@ void TcrMessage::handleByteArrayResponse(
   }
   m_metadata->push_back(bucketServerLocations);
 }
-LOGFINER("Metadata size is %", m_metadata->size());

Review comment:
   guess we've been getting lucky here...

##
File path: cppcache/integration/test/PartitionRegionOpsTest.cpp
##
@@ -168,97 +186,67 @@ void 
putPartitionedRegionWithRedundancyServerGoesDown(bool singleHop) {
 
   int ENTRIES = 30;
 
-  putEntries(region, ENTRIES, 0);
-
-  cluster.getServers()[1].stop();
-
-  putEntries(region, ENTRIES, 1);
-
-  cluster.getServers()[1].start();
-
-  putEntries(region, ENTRIES, 2);
-}
+  putEntries(region, ENTRIES, 0, isAllOp);
 
-void getPartitionedRegionWithRedundancyServerGoesDown(bool singleHop) {
-  Cluster cluster{LocatorCount{1}, ServerCount{2}};
-  cluster.start();
-  cluster.getGfsh()
-  .create()
-  .region()
-  .withName("region")
-  .withType("PARTITION")
-  .withRedundantCopies("1")
-  .execute();
-
-  auto cache = createCache();
-  auto pool = createPool(cluster, cache, singleHop);
-  auto region = setupRegion(cache, pool);
-
-  int ENTRIES = 30;
-
-  putEntries(region, ENTRIES, 0);
-
-  getEntries(region, ENTRIES);
+  getEntries(region, ENTRIES, isAllOp);
 
   cluster.getServers()[1].stop();
 
-  getEntries(region, ENTRIES);
+  getEntries(region, ENTRIES, isAllOp);
 
   cluster.getServers()[1].start();
 
-  getEntries(region, ENTRIES);
+  getEntries(region, ENTRIES, isAllOp);
 }
 
 /**
  * In this test case we verify that in a partition region with redundancy
- * when one server goes down, all gets are still served.
+ * when one server goes down, all puts and gets are still served.
  * Single-hop is enabled in the client.
  * It can be observed in the logs that when one of the server goes down
  * the bucketServerLocations for that server are removed from the
  * client metadata.
  */
 TEST(PartitionRegionOpsTest,
- getPartitionedRegionWithRedundancyServerGoesDownSingleHop) {
+ putgetPartitionedRegionWithRedundancyServerGoesDownSingleHop) {

Review comment:
   these 4 TEST go thru the input matrix of 
`putgetPartitionedRegionWithRedundancyServerGoesDown(bool, bool)` 
   {true,false; false,false; true,true; false,true} and that's great
   I do notice this requires a good bit of comment maintenance, which makes me 
wonder if there is a better way to present these test cases for 
posterity/maintainability...
   

##
File path: cppcache/integration/test/PartitionRegionOpsTest.cpp
##
@@ -89,19 +89,36 @@ std::shared_ptr setupRegion(Cache& cache,
 }
 
 void putEntries(std::shared_ptr region, int numEntries,
-int offsetForValue) {
+int offsetForValue, bool isAllOp) {

Review comment:
   maybe a better mnemonic for the bool param... idk what `AllOp` means

##
File path: cppcache/integration/test/PartitionRegionOpsTest.cpp
##
@@ -89,19 +89,36 @@ std::shared_ptr setupRegion(Cache& cache,
 }
 
 void putEntries(std::shared_ptr region, int numEntries,
-int offsetForValue) {
+int offsetForValue, bool isAllOp) {

Review comment:
   maybe `doAllOperations`  if i'm following the intention here...





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


> Migrate old integration test testThinClientPRSingleHop to new test framework
> 
>
> Key: GEODE-8925
> URL: https://issues.apache.org/jira/browse/GEODE-8925
> Project: Geode
>  Issue Type: Sub-task
>  Components: native client
>Reporter: Michael Martell
>Priority: Major
>
> The following singleHop tests are marked Flaky. This ticket is to port these 
> to the new framework.
>  * testThinClientPRSingleHop
>  * testThinClientPutAllPRSingleHop



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Updated] (GEODE-8925) Migrate old integration test testThinClientPRSingleHop to new test framework

2021-03-09 Thread ASF GitHub Bot (Jira)


 [ 
https://issues.apache.org/jira/browse/GEODE-8925?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

ASF GitHub Bot updated GEODE-8925:
--
Labels: pull-request-available  (was: )

> Migrate old integration test testThinClientPRSingleHop to new test framework
> 
>
> Key: GEODE-8925
> URL: https://issues.apache.org/jira/browse/GEODE-8925
> Project: Geode
>  Issue Type: Sub-task
>  Components: native client
>Reporter: Michael Martell
>Priority: Major
>  Labels: pull-request-available
>
> The following singleHop tests are marked Flaky. This ticket is to port these 
> to the new framework.
>  * testThinClientPRSingleHop
>  * testThinClientPutAllPRSingleHop



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (GEODE-8558) Pulse query is bypassing the QueryResultSetLimit MBean parameter if there is a newline after query or sql comments before the query line

2021-03-09 Thread ASF subversion and git services (Jira)


[ 
https://issues.apache.org/jira/browse/GEODE-8558?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17298399#comment-17298399
 ] 

ASF subversion and git services commented on GEODE-8558:


Commit fab465b0cac9ce447603ef9a41446c7843497f7c in geode's branch 
refs/heads/support/1.13 from Jinmei Liao
[ https://gitbox.apache.org/repos/asf?p=geode.git;h=fab465b ]

GEODE-8558: query input by users should trim newlines and comments. (#5571)


(cherry picked from commit 2485e57707d8f4535bbf9a3e5f5926a26421ca20)


> Pulse query is bypassing the QueryResultSetLimit MBean parameter if there is 
> a newline after query or sql comments before the query line
> 
>
> Key: GEODE-8558
> URL: https://issues.apache.org/jira/browse/GEODE-8558
> Project: Geode
>  Issue Type: Bug
>  Components: querying
>Affects Versions: 1.13.0
>Reporter: Jinmei Liao
>Priority: Major
>  Labels: pull-request-available
> Fix For: 1.12.2, 1.13.3, 1.14.0
>
>
> Setting the JVM MBean attribute QueryResultSetLimit to limit the query result 
> set (e.g. 2 here with my small local cluster) and it seems working, however, 
> in the pulse query textbox, if you have a carriage-return/newline after query 
> line or let's say a commented text/query before the actual query, it's 
> returning all the entries and the setting is not effective anymore. And if 
> this could also affect the queries running with commented text coming from 
> function execution.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (GEODE-8558) Pulse query is bypassing the QueryResultSetLimit MBean parameter if there is a newline after query or sql comments before the query line

2021-03-09 Thread ASF subversion and git services (Jira)


[ 
https://issues.apache.org/jira/browse/GEODE-8558?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17298397#comment-17298397
 ] 

ASF subversion and git services commented on GEODE-8558:


Commit 1edd5d3822eafb5c71d9fb2115c9b48a7d289ff6 in geode's branch 
refs/heads/support/1.12 from Jinmei Liao
[ https://gitbox.apache.org/repos/asf?p=geode.git;h=1edd5d3 ]

GEODE-8558: query input by users should trim newlines and comments. (#5571)


(cherry picked from commit 2485e57707d8f4535bbf9a3e5f5926a26421ca20)


> Pulse query is bypassing the QueryResultSetLimit MBean parameter if there is 
> a newline after query or sql comments before the query line
> 
>
> Key: GEODE-8558
> URL: https://issues.apache.org/jira/browse/GEODE-8558
> Project: Geode
>  Issue Type: Bug
>  Components: querying
>Affects Versions: 1.13.0
>Reporter: Jinmei Liao
>Priority: Major
>  Labels: pull-request-available
> Fix For: 1.14.0
>
>
> Setting the JVM MBean attribute QueryResultSetLimit to limit the query result 
> set (e.g. 2 here with my small local cluster) and it seems working, however, 
> in the pulse query textbox, if you have a carriage-return/newline after query 
> line or let's say a commented text/query before the actual query, it's 
> returning all the entries and the setting is not effective anymore. And if 
> this could also affect the queries running with commented text coming from 
> function execution.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Updated] (GEODE-8558) Pulse query is bypassing the QueryResultSetLimit MBean parameter if there is a newline after query or sql comments before the query line

2021-03-09 Thread Jinmei Liao (Jira)


 [ 
https://issues.apache.org/jira/browse/GEODE-8558?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Jinmei Liao updated GEODE-8558:
---
Fix Version/s: 1.13.3
   1.12.2

> Pulse query is bypassing the QueryResultSetLimit MBean parameter if there is 
> a newline after query or sql comments before the query line
> 
>
> Key: GEODE-8558
> URL: https://issues.apache.org/jira/browse/GEODE-8558
> Project: Geode
>  Issue Type: Bug
>  Components: querying
>Affects Versions: 1.13.0
>Reporter: Jinmei Liao
>Priority: Major
>  Labels: pull-request-available
> Fix For: 1.12.2, 1.13.3, 1.14.0
>
>
> Setting the JVM MBean attribute QueryResultSetLimit to limit the query result 
> set (e.g. 2 here with my small local cluster) and it seems working, however, 
> in the pulse query textbox, if you have a carriage-return/newline after query 
> line or let's say a commented text/query before the actual query, it's 
> returning all the entries and the setting is not effective anymore. And if 
> this could also affect the queries running with commented text coming from 
> function execution.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Resolved] (GEODE-9012) Turn off Github Comments Causing Jira Ticket Comments

2021-03-09 Thread Bill Burcham (Jira)


 [ 
https://issues.apache.org/jira/browse/GEODE-9012?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Bill Burcham resolved GEODE-9012.
-
Fix Version/s: 1.15.0
   Resolution: Fixed

Development infrastructure change only—no Geode functionality changes.

> Turn off Github Comments Causing Jira Ticket Comments
> -
>
> Key: GEODE-9012
> URL: https://issues.apache.org/jira/browse/GEODE-9012
> Project: Geode
>  Issue Type: Task
>  Components: build
>Reporter: Bill Burcham
>Assignee: Bill Burcham
>Priority: Major
>  Labels: pull-request-available
> Fix For: 1.15.0
>
>
> GEODE-8006 the .asf.yaml file to configure tool integration settings. That 
> caused Github comments to be replicated as Jira ticket comments. Those Jira 
> ticket comments are useless. What's worse, they are so "noisy" that 
> subsequent manually-entered comments on the Jira ticket get essentially lost. 
> They aren't really lost, but human readers are inundated with too many 
> comments.
> When this ticket is complete, the {{comment}} directive will be removed from 
> the Jira notifications options described here: 
> [https://cwiki.apache.org/confluence/display/INFRA/Git+-+.asf.yaml+features#Git.asf.yamlfeatures-Jiranotificationoptions]



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (GEODE-9012) Turn off Github Comments Causing Jira Ticket Comments

2021-03-09 Thread ASF subversion and git services (Jira)


[ 
https://issues.apache.org/jira/browse/GEODE-9012?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17298394#comment-17298394
 ] 

ASF subversion and git services commented on GEODE-9012:


Commit cc5fb5bdd7391f54c28b6976b4a95e9d8196bf44 in geode's branch 
refs/heads/develop from Bill Burcham
[ https://gitbox.apache.org/repos/asf?p=geode.git;h=cc5fb5b ]

GEODE-9012: turn off Github PR to Jira comment mirroring (#6103)

* turn off Github PR to Jira comment mirroring
* this is a development infrastructure change only

> Turn off Github Comments Causing Jira Ticket Comments
> -
>
> Key: GEODE-9012
> URL: https://issues.apache.org/jira/browse/GEODE-9012
> Project: Geode
>  Issue Type: Task
>  Components: build
>Reporter: Bill Burcham
>Assignee: Bill Burcham
>Priority: Major
>  Labels: pull-request-available
>
> GEODE-8006 the .asf.yaml file to configure tool integration settings. That 
> caused Github comments to be replicated as Jira ticket comments. Those Jira 
> ticket comments are useless. What's worse, they are so "noisy" that 
> subsequent manually-entered comments on the Jira ticket get essentially lost. 
> They aren't really lost, but human readers are inundated with too many 
> comments.
> When this ticket is complete, the {{comment}} directive will be removed from 
> the Jira notifications options described here: 
> [https://cwiki.apache.org/confluence/display/INFRA/Git+-+.asf.yaml+features#Git.asf.yamlfeatures-Jiranotificationoptions]



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Assigned] (GEODE-9012) Turn off Github Comments Causing Jira Ticket Comments

2021-03-09 Thread Bill Burcham (Jira)


 [ 
https://issues.apache.org/jira/browse/GEODE-9012?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Bill Burcham reassigned GEODE-9012:
---

Assignee: Bill Burcham

> Turn off Github Comments Causing Jira Ticket Comments
> -
>
> Key: GEODE-9012
> URL: https://issues.apache.org/jira/browse/GEODE-9012
> Project: Geode
>  Issue Type: Task
>  Components: build
>Reporter: Bill Burcham
>Assignee: Bill Burcham
>Priority: Major
>  Labels: pull-request-available
>
> GEODE-8006 the .asf.yaml file to configure tool integration settings. That 
> caused Github comments to be replicated as Jira ticket comments. Those Jira 
> ticket comments are useless. What's worse, they are so "noisy" that 
> subsequent manually-entered comments on the Jira ticket get essentially lost. 
> They aren't really lost, but human readers are inundated with too many 
> comments.
> When this ticket is complete, the {{comment}} directive will be removed from 
> the Jira notifications options described here: 
> [https://cwiki.apache.org/confluence/display/INFRA/Git+-+.asf.yaml+features#Git.asf.yamlfeatures-Jiranotificationoptions]



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (GEODE-9012) Turn off Github Comments Causing Jira Ticket Comments

2021-03-09 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/GEODE-9012?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17298392#comment-17298392
 ] 

ASF GitHub Bot commented on GEODE-9012:
---

Bill merged pull request #6103:
URL: https://github.com/apache/geode/pull/6103


   



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


> Turn off Github Comments Causing Jira Ticket Comments
> -
>
> Key: GEODE-9012
> URL: https://issues.apache.org/jira/browse/GEODE-9012
> Project: Geode
>  Issue Type: Task
>  Components: build
>Reporter: Bill Burcham
>Priority: Major
>  Labels: pull-request-available
>
> GEODE-8006 the .asf.yaml file to configure tool integration settings. That 
> caused Github comments to be replicated as Jira ticket comments. Those Jira 
> ticket comments are useless. What's worse, they are so "noisy" that 
> subsequent manually-entered comments on the Jira ticket get essentially lost. 
> They aren't really lost, but human readers are inundated with too many 
> comments.
> When this ticket is complete, the {{comment}} directive will be removed from 
> the Jira notifications options described here: 
> [https://cwiki.apache.org/confluence/display/INFRA/Git+-+.asf.yaml+features#Git.asf.yamlfeatures-Jiranotificationoptions]



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (GEODE-9012) Turn off Github Comments Causing Jira Ticket Comments

2021-03-09 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/GEODE-9012?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17298391#comment-17298391
 ] 

ASF GitHub Bot commented on GEODE-9012:
---

Bill commented on pull request #6103:
URL: https://github.com/apache/geode/pull/6103#issuecomment-794503145


   @onichols-pivotal thanks for pointing out `worklog`. Upon consideration I do 
not want `worklog` for the same reason I don't want `comment`. `worklog` would 
be an improvement over `comment` but both are redundant, so I'm going to merge 
this approved PR as-is.



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


> Turn off Github Comments Causing Jira Ticket Comments
> -
>
> Key: GEODE-9012
> URL: https://issues.apache.org/jira/browse/GEODE-9012
> Project: Geode
>  Issue Type: Task
>  Components: build
>Reporter: Bill Burcham
>Priority: Major
>  Labels: pull-request-available
>
> GEODE-8006 the .asf.yaml file to configure tool integration settings. That 
> caused Github comments to be replicated as Jira ticket comments. Those Jira 
> ticket comments are useless. What's worse, they are so "noisy" that 
> subsequent manually-entered comments on the Jira ticket get essentially lost. 
> They aren't really lost, but human readers are inundated with too many 
> comments.
> When this ticket is complete, the {{comment}} directive will be removed from 
> the Jira notifications options described here: 
> [https://cwiki.apache.org/confluence/display/INFRA/Git+-+.asf.yaml+features#Git.asf.yamlfeatures-Jiranotificationoptions]



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (GEODE-9017) Reload key store and trust store upon change

2021-03-09 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/GEODE-9017?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17298361#comment-17298361
 ] 

ASF GitHub Bot commented on GEODE-9017:
---

aaronlindsey commented on pull request #6106:
URL: https://github.com/apache/geode/pull/6106#issuecomment-794476780


   Hmm `./gradlew build` works but the concourse build job is failing. I will 
take a look...



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


> Reload key store and trust store upon change
> 
>
> Key: GEODE-9017
> URL: https://issues.apache.org/jira/browse/GEODE-9017
> Project: Geode
>  Issue Type: New Feature
>Reporter: Aaron Lindsey
>Assignee: Aaron Lindsey
>Priority: Major
>  Labels: pull-request-available
>
> [Link to 
> RFC|https://cwiki.apache.org/confluence/display/GEODE/Make+key+and+trust+stores+reload+automatically+upon+change]
> (The below text is copied from the RFC document.)
> h3. Problem
> Currently, in order to rotate certificates each member of the cluster needs 
> to be restarted to load new certs and trust. It would be preferable if 
> certificates can be rotated without having to restart members.
> h3. Solution
> When starting up a cluster member we currently read the TLS configuration 
> which, when TLS is enabled has key and trust store files defined. In case 
> those files are defined they are read, and the information inside them is 
> loaded into the key and trust manager objects that are loaded into the 
> SSLContext.
> This solution will introduce wrapper objects for the key and trust managers 
> and file/directory watcher(s) that can detect changes to the key and trust 
> store files. When key and trust store files are changed this will trigger a 
> reload into key and trust managers and through the wrapper objects these new 
> key and trust managers will be injected into the SSLContext so that the 
> context can start using the new key and trust managers in process.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Closed] (GEODE-3118) Replace ACE_SSL with direct OpenSSL use.

2021-03-09 Thread Blake Bender (Jira)


 [ 
https://issues.apache.org/jira/browse/GEODE-3118?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Blake Bender closed GEODE-3118.
---

> Replace ACE_SSL with direct OpenSSL use.
> 
>
> Key: GEODE-3118
> URL: https://issues.apache.org/jira/browse/GEODE-3118
> Project: Geode
>  Issue Type: Improvement
>  Components: native client
>Reporter: Jacob Barrett
>Assignee: Jacob Barrett
>Priority: Major
>
> ACE_SSL does not support OpenSSL 1.1.0. We currently use ACE_SSL to wrap 
> OpenSSL but it provides no true utility.
> https://github.com/DOCGroup/ACE_TAO/issues/434



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Resolved] (GEODE-3118) Replace ACE_SSL with direct OpenSSL use.

2021-03-09 Thread Blake Bender (Jira)


 [ 
https://issues.apache.org/jira/browse/GEODE-3118?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Blake Bender resolved GEODE-3118.
-
Resolution: Fixed

> Replace ACE_SSL with direct OpenSSL use.
> 
>
> Key: GEODE-3118
> URL: https://issues.apache.org/jira/browse/GEODE-3118
> Project: Geode
>  Issue Type: Improvement
>  Components: native client
>Reporter: Jacob Barrett
>Assignee: Jacob Barrett
>Priority: Major
>
> ACE_SSL does not support OpenSSL 1.1.0. We currently use ACE_SSL to wrap 
> OpenSSL but it provides no true utility.
> https://github.com/DOCGroup/ACE_TAO/issues/434



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Updated] (GEODE-9017) Reload key store and trust store upon change

2021-03-09 Thread ASF GitHub Bot (Jira)


 [ 
https://issues.apache.org/jira/browse/GEODE-9017?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

ASF GitHub Bot updated GEODE-9017:
--
Labels: pull-request-available  (was: )

> Reload key store and trust store upon change
> 
>
> Key: GEODE-9017
> URL: https://issues.apache.org/jira/browse/GEODE-9017
> Project: Geode
>  Issue Type: New Feature
>Reporter: Aaron Lindsey
>Assignee: Aaron Lindsey
>Priority: Major
>  Labels: pull-request-available
>
> [Link to 
> RFC|https://cwiki.apache.org/confluence/display/GEODE/Make+key+and+trust+stores+reload+automatically+upon+change]
> (The below text is copied from the RFC document.)
> h3. Problem
> Currently, in order to rotate certificates each member of the cluster needs 
> to be restarted to load new certs and trust. It would be preferable if 
> certificates can be rotated without having to restart members.
> h3. Solution
> When starting up a cluster member we currently read the TLS configuration 
> which, when TLS is enabled has key and trust store files defined. In case 
> those files are defined they are read, and the information inside them is 
> loaded into the key and trust manager objects that are loaded into the 
> SSLContext.
> This solution will introduce wrapper objects for the key and trust managers 
> and file/directory watcher(s) that can detect changes to the key and trust 
> store files. When key and trust store files are changed this will trigger a 
> reload into key and trust managers and through the wrapper objects these new 
> key and trust managers will be injected into the SSLContext so that the 
> context can start using the new key and trust managers in process.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (GEODE-9017) Reload key store and trust store upon change

2021-03-09 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/GEODE-9017?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17298349#comment-17298349
 ] 

ASF GitHub Bot commented on GEODE-9017:
---

aaronlindsey opened a new pull request #6106:
URL: https://github.com/apache/geode/pull/6106


   Reload the KeyManager and TrustManager in the SSLContext when the 
corresponding key store files are modified on disk.
   
   See [the 
RFC](https://cwiki.apache.org/confluence/display/GEODE/Make+key+and+trust+stores+reload+automatically+upon+change)
 for more background on this change.
   
   One thing I am not sure about is whether this should be the default 
behavior. I currently have the file watching key and trust managers enabled 
when the user does not set the "use default SSL context" flag (which means they 
are enabled under Geode's default settings). However, I could see how changing 
the default behavior might be surprising to some users. I could instead add a 
flag which enables the "automatic reload" behavior and make it disabled by 
default if the reviewers think that would be better.



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


> Reload key store and trust store upon change
> 
>
> Key: GEODE-9017
> URL: https://issues.apache.org/jira/browse/GEODE-9017
> Project: Geode
>  Issue Type: New Feature
>Reporter: Aaron Lindsey
>Assignee: Aaron Lindsey
>Priority: Major
>
> [Link to 
> RFC|https://cwiki.apache.org/confluence/display/GEODE/Make+key+and+trust+stores+reload+automatically+upon+change]
> (The below text is copied from the RFC document.)
> h3. Problem
> Currently, in order to rotate certificates each member of the cluster needs 
> to be restarted to load new certs and trust. It would be preferable if 
> certificates can be rotated without having to restart members.
> h3. Solution
> When starting up a cluster member we currently read the TLS configuration 
> which, when TLS is enabled has key and trust store files defined. In case 
> those files are defined they are read, and the information inside them is 
> loaded into the key and trust manager objects that are loaded into the 
> SSLContext.
> This solution will introduce wrapper objects for the key and trust managers 
> and file/directory watcher(s) that can detect changes to the key and trust 
> store files. When key and trust store files are changed this will trigger a 
> reload into key and trust managers and through the wrapper objects these new 
> key and trust managers will be injected into the SSLContext so that the 
> context can start using the new key and trust managers in process.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (GEODE-9004) Issues with queries targeting a map field

2021-03-09 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/GEODE-9004?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17298348#comment-17298348
 ] 

ASF GitHub Bot commented on GEODE-9004:
---

albertogpz commented on pull request #6096:
URL: https://github.com/apache/geode/pull/6096#issuecomment-794446665


   > Overall, switching from JUnit Assert to AssertJ will provide much better 
failure messages that will really help anyone dealing with test failures.
   > 
   > `\n` should always be replaced with `System.lineSeparator()`.
   
   Thanks for your comments, Kirk. I thought of changing completely the files I 
edited to AssertJ but then I realized it would be better just to change to 
AssertJ my changes and leave the cleaning to another PR so that the core of 
this PR is not hidden by the other changes.



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


> Issues with queries targeting a map field
> -
>
> Key: GEODE-9004
> URL: https://issues.apache.org/jira/browse/GEODE-9004
> Project: Geode
>  Issue Type: Bug
>  Components: querying
>Reporter: Alberto Gomez
>Assignee: Alberto Gomez
>Priority: Major
>  Labels: pull-request-available
>
> When defining indexes on a map field several issues have been found:
>  * If the index is defined for one specific key in the map, e.g. 
> positions['SUN'], then an index entry is created for every entry, no matter 
> if the entry contains the 'SUN' key in its map or not. This makes the index 
> take a lot of memory (unnecessarily?).
>  * If the index is specified for more than one key in the map or for all 
> ('*'), then queries in which the "where" contains a != condition for the map, 
> e.g. "p.positions['SUN'] != '3'" return less values than those returned when 
> the query is run without the index.
>  * If the index is specified for more than one key in the map or for all 
> ('*'), then queries in which the "where" contains a "= null" condition for 
> the map, e.g. "p.positions['SUN'] = null" return less values than those 
> returned when the query is run without the index.
>  * If the index is defined for one specific key in the map, e.g. 
> positions['SUN'], queries in which the "where" contains a "!=" condition for 
> the map or a " = null" condition sometimes return less values than those 
> returned when the query is run without the index.
> Apart from the above, looking at the indexes documentation, it seems that Map 
> indexes are only those indexes for which more than one key or '*' is 
> specified for the Map. But if just one key is specified for the Map in the 
> index, then the index is not a Map index but a range index. This should be 
> clarified.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Created] (GEODE-9017) Reload key store and trust store upon change

2021-03-09 Thread Aaron Lindsey (Jira)
Aaron Lindsey created GEODE-9017:


 Summary: Reload key store and trust store upon change
 Key: GEODE-9017
 URL: https://issues.apache.org/jira/browse/GEODE-9017
 Project: Geode
  Issue Type: New Feature
Reporter: Aaron Lindsey


[Link to 
RFC|https://cwiki.apache.org/confluence/display/GEODE/Make+key+and+trust+stores+reload+automatically+upon+change]

(The below text is copied from the RFC document.)
h3. Problem

Currently, in order to rotate certificates each member of the cluster needs to 
be restarted to load new certs and trust. It would be preferable if 
certificates can be rotated without having to restart members.
h3. Solution

When starting up a cluster member we currently read the TLS configuration 
which, when TLS is enabled has key and trust store files defined. In case those 
files are defined they are read, and the information inside them is loaded into 
the key and trust manager objects that are loaded into the SSLContext.

This solution will introduce wrapper objects for the key and trust managers and 
file/directory watcher(s) that can detect changes to the key and trust store 
files. When key and trust store files are changed this will trigger a reload 
into key and trust managers and through the wrapper objects these new key and 
trust managers will be injected into the SSLContext so that the context can 
start using the new key and trust managers in process.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Assigned] (GEODE-9017) Reload key store and trust store upon change

2021-03-09 Thread Aaron Lindsey (Jira)


 [ 
https://issues.apache.org/jira/browse/GEODE-9017?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Aaron Lindsey reassigned GEODE-9017:


Assignee: Aaron Lindsey

> Reload key store and trust store upon change
> 
>
> Key: GEODE-9017
> URL: https://issues.apache.org/jira/browse/GEODE-9017
> Project: Geode
>  Issue Type: New Feature
>Reporter: Aaron Lindsey
>Assignee: Aaron Lindsey
>Priority: Major
>
> [Link to 
> RFC|https://cwiki.apache.org/confluence/display/GEODE/Make+key+and+trust+stores+reload+automatically+upon+change]
> (The below text is copied from the RFC document.)
> h3. Problem
> Currently, in order to rotate certificates each member of the cluster needs 
> to be restarted to load new certs and trust. It would be preferable if 
> certificates can be rotated without having to restart members.
> h3. Solution
> When starting up a cluster member we currently read the TLS configuration 
> which, when TLS is enabled has key and trust store files defined. In case 
> those files are defined they are read, and the information inside them is 
> loaded into the key and trust manager objects that are loaded into the 
> SSLContext.
> This solution will introduce wrapper objects for the key and trust managers 
> and file/directory watcher(s) that can detect changes to the key and trust 
> store files. When key and trust store files are changed this will trigger a 
> reload into key and trust managers and through the wrapper objects these new 
> key and trust managers will be injected into the SSLContext so that the 
> context can start using the new key and trust managers in process.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (GEODE-8943) Filterrs (Interest and CQ) is processed multiple times with transactional operation on PR

2021-03-09 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/GEODE-8943?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17298313#comment-17298313
 ] 

ASF GitHub Bot commented on GEODE-8943:
---

pivotal-eshu commented on pull request #6075:
URL: https://github.com/apache/geode/pull/6075#issuecomment-794397010


   There is a behavior change in transaction with register interest. (From my 
understanding this behavior exists from start of the tx implementation).
   Now register interest will block transaction commit during the period of 
register interest -- esp. on the node performing the register interest from 
client. 
   The performance hit shifts from calculating filterRoutingInfo again on 
remote node to no transactions allowed to commit when register interest is 
under way.
   Also I worry about potential performance hit and even deadlock as the remote 
node processing register interest message can be blocked for a while (trying to 
get the write lock) while there are concurrent transactions.



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


> Filterrs (Interest and CQ) is processed multiple times with transactional 
> operation on PR
> -
>
> Key: GEODE-8943
> URL: https://issues.apache.org/jira/browse/GEODE-8943
> Project: Geode
>  Issue Type: Bug
>  Components: cq, regions
>Affects Versions: 1.14.0
>Reporter: Anilkumar Gingade
>Assignee: Eric Shu
>Priority: Major
>  Labels: blocks-1.14.0​, pull-request-available
>
> The Filters (interest and CQ) could be getting processed multiple times when 
> transactional operation is performed.
> By design on PR the filters are processed on primary bucket where the data is 
> applied/changed and adjunct message with interested clients are sent to 
> remote servers (where the subscription queues are hosted) and replicas; thus 
> performing filter processing once and sending the message to remote servers 
> only if the filters are satisfied; it looks like currently the filter 
> processing is happening both at the primary and secondary buckets for TX 
> operation.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (GEODE-8856) Persist gateway-sender state within Cluster Configuration

2021-03-09 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/GEODE-8856?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17298307#comment-17298307
 ] 

ASF GitHub Bot commented on GEODE-8856:
---

kirklund commented on a change in pull request #6036:
URL: https://github.com/apache/geode/pull/6036#discussion_r590679758



##
File path: 
geode-core/src/test/java/org/apache/geode/internal/cache/xmlcache/CacheXmlParserTest.java
##
@@ -0,0 +1,175 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more 
contributor license
+ * agreements. See the NOTICE file distributed with this work for additional 
information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache 
License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the 
License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software 
distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY 
KIND, either express
+ * or implied. See the License for the specific language governing permissions 
and limitations under
+ * the License.
+ */
+package org.apache.geode.internal.cache.xmlcache;
+
+import static 
org.apache.geode.internal.cache.GemFireCacheImpl.DEFAULT_LOCK_LEASE;
+import static 
org.apache.geode.internal.cache.GemFireCacheImpl.DEFAULT_LOCK_TIMEOUT;
+import static 
org.apache.geode.internal.cache.GemFireCacheImpl.DEFAULT_SEARCH_TIMEOUT;
+import static org.apache.geode.internal.cache.xmlcache.CacheXml.COPY_ON_READ;
+import static org.apache.geode.internal.cache.xmlcache.CacheXml.IS_SERVER;
+import static org.apache.geode.internal.cache.xmlcache.CacheXml.LOCK_LEASE;
+import static org.apache.geode.internal.cache.xmlcache.CacheXml.LOCK_TIMEOUT;
+import static 
org.apache.geode.internal.cache.xmlcache.CacheXml.MESSAGE_SYNC_INTERVAL;
+import static 
org.apache.geode.internal.cache.xmlcache.CacheXml.REMOTE_DISTRIBUTED_SYSTEM_ID;
+import static org.apache.geode.internal.cache.xmlcache.CacheXml.SEARCH_TIMEOUT;
+import static org.apache.geode.internal.cache.xmlcache.CacheXml.STATE;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.powermock.api.mockito.PowerMockito;
+import org.powermock.core.classloader.annotations.PrepareForTest;
+import org.powermock.modules.junit4.PowerMockRunner;
+import org.xml.sax.SAXException;
+import org.xml.sax.helpers.AttributesImpl;
+
+import org.apache.geode.cache.wan.GatewaySenderFactory;
+import org.apache.geode.cache.wan.GatewaySenderState;
+import org.apache.geode.internal.cache.ha.HARegionQueue;
+import org.apache.geode.internal.cache.wan.WANServiceProvider;
+
+@RunWith(PowerMockRunner.class)
+@PrepareForTest(WANServiceProvider.class)
+public class CacheXmlParserTest {
+
+  @Mock
+  private GatewaySenderFactory gatewaySenderFactory;
+
+  @Before
+  public void setUp() {
+PowerMockito.mockStatic(WANServiceProvider.class);

Review comment:
   The Geode community decided on the dev-list to get rid of and avoid 
`PowerMockito`. It causes too many problems including corrupting the unit 
testing JVM for later unit tests. It also discourages us from refactoring 
classes to be better structured and easier to unit test.
   
   The main Jira ticket is 
[https://issues.apache.org/jira/browse/GEODE-6143](https://issues.apache.org/jira/browse/GEODE-6143),
 but there are additional sub-tickets you can search Jira for.





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


> Persist gateway-sender state within Cluster Configuration
> -
>
> Key: GEODE-8856
> URL: https://issues.apache.org/jira/browse/GEODE-8856
> Project: Geode
>  Issue Type: Improvement
>Reporter: Jakov Varenina
>Assignee: Jakov Varenina
>Priority: Major
>  Labels: pull-request-available
>
> More details in following RFC: 
> https://cwiki.apache.org/confluence/display/GEODE/Persist+gateway-sender+state+within+Cluster+Configuration?moved=true
> All changes or findings during development will be reflected in above RFC. 



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (GEODE-9016) NullPointerException during PutAll with CQ LOCAL_DESTROY event

2021-03-09 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/GEODE-9016?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17298301#comment-17298301
 ] 

ASF GitHub Bot commented on GEODE-9016:
---

kirklund commented on a change in pull request #6104:
URL: https://github.com/apache/geode/pull/6104#discussion_r59074



##
File path: 
geode-cq/src/distributedTest/java/org/apache/geode/cache/query/cq/dunit/PartitionedRegionCqQueryDUnitTest.java
##
@@ -120,6 +123,60 @@
 
   private static int bridgeServerPort;
 
+  @Test
+  public void testPutAllWithCQLocalDestroy() throws Exception {
+VM server1 = getVM(0);
+VM server2 = getVM(1);
+VM client = getVM(2);
+
+final String cqName = "testPutAllWithCQLocalDestroy_0";
+createServer(server1);
+createServer(server2);
+final String host = Host.getHost(0).getHostName();
+final int port = server2.invoke(() -> 
PartitionedRegionCqQueryDUnitTest.getCacheServerPort());
+createClient(client, port, host);
+createCQ(client, cqName, cqs[0]);
+
+int numObjects = 1000;
+
+server1.invoke(() -> {
+  Region region = getCache().getRegion(SEPARATOR + "root" + SEPARATOR + 
regions[0]);
+  Map buffer = new HashMap();
+  for (int i = 1; i < numObjects; i++) {
+Portfolio p = new Portfolio(i);
+buffer.put("" + i, p);
+  }
+  region.putAll(buffer);
+});
+
+client.invoke(() -> {
+  QueryService cqService = getCache().getQueryService();
+  CqQuery cqQuery = cqService.getCq(cqName);
+  if (cqQuery == null) {
+fail("Failed to get CQ " + cqName);
+  }
+  try {
+cqQuery.executeWithInitialResults();
+  } catch (Exception ex) {
+fail("Failed to execute  CQ " + cqName, ex);
+  }
+});
+
+server1.invoke(() -> {
+  Region region = getCache().getRegion(SEPARATOR + "root" + SEPARATOR + 
regions[0]);
+  Map buffer = new HashMap();
+  for (int i = 1; i < numObjects; i++) {
+Portfolio p = new Portfolio(-1 * i);
+buffer.put("" + i, p);
+  }
+  region.putAll(buffer);
+});
+
+cqHelper.closeClient(client);
+cqHelper.closeServer(server2);
+cqHelper.closeServer(server1);

Review comment:
   It's better (and more industry standard for writing JUnit tests) if you 
can figure out a way to move cleanup code from the end of the test method to a 
`tearDown` method annotated with `@After`. For example, if you make 
`cqHelper.closeClient` return without error if client is null or not running, 
then you could hoist these local variables to fields and the test would look 
more like this:
   ```
   private VM client;
   private VM server1;
   private VM server2;
   
   @Before 
   public void setUp() {
server1 = getVM(0);
server2 = getVM(1);
client = getVM(2);
   }
   
   @After
   public void tearDown() {
 cqHelper.closeClient(client);
 cqHelper.closeServer(server2);
 cqHelper.closeServer(server1);
   }
   ```
   And `cqHelper` close methods could have some early-outs like this:
   ```
   public void closeClient(VM vm) {
 if (vm == null) {
   return;
 }
 // etc
   }
   ```
   This way if any of the test lines above the `closeClient` line throws an 
Exception, `tearDown` will ensure that cleanup still happens.





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


> NullPointerException during PutAll with CQ LOCAL_DESTROY event
> --
>
> Key: GEODE-9016
> URL: https://issues.apache.org/jira/browse/GEODE-9016
> Project: Geode
>  Issue Type: Bug
>Reporter: Jianxia Chen
>Assignee: Jianxia Chen
>Priority: Major
>  Labels: pull-request-available
>
> It is possible that PutAll operation hits a NPE when CQ LOCAL_DESTROY event 
> is generated.
> {code:java}
> java.lang.NullPointerException
> at 
> java.util.concurrent.ConcurrentHashMap.get(ConcurrentHashMap.java:936)
> at 
> org.apache.geode.cache.query.cq.internal.ServerCQResultsCachePartitionRegionImpl.remove(ServerCQResultsCachePartitionRegionImpl.java:69)
> at 
> org.apache.geode.cache.query.cq.internal.ServerCQImpl.removeFromCqResultKeys(ServerCQImpl.java:297)
> at 
> org.apache.geode.internal.cache.DistributedCacheOperation.removeDestroyTokensFromCqResultKeys(DistributedCacheOperation.java:743)
> at 
> org.apache.geode.internal.cache.DistributedCacheOperation._distribute(DistributedCacheOperation.java:693)
> at 
> 

[jira] [Commented] (GEODE-9016) NullPointerException during PutAll with CQ LOCAL_DESTROY event

2021-03-09 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/GEODE-9016?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17298299#comment-17298299
 ] 

ASF GitHub Bot commented on GEODE-9016:
---

kirklund commented on a change in pull request #6104:
URL: https://github.com/apache/geode/pull/6104#discussion_r590675288



##
File path: 
geode-core/src/main/java/org/apache/geode/internal/cache/DistributedPutAllOperation.java
##
@@ -814,6 +818,46 @@ protected FilterRoutingInfo getRecipientFilterRouting(Set 
cacheOpRecipients) {
 return consolidated;
   }
 
+  @Override
+  protected void removeDestroyTokensFromCqResultKeys(FilterRoutingInfo 
filterRouting) {
+for (InternalDistributedMember m : filterRouting.getMembers()) {
+  FilterInfo filterInfo = filterRouting.getFilterInfo(m);
+  if (filterInfo.getCQs() == null) {
+continue;
+  }
+
+  CacheDistributionAdvisor.CacheProfile cf =
+  (CacheDistributionAdvisor.CacheProfile) ((Bucket) 
getRegion()).getPartitionedRegion()
+  .getCacheDistributionAdvisor().getProfile(m);
+
+  if (cf == null || cf.filterProfile == null || 
cf.filterProfile.isLocalProfile()
+  || cf.filterProfile.getCqMap().isEmpty()) {
+continue;
+  }
+
+  for (Object value : cf.filterProfile.getCqMap().values()) {
+ServerCQ cq = (ServerCQ) value;
+
+for (Map.Entry e : filterInfo.getCQs().entrySet()) {
+  Long cqID = e.getKey();
+  // For the CQs satisfying the event with destroy CQEvent, remove
+  // the entry from CQ cache.
+  for (int i = 0; i < this.putAllData.length; i++) {
+@Unretained
+EntryEventImpl entryEvent = getEventForPosition(i);
+if (entryEvent != null) {
+  if (cq != null && cq.getFilterID() != null && 
cq.getFilterID().equals(cqID)
+  && (e.getValue().equals(MessageType.LOCAL_DESTROY))
+  && entryEvent.getKey() != null) {
+cq.removeFromCqResultKeys(entryEvent.getKey(), true);
+  }
+
+}
+  }
+}
+  }

Review comment:
   In the future, you might want to try extracting some of those 
inner-loops or inner-if-blocks to their own methods. How you name those methods 
can then really improve understanding. It's also too easy to get lost in those 
deeply nested structures like this.
   





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


> NullPointerException during PutAll with CQ LOCAL_DESTROY event
> --
>
> Key: GEODE-9016
> URL: https://issues.apache.org/jira/browse/GEODE-9016
> Project: Geode
>  Issue Type: Bug
>Reporter: Jianxia Chen
>Assignee: Jianxia Chen
>Priority: Major
>  Labels: pull-request-available
>
> It is possible that PutAll operation hits a NPE when CQ LOCAL_DESTROY event 
> is generated.
> {code:java}
> java.lang.NullPointerException
> at 
> java.util.concurrent.ConcurrentHashMap.get(ConcurrentHashMap.java:936)
> at 
> org.apache.geode.cache.query.cq.internal.ServerCQResultsCachePartitionRegionImpl.remove(ServerCQResultsCachePartitionRegionImpl.java:69)
> at 
> org.apache.geode.cache.query.cq.internal.ServerCQImpl.removeFromCqResultKeys(ServerCQImpl.java:297)
> at 
> org.apache.geode.internal.cache.DistributedCacheOperation.removeDestroyTokensFromCqResultKeys(DistributedCacheOperation.java:743)
> at 
> org.apache.geode.internal.cache.DistributedCacheOperation._distribute(DistributedCacheOperation.java:693)
> at 
> org.apache.geode.internal.cache.DistributedCacheOperation.startOperation(DistributedCacheOperation.java:277)
> at 
> org.apache.geode.internal.cache.DistributedRegion.postPutAllSend(DistributedRegion.java:3304)
> at 
> org.apache.geode.internal.cache.LocalRegionDataView.postPutAll(LocalRegionDataView.java:358)
> at 
> org.apache.geode.internal.cache.partitioned.PutAllPRMessage.doPostPutAll(PutAllPRMessage.java:568)
> at 
> org.apache.geode.internal.cache.partitioned.PutAllPRMessage.doLocalPutAll(PutAllPRMessage.java:507)
> at 
> org.apache.geode.internal.cache.partitioned.PutAllPRMessage.operateOnPartitionedRegion(PutAllPRMessage.java:326)
> at 
> org.apache.geode.internal.cache.partitioned.PartitionMessage.process(PartitionMessage.java:333)
> at 
> org.apache.geode.distributed.internal.DistributionMessage.scheduleAction(DistributionMessage.java:376)
> at 
> 

[jira] [Commented] (GEODE-9016) NullPointerException during PutAll with CQ LOCAL_DESTROY event

2021-03-09 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/GEODE-9016?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17298296#comment-17298296
 ] 

ASF GitHub Bot commented on GEODE-9016:
---

kirklund commented on a change in pull request #6104:
URL: https://github.com/apache/geode/pull/6104#discussion_r590674686



##
File path: 
geode-cq/src/distributedTest/java/org/apache/geode/cache/query/cq/dunit/PartitionedRegionCqQueryDUnitTest.java
##
@@ -120,6 +123,60 @@
 
   private static int bridgeServerPort;
 
+  @Test
+  public void testPutAllWithCQLocalDestroy() throws Exception {
+VM server1 = getVM(0);
+VM server2 = getVM(1);
+VM client = getVM(2);
+
+final String cqName = "testPutAllWithCQLocalDestroy_0";
+createServer(server1);
+createServer(server2);
+final String host = Host.getHost(0).getHostName();
+final int port = server2.invoke(() -> 
PartitionedRegionCqQueryDUnitTest.getCacheServerPort());
+createClient(client, port, host);
+createCQ(client, cqName, cqs[0]);
+
+int numObjects = 1000;
+
+server1.invoke(() -> {
+  Region region = getCache().getRegion(SEPARATOR + "root" + SEPARATOR + 
regions[0]);
+  Map buffer = new HashMap();

Review comment:
   PS: You don't really need `SEPARATOR + "root" + SEPARATOR`. This will 
also work:
   ```
   getCache().getRegion(regions[0]);
   ```





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


> NullPointerException during PutAll with CQ LOCAL_DESTROY event
> --
>
> Key: GEODE-9016
> URL: https://issues.apache.org/jira/browse/GEODE-9016
> Project: Geode
>  Issue Type: Bug
>Reporter: Jianxia Chen
>Assignee: Jianxia Chen
>Priority: Major
>  Labels: pull-request-available
>
> It is possible that PutAll operation hits a NPE when CQ LOCAL_DESTROY event 
> is generated.
> {code:java}
> java.lang.NullPointerException
> at 
> java.util.concurrent.ConcurrentHashMap.get(ConcurrentHashMap.java:936)
> at 
> org.apache.geode.cache.query.cq.internal.ServerCQResultsCachePartitionRegionImpl.remove(ServerCQResultsCachePartitionRegionImpl.java:69)
> at 
> org.apache.geode.cache.query.cq.internal.ServerCQImpl.removeFromCqResultKeys(ServerCQImpl.java:297)
> at 
> org.apache.geode.internal.cache.DistributedCacheOperation.removeDestroyTokensFromCqResultKeys(DistributedCacheOperation.java:743)
> at 
> org.apache.geode.internal.cache.DistributedCacheOperation._distribute(DistributedCacheOperation.java:693)
> at 
> org.apache.geode.internal.cache.DistributedCacheOperation.startOperation(DistributedCacheOperation.java:277)
> at 
> org.apache.geode.internal.cache.DistributedRegion.postPutAllSend(DistributedRegion.java:3304)
> at 
> org.apache.geode.internal.cache.LocalRegionDataView.postPutAll(LocalRegionDataView.java:358)
> at 
> org.apache.geode.internal.cache.partitioned.PutAllPRMessage.doPostPutAll(PutAllPRMessage.java:568)
> at 
> org.apache.geode.internal.cache.partitioned.PutAllPRMessage.doLocalPutAll(PutAllPRMessage.java:507)
> at 
> org.apache.geode.internal.cache.partitioned.PutAllPRMessage.operateOnPartitionedRegion(PutAllPRMessage.java:326)
> at 
> org.apache.geode.internal.cache.partitioned.PartitionMessage.process(PartitionMessage.java:333)
> at 
> org.apache.geode.distributed.internal.DistributionMessage.scheduleAction(DistributionMessage.java:376)
> at 
> org.apache.geode.distributed.internal.DistributionMessage$1.run(DistributionMessage.java:440)
> at 
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
> at 
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
> at 
> org.apache.geode.distributed.internal.ClusterOperationExecutors.runUntilShutdown(ClusterOperationExecutors.java:442)
> at 
> org.apache.geode.distributed.internal.ClusterOperationExecutors.doPartitionRegionThread(ClusterOperationExecutors.java:422)
> at 
> org.apache.geode.logging.internal.executors.LoggingThreadFactory.lambda$newThread$0(LoggingThreadFactory.java:119)
> at java.lang.Thread.run(Thread.java:748)
> {code}



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (GEODE-9016) NullPointerException during PutAll with CQ LOCAL_DESTROY event

2021-03-09 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/GEODE-9016?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17298295#comment-17298295
 ] 

ASF GitHub Bot commented on GEODE-9016:
---

kirklund commented on a change in pull request #6104:
URL: https://github.com/apache/geode/pull/6104#discussion_r59074



##
File path: 
geode-cq/src/distributedTest/java/org/apache/geode/cache/query/cq/dunit/PartitionedRegionCqQueryDUnitTest.java
##
@@ -120,6 +123,60 @@
 
   private static int bridgeServerPort;
 
+  @Test
+  public void testPutAllWithCQLocalDestroy() throws Exception {
+VM server1 = getVM(0);
+VM server2 = getVM(1);
+VM client = getVM(2);
+
+final String cqName = "testPutAllWithCQLocalDestroy_0";
+createServer(server1);
+createServer(server2);
+final String host = Host.getHost(0).getHostName();
+final int port = server2.invoke(() -> 
PartitionedRegionCqQueryDUnitTest.getCacheServerPort());
+createClient(client, port, host);
+createCQ(client, cqName, cqs[0]);
+
+int numObjects = 1000;
+
+server1.invoke(() -> {
+  Region region = getCache().getRegion(SEPARATOR + "root" + SEPARATOR + 
regions[0]);
+  Map buffer = new HashMap();
+  for (int i = 1; i < numObjects; i++) {
+Portfolio p = new Portfolio(i);
+buffer.put("" + i, p);
+  }
+  region.putAll(buffer);
+});
+
+client.invoke(() -> {
+  QueryService cqService = getCache().getQueryService();
+  CqQuery cqQuery = cqService.getCq(cqName);
+  if (cqQuery == null) {
+fail("Failed to get CQ " + cqName);
+  }
+  try {
+cqQuery.executeWithInitialResults();
+  } catch (Exception ex) {
+fail("Failed to execute  CQ " + cqName, ex);
+  }
+});
+
+server1.invoke(() -> {
+  Region region = getCache().getRegion(SEPARATOR + "root" + SEPARATOR + 
regions[0]);
+  Map buffer = new HashMap();
+  for (int i = 1; i < numObjects; i++) {
+Portfolio p = new Portfolio(-1 * i);
+buffer.put("" + i, p);
+  }
+  region.putAll(buffer);
+});
+
+cqHelper.closeClient(client);
+cqHelper.closeServer(server2);
+cqHelper.closeServer(server1);

Review comment:
   It's better (and more industry standard for writing JUnit tests) if you 
can figure out a way to move cleanup code from the end of the test method to a 
`tearDown` method annotated with `@After`. For example, if you make 
`cqHelper.closeClient` return without error if client is null or not running, 
then you could hoist these local variables to fields and the test would look 
more like this:
   ```
   private VM client;
   private VM server1;
   private VM server2;
   
   @Before 
   public void setUp() {
server1 = getVM(0);
server2 = getVM(1);
client = getVM(2);
   }
   
   @After
   public void tearDown() {
 cqHelper.closeClient(client);
 cqHelper.closeServer(server2);
 cqHelper.closeServer(server1);
   }
   ```
   And `cqHelper` close methods could have some early-outs like this:
   ```
   public void closeClient(VM vm) {
 if (vm == null) {
   return;
 }
 // etc
   }
   ```





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


> NullPointerException during PutAll with CQ LOCAL_DESTROY event
> --
>
> Key: GEODE-9016
> URL: https://issues.apache.org/jira/browse/GEODE-9016
> Project: Geode
>  Issue Type: Bug
>Reporter: Jianxia Chen
>Assignee: Jianxia Chen
>Priority: Major
>  Labels: pull-request-available
>
> It is possible that PutAll operation hits a NPE when CQ LOCAL_DESTROY event 
> is generated.
> {code:java}
> java.lang.NullPointerException
> at 
> java.util.concurrent.ConcurrentHashMap.get(ConcurrentHashMap.java:936)
> at 
> org.apache.geode.cache.query.cq.internal.ServerCQResultsCachePartitionRegionImpl.remove(ServerCQResultsCachePartitionRegionImpl.java:69)
> at 
> org.apache.geode.cache.query.cq.internal.ServerCQImpl.removeFromCqResultKeys(ServerCQImpl.java:297)
> at 
> org.apache.geode.internal.cache.DistributedCacheOperation.removeDestroyTokensFromCqResultKeys(DistributedCacheOperation.java:743)
> at 
> org.apache.geode.internal.cache.DistributedCacheOperation._distribute(DistributedCacheOperation.java:693)
> at 
> org.apache.geode.internal.cache.DistributedCacheOperation.startOperation(DistributedCacheOperation.java:277)
> at 
> org.apache.geode.internal.cache.DistributedRegion.postPutAllSend(DistributedRegion.java:3304)
> at 

[jira] [Commented] (GEODE-9016) NullPointerException during PutAll with CQ LOCAL_DESTROY event

2021-03-09 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/GEODE-9016?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17298293#comment-17298293
 ] 

ASF GitHub Bot commented on GEODE-9016:
---

kirklund commented on a change in pull request #6104:
URL: https://github.com/apache/geode/pull/6104#discussion_r590657219



##
File path: 
geode-core/src/main/java/org/apache/geode/internal/cache/DistributedPutAllOperation.java
##
@@ -814,6 +818,46 @@ protected FilterRoutingInfo getRecipientFilterRouting(Set 
cacheOpRecipients) {
 return consolidated;
   }
 
+  @Override
+  protected void removeDestroyTokensFromCqResultKeys(FilterRoutingInfo 
filterRouting) {
+for (InternalDistributedMember m : filterRouting.getMembers()) {
+  FilterInfo filterInfo = filterRouting.getFilterInfo(m);
+  if (filterInfo.getCQs() == null) {
+continue;
+  }
+
+  CacheDistributionAdvisor.CacheProfile cf =
+  (CacheDistributionAdvisor.CacheProfile) ((Bucket) 
getRegion()).getPartitionedRegion()
+  .getCacheDistributionAdvisor().getProfile(m);
+
+  if (cf == null || cf.filterProfile == null || 
cf.filterProfile.isLocalProfile()
+  || cf.filterProfile.getCqMap().isEmpty()) {
+continue;
+  }
+
+  for (Object value : cf.filterProfile.getCqMap().values()) {

Review comment:
   In the future, you might want to try extracting some of those 
inner-loops or inner-if-blocks to their own methods. How you name those methods 
can then really improve understanding. It's also too easy to get lost in those 
deeply nested structures like this.

##
File path: 
geode-cq/src/distributedTest/java/org/apache/geode/cache/query/cq/dunit/PartitionedRegionCqQueryDUnitTest.java
##
@@ -120,6 +123,60 @@
 
   private static int bridgeServerPort;
 
+  @Test
+  public void testPutAllWithCQLocalDestroy() throws Exception {
+VM server1 = getVM(0);
+VM server2 = getVM(1);
+VM client = getVM(2);
+
+final String cqName = "testPutAllWithCQLocalDestroy_0";
+createServer(server1);
+createServer(server2);
+final String host = Host.getHost(0).getHostName();
+final int port = server2.invoke(() -> 
PartitionedRegionCqQueryDUnitTest.getCacheServerPort());
+createClient(client, port, host);
+createCQ(client, cqName, cqs[0]);
+
+int numObjects = 1000;
+
+server1.invoke(() -> {
+  Region region = getCache().getRegion(SEPARATOR + "root" + SEPARATOR + 
regions[0]);
+  Map buffer = new HashMap();
+  for (int i = 1; i < numObjects; i++) {
+Portfolio p = new Portfolio(i);
+buffer.put("" + i, p);
+  }
+  region.putAll(buffer);
+});
+
+client.invoke(() -> {
+  QueryService cqService = getCache().getQueryService();
+  CqQuery cqQuery = cqService.getCq(cqName);
+  if (cqQuery == null) {
+fail("Failed to get CQ " + cqName);
+  }

Review comment:
   Please use assertions instead of if-fail patterns:
   ```
   assertThat(cqQuery)
   .withFailMessage("Failed to get CQ " + cqName)
   .isNotNull();
   ```

##
File path: 
geode-cq/src/distributedTest/java/org/apache/geode/cache/query/cq/dunit/PartitionedRegionCqQueryDUnitTest.java
##
@@ -120,6 +123,60 @@
 
   private static int bridgeServerPort;
 
+  @Test
+  public void testPutAllWithCQLocalDestroy() throws Exception {
+VM server1 = getVM(0);
+VM server2 = getVM(1);
+VM client = getVM(2);
+
+final String cqName = "testPutAllWithCQLocalDestroy_0";
+createServer(server1);
+createServer(server2);
+final String host = Host.getHost(0).getHostName();
+final int port = server2.invoke(() -> 
PartitionedRegionCqQueryDUnitTest.getCacheServerPort());
+createClient(client, port, host);
+createCQ(client, cqName, cqs[0]);
+
+int numObjects = 1000;
+
+server1.invoke(() -> {
+  Region region = getCache().getRegion(SEPARATOR + "root" + SEPARATOR + 
regions[0]);
+  Map buffer = new HashMap();

Review comment:
   You should avoid raw types in both product and test code. For example: 
`Region` or even `Region`. Same with Map.

##
File path: 
geode-cq/src/distributedTest/java/org/apache/geode/cache/query/cq/dunit/PartitionedRegionCqQueryDUnitTest.java
##
@@ -120,6 +123,60 @@
 
   private static int bridgeServerPort;
 
+  @Test
+  public void testPutAllWithCQLocalDestroy() throws Exception {
+VM server1 = getVM(0);
+VM server2 = getVM(1);
+VM client = getVM(2);
+
+final String cqName = "testPutAllWithCQLocalDestroy_0";
+createServer(server1);
+createServer(server2);
+final String host = Host.getHost(0).getHostName();

Review comment:
   Avoid `Host` and just use:
   ```
   String hostName = VM.getHostName();
   ```

##
File path: 
geode-cq/src/distributedTest/java/org/apache/geode/cache/query/cq/dunit/PartitionedRegionCqQueryDUnitTest.java
##
@@ 

[jira] [Commented] (GEODE-8965) Implement Redis "noevict" policy for Geode Redis

2021-03-09 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/GEODE-8965?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17298292#comment-17298292
 ] 

ASF GitHub Bot commented on GEODE-8965:
---

ringles commented on a change in pull request #6085:
URL: https://github.com/apache/geode/pull/6085#discussion_r590672235



##
File path: 
geode-redis/src/distributedTest/java/org/apache/geode/redis/OutOfMemoryDUnitTest.java
##
@@ -0,0 +1,188 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more 
contributor license
+ * agreements. See the NOTICE file distributed with this work for additional 
information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache 
License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the 
License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software 
distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY 
KIND, either express
+ * or implied. See the License for the specific language governing permissions 
and limitations under
+ * the License.
+ */
+
+package org.apache.geode.redis;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatNoException;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+import java.util.Arrays;
+import java.util.Properties;
+
+import org.junit.AfterClass;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.ClassRule;
+import org.junit.Rule;
+import org.junit.Test;
+import redis.clients.jedis.Jedis;
+import redis.clients.jedis.exceptions.JedisException;
+
+import org.apache.geode.test.awaitility.GeodeAwaitility;
+import org.apache.geode.test.dunit.IgnoredException;
+import org.apache.geode.test.dunit.rules.MemberVM;
+import org.apache.geode.test.dunit.rules.RedisClusterStartupRule;
+import org.apache.geode.test.junit.rules.ExecutorServiceRule;
+
+public class OutOfMemoryDUnitTest {
+
+  @ClassRule
+  public static RedisClusterStartupRule clusterStartUp = new 
RedisClusterStartupRule(4);
+
+  @Rule
+  public ExecutorServiceRule executor = new ExecutorServiceRule();
+
+  private static final String expectedEx = "Member: .*? above .*? critical 
threshold";
+  public static final String FILLER_KEY = "fillerKey-";
+  private static final String LOCAL_HOST = "127.0.0.1";
+  public static final int KEY_TTL_SECONDS = 10;
+  private static final int MAX_ITERATION_COUNT = 4000;
+  public static final int LARGE_VALUE_SIZE = 128 * 1024;
+  public static final int SMALL_VALUE_SIZE = 16 * 1024;
+  private static final int JEDIS_TIMEOUT =
+  Math.toIntExact(GeodeAwaitility.getTimeout().toMillis());
+  private static Jedis jedis1;
+  private static Jedis jedis2;
+
+  private static MemberVM server1;
+  private static MemberVM server2;
+
+  @BeforeClass
+  public static void classSetup() {
+Properties serverProperties;
+MemberVM locator;
+int redisServerPort1;
+int redisServerPort2;
+
+IgnoredException.addIgnoredException(expectedEx);
+serverProperties = new Properties();
+
+locator = clusterStartUp.startLocatorVM(0);
+server1 = clusterStartUp.startRedisVM(1, serverProperties, 
locator.getPort());
+server2 = clusterStartUp.startRedisVM(2, serverProperties, 
locator.getPort());
+
+server1.getVM().invoke(() -> 
RedisClusterStartupRule.getCache().getResourceManager()
+.setCriticalHeapPercentage(5.0F));
+server2.getVM().invoke(() -> 
RedisClusterStartupRule.getCache().getResourceManager()
+.setCriticalHeapPercentage(5.0F));
+
+redisServerPort1 = clusterStartUp.getRedisPort(1);
+redisServerPort2 = clusterStartUp.getRedisPort(2);
+
+jedis1 = new Jedis(LOCAL_HOST, redisServerPort1, JEDIS_TIMEOUT);
+jedis2 = new Jedis(LOCAL_HOST, redisServerPort2, JEDIS_TIMEOUT);
+  }
+
+  @Before
+  public void testSetup() {
+jedis1.flushAll();
+  }
+
+  @AfterClass
+  public static void tearDown() {
+jedis1.disconnect();
+jedis2.disconnect();
+
+server1.stop();
+server2.stop();
+  }
+
+  @Test
+  public void shouldReturnOOMError_forWriteOperations_whenThresholdReached() {
+IgnoredException.addIgnoredException(expectedEx);
+IgnoredException.addIgnoredException("LowMemoryException");
+
+fillMemory(jedis1, false);
+
+assertThatThrownBy(() -> jedis2.set("oneMoreKey", makeLongStringValue(2 * 
LARGE_VALUE_SIZE)))
+.hasMessageContaining("OOM");
+  }
+
+  @Test
+  public void shouldAllowDeleteOperations_afterThresholdReached() {
+IgnoredException.addIgnoredException(expectedEx);
+IgnoredException.addIgnoredException("LowMemoryException");
+
+fillMemory(jedis1, false);
+
+assertThatNoException().isThrownBy(() -> jedis2.del(FILLER_KEY + 1));

[jira] [Commented] (GEODE-8965) Implement Redis "noevict" policy for Geode Redis

2021-03-09 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/GEODE-8965?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17298277#comment-17298277
 ] 

ASF GitHub Bot commented on GEODE-8965:
---

ringles commented on a change in pull request #6085:
URL: https://github.com/apache/geode/pull/6085#discussion_r590647943



##
File path: 
geode-redis/src/distributedTest/java/org/apache/geode/redis/OutOfMemoryDUnitTest.java
##
@@ -0,0 +1,175 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more 
contributor license
+ * agreements. See the NOTICE file distributed with this work for additional 
information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache 
License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the 
License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software 
distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY 
KIND, either express
+ * or implied. See the License for the specific language governing permissions 
and limitations under
+ * the License.
+ */
+
+package org.apache.geode.redis;
+
+import static 
org.apache.geode.distributed.ConfigurationProperties.MAX_WAIT_TIME_RECONNECT;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatNoException;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+import java.util.Arrays;
+import java.util.Properties;
+
+import org.junit.AfterClass;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.ClassRule;
+import org.junit.Rule;
+import org.junit.Test;
+import redis.clients.jedis.Jedis;
+import redis.clients.jedis.exceptions.JedisException;
+
+import org.apache.geode.test.awaitility.GeodeAwaitility;
+import org.apache.geode.test.dunit.IgnoredException;
+import org.apache.geode.test.dunit.rules.MemberVM;
+import org.apache.geode.test.dunit.rules.RedisClusterStartupRule;
+import org.apache.geode.test.junit.rules.ExecutorServiceRule;
+
+@SuppressWarnings("unchecked")
+public class OutOfMemoryDUnitTest {
+
+  @ClassRule
+  public static RedisClusterStartupRule clusterStartUp = new 
RedisClusterStartupRule(4);
+
+  @Rule
+  public ExecutorServiceRule executor = new ExecutorServiceRule();
+
+  private static final String expectedEx = "Member: .*? above .*? critical 
threshold";
+  public static final String FILLER_KEY = "fillerKey-";
+  private static final String LOCAL_HOST = "127.0.0.1";
+  public static final int KEY_TTL_SECONDS = 10;
+  private static final int MAX_ITERATION_COUNT = 4000;
+  public static final int VALUE_SIZE = 128 * 1024;
+  private static final int JEDIS_TIMEOUT =
+  Math.toIntExact(GeodeAwaitility.getTimeout().toMillis());
+  private static Jedis jedis1;
+  private static Jedis jedis2;
+
+  private static Properties locatorProperties;
+  private static Properties serverProperties;
+
+  private static MemberVM locator;
+  private static MemberVM server1;
+  private static MemberVM server2;
+
+  private static int redisServerPort1;
+  private static int redisServerPort2;
+  private static String valueString = "";
+
+  @BeforeClass
+  public static void classSetup() {
+IgnoredException.addIgnoredException(expectedEx);
+locatorProperties = new Properties();
+serverProperties = new Properties();
+locatorProperties.setProperty(MAX_WAIT_TIME_RECONNECT, "15000");
+
+locator = clusterStartUp.startLocatorVM(0, locatorProperties);
+server1 = clusterStartUp.startRedisVM(1, serverProperties, 
locator.getPort());
+server2 = clusterStartUp.startRedisVM(2, serverProperties, 
locator.getPort());
+
+server1.getVM().invoke(() -> {
+  
RedisClusterStartupRule.getCache().getResourceManager().setCriticalHeapPercentage(5.0F);
+});
+server2.getVM().invoke(() -> {
+  
RedisClusterStartupRule.getCache().getResourceManager().setCriticalHeapPercentage(5.0F);
+});
+
+redisServerPort1 = clusterStartUp.getRedisPort(1);
+redisServerPort2 = clusterStartUp.getRedisPort(2);
+
+jedis1 = new Jedis(LOCAL_HOST, redisServerPort1, JEDIS_TIMEOUT);
+jedis2 = new Jedis(LOCAL_HOST, redisServerPort2, JEDIS_TIMEOUT);
+
+char[] largeCharData = new char[VALUE_SIZE];
+Arrays.fill(largeCharData, 'a');
+valueString = new String(largeCharData);
+
+  }
+
+  @Before
+  public void testSetup() {
+jedis1.flushAll();
+  }
+
+  @AfterClass
+  public static void tearDown() {
+jedis1.disconnect();
+jedis2.disconnect();
+
+server1.stop();
+server2.stop();
+  }
+
+  @Test
+  public void shouldReturnOOMError_forWriteOperations_whenThresholdReached() {
+IgnoredException.addIgnoredException(expectedEx);
+

[jira] [Commented] (GEODE-8965) Implement Redis "noevict" policy for Geode Redis

2021-03-09 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/GEODE-8965?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17298275#comment-17298275
 ] 

ASF GitHub Bot commented on GEODE-8965:
---

ringles commented on a change in pull request #6085:
URL: https://github.com/apache/geode/pull/6085#discussion_r590647743



##
File path: 
geode-redis/src/distributedTest/java/org/apache/geode/redis/OutOfMemoryDUnitTest.java
##
@@ -0,0 +1,188 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more 
contributor license
+ * agreements. See the NOTICE file distributed with this work for additional 
information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache 
License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the 
License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software 
distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY 
KIND, either express
+ * or implied. See the License for the specific language governing permissions 
and limitations under
+ * the License.
+ */
+
+package org.apache.geode.redis;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatNoException;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+import java.util.Arrays;
+import java.util.Properties;
+
+import org.junit.AfterClass;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.ClassRule;
+import org.junit.Rule;
+import org.junit.Test;
+import redis.clients.jedis.Jedis;
+import redis.clients.jedis.exceptions.JedisException;
+
+import org.apache.geode.test.awaitility.GeodeAwaitility;
+import org.apache.geode.test.dunit.IgnoredException;
+import org.apache.geode.test.dunit.rules.MemberVM;
+import org.apache.geode.test.dunit.rules.RedisClusterStartupRule;
+import org.apache.geode.test.junit.rules.ExecutorServiceRule;

Review comment:
    





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


> Implement Redis "noevict" policy for Geode Redis
> 
>
> Key: GEODE-8965
> URL: https://issues.apache.org/jira/browse/GEODE-8965
> Project: Geode
>  Issue Type: New Feature
>  Components: redis
>Reporter: Raymond Ingles
>Assignee: Raymond Ingles
>Priority: Major
>  Labels: blocks-1.14.0​, pull-request-available
>
> Redis supports the "noevict" eviction policy. When this is selected, Redis 
> returns an OOM error when commands that consume new memory are attempted. New 
> data cannot be written to the cache until either TTL expiration or explicit 
> deletion of keys has freed enough memory to allow write operations again.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (GEODE-8965) Implement Redis "noevict" policy for Geode Redis

2021-03-09 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/GEODE-8965?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17298272#comment-17298272
 ] 

ASF GitHub Bot commented on GEODE-8965:
---

ringles commented on a change in pull request #6085:
URL: https://github.com/apache/geode/pull/6085#discussion_r590647230



##
File path: 
geode-redis/src/distributedTest/java/org/apache/geode/redis/OutOfMemoryDUnitTest.java
##
@@ -0,0 +1,188 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more 
contributor license
+ * agreements. See the NOTICE file distributed with this work for additional 
information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache 
License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the 
License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software 
distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY 
KIND, either express
+ * or implied. See the License for the specific language governing permissions 
and limitations under
+ * the License.
+ */
+
+package org.apache.geode.redis;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatNoException;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+import java.util.Arrays;
+import java.util.Properties;
+
+import org.junit.AfterClass;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.ClassRule;
+import org.junit.Rule;
+import org.junit.Test;
+import redis.clients.jedis.Jedis;
+import redis.clients.jedis.exceptions.JedisException;
+
+import org.apache.geode.test.awaitility.GeodeAwaitility;
+import org.apache.geode.test.dunit.IgnoredException;
+import org.apache.geode.test.dunit.rules.MemberVM;
+import org.apache.geode.test.dunit.rules.RedisClusterStartupRule;
+import org.apache.geode.test.junit.rules.ExecutorServiceRule;
+
+public class OutOfMemoryDUnitTest {
+
+  @ClassRule
+  public static RedisClusterStartupRule clusterStartUp = new 
RedisClusterStartupRule(4);
+
+  @Rule
+  public ExecutorServiceRule executor = new ExecutorServiceRule();
+
+  private static final String expectedEx = "Member: .*? above .*? critical 
threshold";
+  public static final String FILLER_KEY = "fillerKey-";
+  private static final String LOCAL_HOST = "127.0.0.1";
+  public static final int KEY_TTL_SECONDS = 10;
+  private static final int MAX_ITERATION_COUNT = 4000;
+  public static final int LARGE_VALUE_SIZE = 128 * 1024;
+  public static final int SMALL_VALUE_SIZE = 16 * 1024;
+  private static final int JEDIS_TIMEOUT =
+  Math.toIntExact(GeodeAwaitility.getTimeout().toMillis());
+  private static Jedis jedis1;
+  private static Jedis jedis2;
+
+  private static MemberVM server1;
+  private static MemberVM server2;
+
+  @BeforeClass
+  public static void classSetup() {
+Properties serverProperties;
+MemberVM locator;
+int redisServerPort1;
+int redisServerPort2;
+
+IgnoredException.addIgnoredException(expectedEx);
+serverProperties = new Properties();
+
+locator = clusterStartUp.startLocatorVM(0);
+server1 = clusterStartUp.startRedisVM(1, serverProperties, 
locator.getPort());
+server2 = clusterStartUp.startRedisVM(2, serverProperties, 
locator.getPort());
+
+server1.getVM().invoke(() -> 
RedisClusterStartupRule.getCache().getResourceManager()
+.setCriticalHeapPercentage(5.0F));
+server2.getVM().invoke(() -> 
RedisClusterStartupRule.getCache().getResourceManager()
+.setCriticalHeapPercentage(5.0F));
+
+redisServerPort1 = clusterStartUp.getRedisPort(1);
+redisServerPort2 = clusterStartUp.getRedisPort(2);
+
+jedis1 = new Jedis(LOCAL_HOST, redisServerPort1, JEDIS_TIMEOUT);
+jedis2 = new Jedis(LOCAL_HOST, redisServerPort2, JEDIS_TIMEOUT);
+  }
+
+  @Before
+  public void testSetup() {
+jedis1.flushAll();
+  }
+
+  @AfterClass
+  public static void tearDown() {
+jedis1.disconnect();
+jedis2.disconnect();
+
+server1.stop();
+server2.stop();
+  }
+
+  @Test
+  public void shouldReturnOOMError_forWriteOperations_whenThresholdReached() {
+IgnoredException.addIgnoredException(expectedEx);
+IgnoredException.addIgnoredException("LowMemoryException");
+
+fillMemory(jedis1, false);
+
+assertThatThrownBy(() -> jedis2.set("oneMoreKey", makeLongStringValue(2 * 
LARGE_VALUE_SIZE)))
+.hasMessageContaining("OOM");
+  }
+
+  @Test
+  public void shouldAllowDeleteOperations_afterThresholdReached() {
+IgnoredException.addIgnoredException(expectedEx);
+IgnoredException.addIgnoredException("LowMemoryException");
+
+fillMemory(jedis1, false);
+
+assertThatNoException().isThrownBy(() -> jedis2.del(FILLER_KEY + 1));

[jira] [Commented] (GEODE-8965) Implement Redis "noevict" policy for Geode Redis

2021-03-09 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/GEODE-8965?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17298271#comment-17298271
 ] 

ASF GitHub Bot commented on GEODE-8965:
---

ringles commented on a change in pull request #6085:
URL: https://github.com/apache/geode/pull/6085#discussion_r590646547



##
File path: 
geode-redis/src/distributedTest/java/org/apache/geode/redis/OutOfMemoryDUnitTest.java
##
@@ -0,0 +1,188 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more 
contributor license
+ * agreements. See the NOTICE file distributed with this work for additional 
information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache 
License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the 
License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software 
distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY 
KIND, either express
+ * or implied. See the License for the specific language governing permissions 
and limitations under
+ * the License.
+ */
+
+package org.apache.geode.redis;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatNoException;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+import java.util.Arrays;
+import java.util.Properties;
+
+import org.junit.AfterClass;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.ClassRule;
+import org.junit.Rule;
+import org.junit.Test;
+import redis.clients.jedis.Jedis;
+import redis.clients.jedis.exceptions.JedisException;
+
+import org.apache.geode.test.awaitility.GeodeAwaitility;
+import org.apache.geode.test.dunit.IgnoredException;
+import org.apache.geode.test.dunit.rules.MemberVM;
+import org.apache.geode.test.dunit.rules.RedisClusterStartupRule;
+import org.apache.geode.test.junit.rules.ExecutorServiceRule;
+
+public class OutOfMemoryDUnitTest {
+
+  @ClassRule
+  public static RedisClusterStartupRule clusterStartUp = new 
RedisClusterStartupRule(4);
+
+  @Rule
+  public ExecutorServiceRule executor = new ExecutorServiceRule();
+
+  private static final String expectedEx = "Member: .*? above .*? critical 
threshold";
+  public static final String FILLER_KEY = "fillerKey-";
+  private static final String LOCAL_HOST = "127.0.0.1";
+  public static final int KEY_TTL_SECONDS = 10;
+  private static final int MAX_ITERATION_COUNT = 4000;
+  public static final int LARGE_VALUE_SIZE = 128 * 1024;
+  public static final int SMALL_VALUE_SIZE = 16 * 1024;
+  private static final int JEDIS_TIMEOUT =
+  Math.toIntExact(GeodeAwaitility.getTimeout().toMillis());
+  private static Jedis jedis1;
+  private static Jedis jedis2;
+
+  private static MemberVM server1;
+  private static MemberVM server2;
+
+  @BeforeClass
+  public static void classSetup() {
+Properties serverProperties;

Review comment:
   Done, for the rest of the items in classSetup().





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


> Implement Redis "noevict" policy for Geode Redis
> 
>
> Key: GEODE-8965
> URL: https://issues.apache.org/jira/browse/GEODE-8965
> Project: Geode
>  Issue Type: New Feature
>  Components: redis
>Reporter: Raymond Ingles
>Assignee: Raymond Ingles
>Priority: Major
>  Labels: blocks-1.14.0​, pull-request-available
>
> Redis supports the "noevict" eviction policy. When this is selected, Redis 
> returns an OOM error when commands that consume new memory are attempted. New 
> data cannot be written to the cache until either TTL expiration or explicit 
> deletion of keys has freed enough memory to allow write operations again.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (GEODE-8965) Implement Redis "noevict" policy for Geode Redis

2021-03-09 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/GEODE-8965?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17298270#comment-17298270
 ] 

ASF GitHub Bot commented on GEODE-8965:
---

ringles commented on a change in pull request #6085:
URL: https://github.com/apache/geode/pull/6085#discussion_r590645183



##
File path: 
geode-redis/src/distributedTest/java/org/apache/geode/redis/OutOfMemoryDUnitTest.java
##
@@ -0,0 +1,188 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more 
contributor license
+ * agreements. See the NOTICE file distributed with this work for additional 
information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache 
License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the 
License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software 
distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY 
KIND, either express
+ * or implied. See the License for the specific language governing permissions 
and limitations under
+ * the License.
+ */
+
+package org.apache.geode.redis;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatNoException;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+import java.util.Arrays;
+import java.util.Properties;
+
+import org.junit.AfterClass;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.ClassRule;
+import org.junit.Rule;
+import org.junit.Test;
+import redis.clients.jedis.Jedis;
+import redis.clients.jedis.exceptions.JedisException;
+
+import org.apache.geode.test.awaitility.GeodeAwaitility;
+import org.apache.geode.test.dunit.IgnoredException;
+import org.apache.geode.test.dunit.rules.MemberVM;
+import org.apache.geode.test.dunit.rules.RedisClusterStartupRule;
+import org.apache.geode.test.junit.rules.ExecutorServiceRule;
+
+public class OutOfMemoryDUnitTest {
+
+  @ClassRule
+  public static RedisClusterStartupRule clusterStartUp = new 
RedisClusterStartupRule(4);
+
+  @Rule
+  public ExecutorServiceRule executor = new ExecutorServiceRule();
+
+  private static final String expectedEx = "Member: .*? above .*? critical 
threshold";
+  public static final String FILLER_KEY = "fillerKey-";

Review comment:
   Done!
   
   (BTW, the intent is to test Geode's low memory handling, not the JVM. We're 
setting the CriticalHeapPercentage to force LowMemoryExceptions quickly. At 
that level, there should be no chance of actually filling up the JVM's heap.)





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


> Implement Redis "noevict" policy for Geode Redis
> 
>
> Key: GEODE-8965
> URL: https://issues.apache.org/jira/browse/GEODE-8965
> Project: Geode
>  Issue Type: New Feature
>  Components: redis
>Reporter: Raymond Ingles
>Assignee: Raymond Ingles
>Priority: Major
>  Labels: blocks-1.14.0​, pull-request-available
>
> Redis supports the "noevict" eviction policy. When this is selected, Redis 
> returns an OOM error when commands that consume new memory are attempted. New 
> data cannot be written to the cache until either TTL expiration or explicit 
> deletion of keys has freed enough memory to allow write operations again.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (GEODE-8971) Batches with incomplete transactions when stopping the gateway sender

2021-03-09 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/GEODE-8971?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17298263#comment-17298263
 ] 

ASF GitHub Bot commented on GEODE-8971:
---

albertogpz commented on a change in pull request #6052:
URL: https://github.com/apache/geode/pull/6052#discussion_r590641755



##
File path: 
geode-core/src/main/java/org/apache/geode/internal/cache/wan/AbstractGatewaySender.java
##
@@ -686,6 +689,22 @@ public boolean beforeEnqueue(GatewayQueueEvent 
gatewayEvent) {
 return enqueue;
   }
 
+  protected void preStop() {
+if (!mustGroupTransactionEvents() || isStopping) {

Review comment:
   You are right. I have removed the `isStopping` variable from the 
condition. 





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)


[jira] [Commented] (GEODE-8971) Batches with incomplete transactions when stopping the gateway sender

2021-03-09 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/GEODE-8971?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17298249#comment-17298249
 ] 

ASF GitHub Bot commented on GEODE-8971:
---

albertogpz commented on pull request #6052:
URL: https://github.com/apache/geode/pull/6052#issuecomment-794287751


   > Quite a lot of changes here to clean up the code. I'm also concerned that 
there's not enough test coverage for the changes in terms of possible failure 
conditions, such as if two threads call `stop()` on the same gateway sender 
within quick succession. More unit tests should also be added for the 
new/modified behaviour.
   
   Thanks a lot for your thorough review. Impressive, as usual.
   
   I have added a couple of tests in `SerialGatewaySenderImplTest` and 
`ParallelGatewalSenderImplTest`to verify that two threads calling `stop()` on 
the same gateway sender within quick succession does not provoke any harm and 
the `preStop` and `postStop` methods do their job.



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)


[jira] [Updated] (GEODE-8521) Add P2P message reader threads to thread monitoring

2021-03-09 Thread Darrel Schneider (Jira)


 [ 
https://issues.apache.org/jira/browse/GEODE-8521?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Darrel Schneider updated GEODE-8521:

Description: 
Add P2P message reader threads to thread monitoring to help with identifying 
stuck P2P message readers.
*NOTE:* If this fix is back ported then also back port: 
https://issues.apache.org/jira/browse/GEODE-8998

  was:Add P2P message reader threads to thread monitoring to help with 
identifying stuck P2P message readers.


> Add P2P message reader threads to thread monitoring
> ---
>
> Key: GEODE-8521
> URL: https://issues.apache.org/jira/browse/GEODE-8521
> Project: Geode
>  Issue Type: Wish
>  Components: messaging
>Reporter: Kirk Lund
>Assignee: Darrel Schneider
>Priority: Major
>  Labels: GeodeOperationAPI, pull-request-available
> Fix For: 1.14.0
>
>
> Add P2P message reader threads to thread monitoring to help with identifying 
> stuck P2P message readers.
> *NOTE:* If this fix is back ported then also back port: 
> https://issues.apache.org/jira/browse/GEODE-8998



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Reopened] (GEODE-8761) Add ServerConnection threads to ThreadMonitoring Service

2021-03-09 Thread Owen Nichols (Jira)


 [ 
https://issues.apache.org/jira/browse/GEODE-8761?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Owen Nichols reopened GEODE-8761:
-

reopened for backport to 1.14

> Add ServerConnection threads to ThreadMonitoring Service
> 
>
> Key: GEODE-8761
> URL: https://issues.apache.org/jira/browse/GEODE-8761
> Project: Geode
>  Issue Type: Improvement
>  Components: regions
>Affects Versions: 1.9.2, 1.10.0, 1.11.0, 1.12.0, 1.12.1, 1.13.0, 1.13.1, 
> 1.14.0
>Reporter: Anilkumar Gingade
>Assignee: Darrel Schneider
>Priority: Major
>  Labels: GeodeOperationAPI, blocks-1.14.0​, pull-request-available
> Fix For: 1.15.0
>
>
> In Geode the Thread Monitoring Service (TMS) allows to monitor a thread to 
> see if a thread is stuck doing particular operation; it provides thread dump 
> of stuck thread after configured time. This ticket is to add ServerConnection 
> threads to be monitored by TMS.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Updated] (GEODE-8761) Add ServerConnection threads to ThreadMonitoring Service

2021-03-09 Thread Owen Nichols (Jira)


 [ 
https://issues.apache.org/jira/browse/GEODE-8761?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Owen Nichols updated GEODE-8761:

Labels: GeodeOperationAPI blocks-1.14.0​ pull-request-available  (was: 
GeodeOperationAPI pull-request-available)

> Add ServerConnection threads to ThreadMonitoring Service
> 
>
> Key: GEODE-8761
> URL: https://issues.apache.org/jira/browse/GEODE-8761
> Project: Geode
>  Issue Type: Improvement
>  Components: regions
>Affects Versions: 1.9.2, 1.10.0, 1.11.0, 1.12.0, 1.12.1, 1.13.0, 1.13.1, 
> 1.14.0
>Reporter: Anilkumar Gingade
>Assignee: Darrel Schneider
>Priority: Major
>  Labels: GeodeOperationAPI, blocks-1.14.0​, pull-request-available
> Fix For: 1.15.0
>
>
> In Geode the Thread Monitoring Service (TMS) allows to monitor a thread to 
> see if a thread is stuck doing particular operation; it provides thread dump 
> of stuck thread after configured time. This ticket is to add ServerConnection 
> threads to be monitored by TMS.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Resolved] (GEODE-8992) When a GatewaySenderEventImpl is serialized, its operationDetail field is not included

2021-03-09 Thread Barrett Oglesby (Jira)


 [ 
https://issues.apache.org/jira/browse/GEODE-8992?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Barrett Oglesby resolved GEODE-8992.

Fix Version/s: 1.15.0
   Resolution: Fixed

> When a GatewaySenderEventImpl is serialized, its operationDetail field is not 
> included
> --
>
> Key: GEODE-8992
> URL: https://issues.apache.org/jira/browse/GEODE-8992
> Project: Geode
>  Issue Type: Bug
>  Components: wan
>Reporter: Barrett Oglesby
>Assignee: Barrett Oglesby
>Priority: Major
>  Labels: blocks-1.15.0​, pull-request-available
> Fix For: 1.15.0
>
>
> This causes the operation to become less specific when the 
> {{GatewaySenderEventImpl}} is deserialized.
> Here is an example.
> If the original {{GatewaySenderEventImpl}} is a *PUTALL_CREATE* like:
> {noformat}
> GatewaySenderEventImpl[id=EventID[id=31 
> bytes;threadID=0x10063|1;sequenceID=0;bucketId=99];action=0;operation=PUTALL_CREATE;region=/data;key=0;value=0;...]
> {noformat}
> Then, when the {{GatewaySenderEventImpl}} is serialized and deserialized, its 
> operation becomes a *CREATE*:
> {noformat}
> GatewaySenderEventImpl[id=EventID[id=31 
> bytes;threadID=0x10063|1;sequenceID=0;bucketId=99];action=0;operation=CREATE;region=/data;key=0;value=0;...]
> {noformat}
> Thats because {{GatewaySenderEventImpl.getOperation}} uses both *action* and 
> *operationDetail* to determine its operation:
> {noformat}
> public Operation getOperation() {
>   Operation op = null;
>   switch (this.action) {
> case CREATE_ACTION:
>   switch (this.operationDetail) {
> case ...
> case OP_DETAIL_PUTALL:
>   op = Operation.PUTALL_CREATE;
>   break;
> default:
>   op = Operation.CREATE;
>   break;
>   }
> ...
> {noformat}



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (GEODE-8761) Add ServerConnection threads to ThreadMonitoring Service

2021-03-09 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/GEODE-8761?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17298199#comment-17298199
 ] 

ASF GitHub Bot commented on GEODE-8761:
---

dschneider-pivotal opened a new pull request #6105:
URL: https://github.com/apache/geode/pull/6105


   (cherry picked from commit df7b5b167472d87d8d262515533e7c80d7306e67)
   
   Thank you for submitting a contribution to Apache Geode.
   
   In order to streamline the review of the contribution we ask you
   to ensure the following steps have been taken:
   
   ### For all changes:
   - [ ] Is there a JIRA ticket associated with this PR? Is it referenced in 
the commit message?
   
   - [ ] Has your PR been rebased against the latest commit within the target 
branch (typically `develop`)?
   
   - [ ] Is your initial contribution a single, squashed commit?
   
   - [ ] Does `gradlew build` run cleanly?
   
   - [ ] Have you written or updated unit tests to verify your changes?
   
   - [ ] If adding new dependencies to the code, are these dependencies 
licensed in a way that is compatible for inclusion under [ASF 
2.0](http://www.apache.org/legal/resolved.html#category-a)?
   
   ### Note:
   Please ensure that once the PR is submitted, check Concourse for build 
issues and
   submit an update to your PR as soon as possible. If you need help, please 
send an
   email to d...@geode.apache.org.
   



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


> Add ServerConnection threads to ThreadMonitoring Service
> 
>
> Key: GEODE-8761
> URL: https://issues.apache.org/jira/browse/GEODE-8761
> Project: Geode
>  Issue Type: Improvement
>  Components: regions
>Affects Versions: 1.9.2, 1.10.0, 1.11.0, 1.12.0, 1.12.1, 1.13.0, 1.13.1, 
> 1.14.0
>Reporter: Anilkumar Gingade
>Assignee: Darrel Schneider
>Priority: Major
>  Labels: GeodeOperationAPI, pull-request-available
> Fix For: 1.15.0
>
>
> In Geode the Thread Monitoring Service (TMS) allows to monitor a thread to 
> see if a thread is stuck doing particular operation; it provides thread dump 
> of stuck thread after configured time. This ticket is to add ServerConnection 
> threads to be monitored by TMS.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (GEODE-8761) Add ServerConnection threads to ThreadMonitoring Service

2021-03-09 Thread ASF subversion and git services (Jira)


[ 
https://issues.apache.org/jira/browse/GEODE-8761?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17298192#comment-17298192
 ] 

ASF subversion and git services commented on GEODE-8761:


Commit df7b5b167472d87d8d262515533e7c80d7306e67 in geode's branch 
refs/heads/develop from Darrel Schneider
[ https://gitbox.apache.org/repos/asf?p=geode.git;h=df7b5b1 ]

GEODE-8761: detect stuck server connection threads (#6094)



> Add ServerConnection threads to ThreadMonitoring Service
> 
>
> Key: GEODE-8761
> URL: https://issues.apache.org/jira/browse/GEODE-8761
> Project: Geode
>  Issue Type: Improvement
>  Components: regions
>Affects Versions: 1.9.2, 1.10.0, 1.11.0, 1.12.0, 1.12.1, 1.13.0, 1.13.1, 
> 1.14.0
>Reporter: Anilkumar Gingade
>Assignee: Darrel Schneider
>Priority: Major
>  Labels: GeodeOperationAPI, pull-request-available
> Fix For: 1.15.0
>
>
> In Geode the Thread Monitoring Service (TMS) allows to monitor a thread to 
> see if a thread is stuck doing particular operation; it provides thread dump 
> of stuck thread after configured time. This ticket is to add ServerConnection 
> threads to be monitored by TMS.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Updated] (GEODE-8761) Add ServerConnection threads to ThreadMonitoring Service

2021-03-09 Thread Darrel Schneider (Jira)


 [ 
https://issues.apache.org/jira/browse/GEODE-8761?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Darrel Schneider updated GEODE-8761:

Affects Version/s: 1.9.2
   1.10.0
   1.11.0
   1.12.0
   1.12.1
   1.13.0
   1.13.1

> Add ServerConnection threads to ThreadMonitoring Service
> 
>
> Key: GEODE-8761
> URL: https://issues.apache.org/jira/browse/GEODE-8761
> Project: Geode
>  Issue Type: Improvement
>  Components: regions
>Affects Versions: 1.9.2, 1.10.0, 1.11.0, 1.12.0, 1.12.1, 1.13.0, 1.13.1, 
> 1.14.0
>Reporter: Anilkumar Gingade
>Assignee: Darrel Schneider
>Priority: Major
>  Labels: GeodeOperationAPI, pull-request-available
>
> In Geode the Thread Monitoring Service (TMS) allows to monitor a thread to 
> see if a thread is stuck doing particular operation; it provides thread dump 
> of stuck thread after configured time. This ticket is to add ServerConnection 
> threads to be monitored by TMS.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Resolved] (GEODE-8761) Add ServerConnection threads to ThreadMonitoring Service

2021-03-09 Thread Darrel Schneider (Jira)


 [ 
https://issues.apache.org/jira/browse/GEODE-8761?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Darrel Schneider resolved GEODE-8761.
-
Fix Version/s: 1.15.0
   Resolution: Fixed

> Add ServerConnection threads to ThreadMonitoring Service
> 
>
> Key: GEODE-8761
> URL: https://issues.apache.org/jira/browse/GEODE-8761
> Project: Geode
>  Issue Type: Improvement
>  Components: regions
>Affects Versions: 1.9.2, 1.10.0, 1.11.0, 1.12.0, 1.12.1, 1.13.0, 1.13.1, 
> 1.14.0
>Reporter: Anilkumar Gingade
>Assignee: Darrel Schneider
>Priority: Major
>  Labels: GeodeOperationAPI, pull-request-available
> Fix For: 1.15.0
>
>
> In Geode the Thread Monitoring Service (TMS) allows to monitor a thread to 
> see if a thread is stuck doing particular operation; it provides thread dump 
> of stuck thread after configured time. This ticket is to add ServerConnection 
> threads to be monitored by TMS.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (GEODE-8761) Add ServerConnection threads to ThreadMonitoring Service

2021-03-09 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/GEODE-8761?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17298190#comment-17298190
 ] 

ASF GitHub Bot commented on GEODE-8761:
---

dschneider-pivotal merged pull request #6094:
URL: https://github.com/apache/geode/pull/6094


   



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


> Add ServerConnection threads to ThreadMonitoring Service
> 
>
> Key: GEODE-8761
> URL: https://issues.apache.org/jira/browse/GEODE-8761
> Project: Geode
>  Issue Type: Improvement
>  Components: regions
>Affects Versions: 1.14.0
>Reporter: Anilkumar Gingade
>Assignee: Darrel Schneider
>Priority: Major
>  Labels: GeodeOperationAPI, pull-request-available
>
> In Geode the Thread Monitoring Service (TMS) allows to monitor a thread to 
> see if a thread is stuck doing particular operation; it provides thread dump 
> of stuck thread after configured time. This ticket is to add ServerConnection 
> threads to be monitored by TMS.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (GEODE-8972) remove shunnedMembers collection from GMSMembership

2021-03-09 Thread ASF subversion and git services (Jira)


[ 
https://issues.apache.org/jira/browse/GEODE-8972?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17298141#comment-17298141
 ] 

ASF subversion and git services commented on GEODE-8972:


Commit c2fc107bad1de221157072470e2a6ad426533f20 in geode's branch 
refs/heads/develop from Kamilla Aslami
[ https://gitbox.apache.org/repos/asf?p=geode.git;h=c2fc107 ]

GEODE-8972: remove shunnedMembers collection from GMSMembership (#6089)

* GEODE-8972: remove shunnedMembers collection from GMSMembership

* Changes after the code review

> remove shunnedMembers collection from GMSMembership
> ---
>
> Key: GEODE-8972
> URL: https://issues.apache.org/jira/browse/GEODE-8972
> Project: Geode
>  Issue Type: Improvement
>  Components: membership
>Reporter: Bruce J Schuchardt
>Assignee: Kamilla Aslami
>Priority: Major
>  Labels: pull-request-available
>
> GMSMembership has a _shunnedMembers_ collection that is used to track the IDs 
> of nodes that are no longer part of the cluster.  This collection is no 
> longer needed since we can tell if a node is old by comparing the view ID in 
> its identifier to that of the current view (called _latestView_ in that 
> class.  Checks like this are already in place in some parts of the code.
> All uses of _shunnedMembers_ should be replaced with this check.
> MembershipView view = latestView;
> boolean shunned = memberId.getVmViewId() <= view.getViewId() && 
> !view.contains(memberId);



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (GEODE-8972) remove shunnedMembers collection from GMSMembership

2021-03-09 Thread ASF subversion and git services (Jira)


[ 
https://issues.apache.org/jira/browse/GEODE-8972?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17298142#comment-17298142
 ] 

ASF subversion and git services commented on GEODE-8972:


Commit c2fc107bad1de221157072470e2a6ad426533f20 in geode's branch 
refs/heads/develop from Kamilla Aslami
[ https://gitbox.apache.org/repos/asf?p=geode.git;h=c2fc107 ]

GEODE-8972: remove shunnedMembers collection from GMSMembership (#6089)

* GEODE-8972: remove shunnedMembers collection from GMSMembership

* Changes after the code review

> remove shunnedMembers collection from GMSMembership
> ---
>
> Key: GEODE-8972
> URL: https://issues.apache.org/jira/browse/GEODE-8972
> Project: Geode
>  Issue Type: Improvement
>  Components: membership
>Reporter: Bruce J Schuchardt
>Assignee: Kamilla Aslami
>Priority: Major
>  Labels: pull-request-available
>
> GMSMembership has a _shunnedMembers_ collection that is used to track the IDs 
> of nodes that are no longer part of the cluster.  This collection is no 
> longer needed since we can tell if a node is old by comparing the view ID in 
> its identifier to that of the current view (called _latestView_ in that 
> class.  Checks like this are already in place in some parts of the code.
> All uses of _shunnedMembers_ should be replaced with this check.
> MembershipView view = latestView;
> boolean shunned = memberId.getVmViewId() <= view.getViewId() && 
> !view.contains(memberId);



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (GEODE-8972) remove shunnedMembers collection from GMSMembership

2021-03-09 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/GEODE-8972?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17298139#comment-17298139
 ] 

ASF GitHub Bot commented on GEODE-8972:
---

bschuchardt merged pull request #6089:
URL: https://github.com/apache/geode/pull/6089


   



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


> remove shunnedMembers collection from GMSMembership
> ---
>
> Key: GEODE-8972
> URL: https://issues.apache.org/jira/browse/GEODE-8972
> Project: Geode
>  Issue Type: Improvement
>  Components: membership
>Reporter: Bruce J Schuchardt
>Assignee: Kamilla Aslami
>Priority: Major
>  Labels: pull-request-available
>
> GMSMembership has a _shunnedMembers_ collection that is used to track the IDs 
> of nodes that are no longer part of the cluster.  This collection is no 
> longer needed since we can tell if a node is old by comparing the view ID in 
> its identifier to that of the current view (called _latestView_ in that 
> class.  Checks like this are already in place in some parts of the code.
> All uses of _shunnedMembers_ should be replaced with this check.
> MembershipView view = latestView;
> boolean shunned = memberId.getVmViewId() <= view.getViewId() && 
> !view.contains(memberId);



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (GEODE-8971) Batches with incomplete transactions when stopping the gateway sender

2021-03-09 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/GEODE-8971?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17298048#comment-17298048
 ] 

ASF GitHub Bot commented on GEODE-8971:
---

albertogpz commented on a change in pull request #6052:
URL: https://github.com/apache/geode/pull/6052#discussion_r590322816



##
File path: 
geode-wan/src/distributedTest/java/org/apache/geode/internal/cache/wan/serial/SerialWANStatsDUnitTest.java
##
@@ -348,7 +349,276 @@ public void 
testReplicatedSerialPropagationWithBatchRedistWithGroupTransactionEv
   }
 
   @Test
-  public void testReplicatedSerialPropagationWithMultipleDispatchers() throws 
Exception {
+  public void 
testReplicatedSerialPropagationWithGroupTransactionEventsDoesNotSendBatchesWithIncompleteTransactionsIfGatewaySenderIsStoppedWhileReceivingTrafficAndLaterStarted()
+  throws InterruptedException {
+Integer lnPort = vm0.invoke(() -> 
WANTestBase.createFirstLocatorWithDSId(1));
+Integer nyPort = vm1.invoke(() -> WANTestBase.createFirstRemoteLocator(2, 
lnPort));
+
+String regionName = testName + "_RR";
+
+createCacheInVMs(nyPort, vm2);
+createReceiverInVMs(vm2);
+
+createCacheInVMs(lnPort, vm4, vm5);
+boolean groupTransactionEvents = true;
+int batchSize = 10;
+vm4.invoke(
+() -> WANTestBase.createSender("ln", 2, false, 100, batchSize, false, 
true, null, true,
+groupTransactionEvents));
+vm5.invoke(
+() -> WANTestBase.createSender("ln", 2, false, 100, batchSize, false, 
true, null, true,
+groupTransactionEvents));
+
+vm2.invoke(() -> WANTestBase.createReplicatedRegion(regionName, null, 
isOffHeap()));
+
+vm4.invoke(() -> WANTestBase.createReplicatedRegion(regionName, "ln", 
isOffHeap()));
+vm5.invoke(() -> WANTestBase.createReplicatedRegion(regionName, "ln", 
isOffHeap()));
+
+startSenderInVMs("ln", vm4, vm5);
+
+final Map keyValues = new LinkedHashMap<>();
+int entries = 2200;
+for (int i = 0; i < entries; i++) {
+  keyValues.put(i, i + "_Value");
+}
+
+int eventsPerTransaction = 11;
+System.out.println("Starting puts");
+AsyncInvocation inv1 =
+vm4.invokeAsync(
+() -> WANTestBase.doPutsInsideTransactions(regionName, keyValues,
+eventsPerTransaction));
+
+// wait for batches to be distributed and then stop the sender
+System.out.println("Waiting for some batches to be distributed");
+vm4.invoke(() -> await()
+.until(() -> WANTestBase.getSenderStats("ln", -1).get(4) > 0));
+System.out
+.println("Some batches distributed: " + vm4.invoke(() -> 
getSenderStats("ln", -1).get(4)));
+
+addIgnoredException("Exception occurred in CacheListener");

Review comment:
   ok





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)


[jira] [Commented] (GEODE-8971) Batches with incomplete transactions when stopping the gateway sender

2021-03-09 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/GEODE-8971?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17298043#comment-17298043
 ] 

ASF GitHub Bot commented on GEODE-8971:
---

albertogpz commented on a change in pull request #6052:
URL: https://github.com/apache/geode/pull/6052#discussion_r590311772



##
File path: 
geode-wan/src/distributedTest/java/org/apache/geode/internal/cache/wan/serial/SerialWANStatsDUnitTest.java
##
@@ -348,7 +349,276 @@ public void 
testReplicatedSerialPropagationWithBatchRedistWithGroupTransactionEv
   }
 
   @Test
-  public void testReplicatedSerialPropagationWithMultipleDispatchers() throws 
Exception {
+  public void 
testReplicatedSerialPropagationWithGroupTransactionEventsDoesNotSendBatchesWithIncompleteTransactionsIfGatewaySenderIsStoppedWhileReceivingTrafficAndLaterStarted()
+  throws InterruptedException {
+Integer lnPort = vm0.invoke(() -> 
WANTestBase.createFirstLocatorWithDSId(1));
+Integer nyPort = vm1.invoke(() -> WANTestBase.createFirstRemoteLocator(2, 
lnPort));
+
+String regionName = testName + "_RR";
+
+createCacheInVMs(nyPort, vm2);
+createReceiverInVMs(vm2);
+
+createCacheInVMs(lnPort, vm4, vm5);
+boolean groupTransactionEvents = true;
+int batchSize = 10;
+vm4.invoke(
+() -> WANTestBase.createSender("ln", 2, false, 100, batchSize, false, 
true, null, true,
+groupTransactionEvents));
+vm5.invoke(
+() -> WANTestBase.createSender("ln", 2, false, 100, batchSize, false, 
true, null, true,
+groupTransactionEvents));
+
+vm2.invoke(() -> WANTestBase.createReplicatedRegion(regionName, null, 
isOffHeap()));
+
+vm4.invoke(() -> WANTestBase.createReplicatedRegion(regionName, "ln", 
isOffHeap()));
+vm5.invoke(() -> WANTestBase.createReplicatedRegion(regionName, "ln", 
isOffHeap()));
+
+startSenderInVMs("ln", vm4, vm5);
+
+final Map keyValues = new LinkedHashMap<>();
+int entries = 2200;

Review comment:
   I picked a number divisible by 11 (as transactions are that size) and 
big enough so that not all entries are replicated by the gateway sender before 
it is stopped.





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)


[jira] [Commented] (GEODE-8971) Batches with incomplete transactions when stopping the gateway sender

2021-03-09 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/GEODE-8971?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17298042#comment-17298042
 ] 

ASF GitHub Bot commented on GEODE-8971:
---

albertogpz commented on a change in pull request #6052:
URL: https://github.com/apache/geode/pull/6052#discussion_r590311772



##
File path: 
geode-wan/src/distributedTest/java/org/apache/geode/internal/cache/wan/serial/SerialWANStatsDUnitTest.java
##
@@ -348,7 +349,276 @@ public void 
testReplicatedSerialPropagationWithBatchRedistWithGroupTransactionEv
   }
 
   @Test
-  public void testReplicatedSerialPropagationWithMultipleDispatchers() throws 
Exception {
+  public void 
testReplicatedSerialPropagationWithGroupTransactionEventsDoesNotSendBatchesWithIncompleteTransactionsIfGatewaySenderIsStoppedWhileReceivingTrafficAndLaterStarted()
+  throws InterruptedException {
+Integer lnPort = vm0.invoke(() -> 
WANTestBase.createFirstLocatorWithDSId(1));
+Integer nyPort = vm1.invoke(() -> WANTestBase.createFirstRemoteLocator(2, 
lnPort));
+
+String regionName = testName + "_RR";
+
+createCacheInVMs(nyPort, vm2);
+createReceiverInVMs(vm2);
+
+createCacheInVMs(lnPort, vm4, vm5);
+boolean groupTransactionEvents = true;
+int batchSize = 10;
+vm4.invoke(
+() -> WANTestBase.createSender("ln", 2, false, 100, batchSize, false, 
true, null, true,
+groupTransactionEvents));
+vm5.invoke(
+() -> WANTestBase.createSender("ln", 2, false, 100, batchSize, false, 
true, null, true,
+groupTransactionEvents));
+
+vm2.invoke(() -> WANTestBase.createReplicatedRegion(regionName, null, 
isOffHeap()));
+
+vm4.invoke(() -> WANTestBase.createReplicatedRegion(regionName, "ln", 
isOffHeap()));
+vm5.invoke(() -> WANTestBase.createReplicatedRegion(regionName, "ln", 
isOffHeap()));
+
+startSenderInVMs("ln", vm4, vm5);
+
+final Map keyValues = new LinkedHashMap<>();
+int entries = 2200;

Review comment:
   Not really. I picked a number divisible by 11 (as transactions are that 
size) and big enough so that not all entries are replicated by the gateway 
sender before it is stopped.





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)


[jira] [Commented] (GEODE-8971) Batches with incomplete transactions when stopping the gateway sender

2021-03-09 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/GEODE-8971?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17298039#comment-17298039
 ] 

ASF GitHub Bot commented on GEODE-8971:
---

albertogpz commented on a change in pull request #6052:
URL: https://github.com/apache/geode/pull/6052#discussion_r590311772



##
File path: 
geode-wan/src/distributedTest/java/org/apache/geode/internal/cache/wan/serial/SerialWANStatsDUnitTest.java
##
@@ -348,7 +349,276 @@ public void 
testReplicatedSerialPropagationWithBatchRedistWithGroupTransactionEv
   }
 
   @Test
-  public void testReplicatedSerialPropagationWithMultipleDispatchers() throws 
Exception {
+  public void 
testReplicatedSerialPropagationWithGroupTransactionEventsDoesNotSendBatchesWithIncompleteTransactionsIfGatewaySenderIsStoppedWhileReceivingTrafficAndLaterStarted()
+  throws InterruptedException {
+Integer lnPort = vm0.invoke(() -> 
WANTestBase.createFirstLocatorWithDSId(1));
+Integer nyPort = vm1.invoke(() -> WANTestBase.createFirstRemoteLocator(2, 
lnPort));
+
+String regionName = testName + "_RR";
+
+createCacheInVMs(nyPort, vm2);
+createReceiverInVMs(vm2);
+
+createCacheInVMs(lnPort, vm4, vm5);
+boolean groupTransactionEvents = true;
+int batchSize = 10;
+vm4.invoke(
+() -> WANTestBase.createSender("ln", 2, false, 100, batchSize, false, 
true, null, true,
+groupTransactionEvents));
+vm5.invoke(
+() -> WANTestBase.createSender("ln", 2, false, 100, batchSize, false, 
true, null, true,
+groupTransactionEvents));
+
+vm2.invoke(() -> WANTestBase.createReplicatedRegion(regionName, null, 
isOffHeap()));
+
+vm4.invoke(() -> WANTestBase.createReplicatedRegion(regionName, "ln", 
isOffHeap()));
+vm5.invoke(() -> WANTestBase.createReplicatedRegion(regionName, "ln", 
isOffHeap()));
+
+startSenderInVMs("ln", vm4, vm5);
+
+final Map keyValues = new LinkedHashMap<>();
+int entries = 2200;

Review comment:
   Not really. I picked a number divisible by 11 (as transactions are that 
size) and big enough to allow for some entries to be replicated by the gateway 
sender before it was stopped 





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)


[jira] [Commented] (GEODE-8918) Geode replication event forwarding does not honor GW sender state

2021-03-09 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/GEODE-8918?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17297980#comment-17297980
 ] 

ASF GitHub Bot commented on GEODE-8918:
---

mkevo commented on pull request #6008:
URL: https://github.com/apache/geode/pull/6008#issuecomment-793652235


   @nabarunnag , can you re-review it please?



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


> Geode replication event forwarding does not honor GW sender state
> -
>
> Key: GEODE-8918
> URL: https://issues.apache.org/jira/browse/GEODE-8918
> Project: Geode
>  Issue Type: Bug
>  Components: wan
>Reporter: Mario Kevo
>Assignee: Mario Kevo
>Priority: Major
>  Labels: pull-request-available
>
> {color:#172b4d}With 3+ geo-red systems Geode replication has the forwarding 
> feature which means that receiving cluster will forward the event it just got 
> to all clusters it is connected to that have not yet received the 
> event.{color}
> {color:#172b4d}This is possible because the originating cluster is setting 
> metadata in the replication event like this:
> GatewaySenderEventCallbackArgument 
> [originalCallbackArg=null;originatingSenderId=1;recipientGatewayReceivers= 
> {color}{color:#172b4d}{3, 2}{color}{color:#172b4d}]
>  
> Site receiving this event thus knows which is the originating site and which 
> sites should have received this event. All others will have this event 
> forwarded to. All this is legacy Geode behavior.
>  
> However, originating site does not care if GW sender to a destination is 
> stopped or not - only the fact GW sender is *created and attached* to a 
> region is enough. This means if e.g. GW sender from Site1 to Site 3 is 
> stopped (and has been stopped for a while - so this has nothing to do with 
> timing) at the moment an event hits the replication it is only going to be 
> sent to Site 2 *but with the same metadata*_._ Hence Site 2 will not forward 
> to Site 3 (assuming it has a connection to it).{color}



--
This message was sent by Atlassian Jira
(v8.3.4#803005)