[jira] [Commented] (GEODE-8846) Transaction commit fail when uniqueID integer overflow to negative value

[ASF GitHub Bot (Jira)]

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

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

jvarenina commented on a change in pull request #5927:
URL: https://github.com/apache/geode/pull/5927#discussion_r562461265



##
File path: 
geode-core/src/distributedTest/java/org/apache/geode/internal/cache/ClientServerTransactionDistributedTest.java
##
@@ -0,0 +1,152 @@
+/*
+ * 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;
+
+import static org.apache.geode.test.dunit.VM.getHostName;
+import static org.apache.geode.test.dunit.VM.getVM;
+import static org.junit.Assert.assertEquals;
+
+import java.io.Serializable;
+
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+
+import org.apache.geode.cache.PartitionAttributes;
+import org.apache.geode.cache.PartitionAttributesFactory;
+import org.apache.geode.cache.Region;
+import org.apache.geode.cache.RegionShortcut;
+import org.apache.geode.cache.client.ClientRegionFactory;
+import org.apache.geode.cache.client.ClientRegionShortcut;
+import org.apache.geode.cache.client.PoolFactory;
+import org.apache.geode.cache.client.PoolManager;
+import org.apache.geode.cache.client.internal.PoolImpl;
+import org.apache.geode.cache.server.CacheServer;
+import org.apache.geode.distributed.internal.ServerLocation;
+import org.apache.geode.logging.internal.log4j.api.LogService;
+import org.apache.geode.test.dunit.VM;
+import org.apache.geode.test.dunit.rules.CacheRule;
+import org.apache.geode.test.dunit.rules.ClientCacheRule;
+import org.apache.geode.test.dunit.rules.DistributedRule;
+import org.apache.geode.test.junit.rules.serializable.SerializableTestName;
+
+@SuppressWarnings("serial")
+public class ClientServerTransactionDistributedTest implements Serializable {
+
+  private String hostName;
+  private String uniqueName;
+  private String regionName;
+  private VM server1;
+  private int port1;
+
+  @Rule
+  public DistributedRule distributedRule = new DistributedRule();
+
+  @Rule
+  public CacheRule cacheRule = new CacheRule();
+
+  @Rule
+  public ClientCacheRule clientCacheRule = new ClientCacheRule();
+
+  @Rule
+  public SerializableTestName testName = new SerializableTestName();
+
+  @Before
+  public void setUp() {
+server1 = getVM(0);
+hostName = getHostName();
+uniqueName = getClass().getSimpleName() + "_" + testName.getMethodName();
+regionName = uniqueName + "_region";
+  }
+
+  @Test
+  public void clientTransactionIDAboveIntegerMaxValue() {
+port1 = server1.invoke(() -> createServerRegion());
+
+// Test that transaction ID overflow to one
+TXManagerImpl.INITIAL_UNIQUE_ID_VALUE = Integer.MAX_VALUE;
+createClientRegion(true, port1);
+
+TXManagerImpl txManager =
+(TXManagerImpl) 
clientCacheRule.getClientCache().getCacheTransactionManager();
+
+txManager.begin();
+TXStateProxyImpl txStateProxy = (TXStateProxyImpl) txManager.getTXState();
+int transactionID = ((TXId) txStateProxy.getTransactionId()).getUniqId();
+
+int numOfOperations = 5;
+putData(numOfOperations);
+txManager.commit();

Review comment:
   You are right, I have created additional test for the rollback case.





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


> Transaction commit fail when uniqueID integer overflow to negative value
> 
>
> Key: GEODE-8846
> URL: https://issues.apache.org/jira/browse/GEODE-8846
> Project: Geode
>  Issue Type: Bug
>  Components: client/server
>Affects Versions: 1.13.0, 1.13.1
>Reporter: Jakov Varenina
>Assignee: Jakov Varenina
>Priority: Major
>  Labels: pull-request-avail

[jira] [Commented] (GEODE-8846) Transaction commit fail when uniqueID integer overflow to negative value

[ASF GitHub Bot (Jira)]

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

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

jvarenina commented on a change in pull request #5927:
URL: https://github.com/apache/geode/pull/5927#discussion_r562461019



##
File path: 
geode-core/src/distributedTest/java/org/apache/geode/internal/cache/ClientServerTransactionDistributedTest.java
##
@@ -0,0 +1,152 @@
+/*
+ * 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;
+
+import static org.apache.geode.test.dunit.VM.getHostName;
+import static org.apache.geode.test.dunit.VM.getVM;
+import static org.junit.Assert.assertEquals;
+
+import java.io.Serializable;
+
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+
+import org.apache.geode.cache.PartitionAttributes;
+import org.apache.geode.cache.PartitionAttributesFactory;
+import org.apache.geode.cache.Region;
+import org.apache.geode.cache.RegionShortcut;
+import org.apache.geode.cache.client.ClientRegionFactory;
+import org.apache.geode.cache.client.ClientRegionShortcut;
+import org.apache.geode.cache.client.PoolFactory;
+import org.apache.geode.cache.client.PoolManager;
+import org.apache.geode.cache.client.internal.PoolImpl;
+import org.apache.geode.cache.server.CacheServer;
+import org.apache.geode.distributed.internal.ServerLocation;
+import org.apache.geode.logging.internal.log4j.api.LogService;
+import org.apache.geode.test.dunit.VM;
+import org.apache.geode.test.dunit.rules.CacheRule;
+import org.apache.geode.test.dunit.rules.ClientCacheRule;
+import org.apache.geode.test.dunit.rules.DistributedRule;
+import org.apache.geode.test.junit.rules.serializable.SerializableTestName;
+
+@SuppressWarnings("serial")
+public class ClientServerTransactionDistributedTest implements Serializable {
+
+  private String hostName;
+  private String uniqueName;
+  private String regionName;
+  private VM server1;
+  private int port1;

Review comment:
   Hi @DonalEvans , first thanks for your comments. Related to this 
comment, I haven't made the conversion, because I have created new TC for 
rollback where this variable is used. Hope this is 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


> Transaction commit fail when uniqueID integer overflow to negative value
> 
>
> Key: GEODE-8846
> URL: https://issues.apache.org/jira/browse/GEODE-8846
> Project: Geode
>  Issue Type: Bug
>  Components: client/server
>Affects Versions: 1.13.0, 1.13.1
>Reporter: Jakov Varenina
>Assignee: Jakov Varenina
>Priority: Major
>  Labels: pull-request-available
>
> When client increments *uniqId* above Integer.MAX_VALUE (2147483647) then due 
> to memory overflow the *uniqId* is set to negative value Integer.MIN_VALUE 
> (-2147483648). TransactionID (uniqId) is  sent to the server as a part of 
> transaction message.
> {code:java}
> public class TXManagerImpl implements CacheTransactionManager, 
> MembershipListener {
> ...
>   // The unique transaction ID for this Manager
>   private final AtomicInteger uniqId;
> 
> TXId id = new TXId(this.distributionMgrId, this.uniqId.incrementAndGet());
> 
> {code}
> Currently server will interpret any negative value of transactionID (uniqID) 
> as non-transactional traffic. Please notice that getTransactionID() actually 
> retrieves uniqID value that is set by client.
> {code:java}
> /**
>  * checks to see if this thread needs to masquerade as a transactional 
> thread. clients after
>  * GFE_66 should be able to start a transaction.
>  *
>  * @return true if thread should masquerade as a transactional thread.
>  */
> protected boolean shouldMasquerad

[jira] [Commented] (GEODE-8846) Transaction commit fail when uniqueID integer overflow to negative value

[ASF GitHub Bot (Jira)]

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

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

jvarenina commented on a change in pull request #5927:
URL: https://github.com/apache/geode/pull/5927#discussion_r562461340



##
File path: 
geode-core/src/distributedTest/java/org/apache/geode/internal/cache/ClientServerTransactionDistributedTest.java
##
@@ -0,0 +1,152 @@
+/*
+ * 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;
+
+import static org.apache.geode.test.dunit.VM.getHostName;
+import static org.apache.geode.test.dunit.VM.getVM;
+import static org.junit.Assert.assertEquals;
+
+import java.io.Serializable;
+
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+
+import org.apache.geode.cache.PartitionAttributes;
+import org.apache.geode.cache.PartitionAttributesFactory;
+import org.apache.geode.cache.Region;
+import org.apache.geode.cache.RegionShortcut;
+import org.apache.geode.cache.client.ClientRegionFactory;
+import org.apache.geode.cache.client.ClientRegionShortcut;
+import org.apache.geode.cache.client.PoolFactory;
+import org.apache.geode.cache.client.PoolManager;
+import org.apache.geode.cache.client.internal.PoolImpl;
+import org.apache.geode.cache.server.CacheServer;
+import org.apache.geode.distributed.internal.ServerLocation;
+import org.apache.geode.logging.internal.log4j.api.LogService;
+import org.apache.geode.test.dunit.VM;
+import org.apache.geode.test.dunit.rules.CacheRule;
+import org.apache.geode.test.dunit.rules.ClientCacheRule;
+import org.apache.geode.test.dunit.rules.DistributedRule;
+import org.apache.geode.test.junit.rules.serializable.SerializableTestName;
+
+@SuppressWarnings("serial")
+public class ClientServerTransactionDistributedTest implements Serializable {
+
+  private String hostName;
+  private String uniqueName;
+  private String regionName;
+  private VM server1;
+  private int port1;
+
+  @Rule
+  public DistributedRule distributedRule = new DistributedRule();
+
+  @Rule
+  public CacheRule cacheRule = new CacheRule();
+
+  @Rule
+  public ClientCacheRule clientCacheRule = new ClientCacheRule();
+
+  @Rule
+  public SerializableTestName testName = new SerializableTestName();
+
+  @Before
+  public void setUp() {
+server1 = getVM(0);
+hostName = getHostName();
+uniqueName = getClass().getSimpleName() + "_" + testName.getMethodName();
+regionName = uniqueName + "_region";
+  }
+
+  @Test
+  public void clientTransactionIDAboveIntegerMaxValue() {
+port1 = server1.invoke(() -> createServerRegion());

Review comment:
   done

##
File path: 
geode-core/src/distributedTest/java/org/apache/geode/internal/cache/ClientServerTransactionDistributedTest.java
##
@@ -0,0 +1,152 @@
+/*
+ * 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;
+
+import static org.apache.geode.test.dunit.VM.getHostName;
+import static org.apache.geode.test.dunit.VM.getVM;
+import static org.junit.Assert.assertEquals;
+
+import java.io.Serializable;
+
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+
+import org.apache.geode.cache.PartitionAttributes;
+import org.apache.geode.cache.PartitionAttributesFactory;
+import org.apache.geode.cache.Region;
+import org.apache.geode.cache.R

[jira] [Commented] (GEODE-8846) Transaction commit fail when uniqueID integer overflow to negative value

[ASF GitHub Bot (Jira)]

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

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

jvarenina commented on a change in pull request #5927:
URL: https://github.com/apache/geode/pull/5927#discussion_r562461776



##
File path: 
geode-core/src/distributedTest/java/org/apache/geode/internal/cache/ClientServerTransactionDistributedTest.java
##
@@ -0,0 +1,152 @@
+/*
+ * 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;
+
+import static org.apache.geode.test.dunit.VM.getHostName;
+import static org.apache.geode.test.dunit.VM.getVM;
+import static org.junit.Assert.assertEquals;
+
+import java.io.Serializable;
+
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+
+import org.apache.geode.cache.PartitionAttributes;
+import org.apache.geode.cache.PartitionAttributesFactory;
+import org.apache.geode.cache.Region;
+import org.apache.geode.cache.RegionShortcut;
+import org.apache.geode.cache.client.ClientRegionFactory;
+import org.apache.geode.cache.client.ClientRegionShortcut;
+import org.apache.geode.cache.client.PoolFactory;
+import org.apache.geode.cache.client.PoolManager;
+import org.apache.geode.cache.client.internal.PoolImpl;
+import org.apache.geode.cache.server.CacheServer;
+import org.apache.geode.distributed.internal.ServerLocation;
+import org.apache.geode.logging.internal.log4j.api.LogService;
+import org.apache.geode.test.dunit.VM;
+import org.apache.geode.test.dunit.rules.CacheRule;
+import org.apache.geode.test.dunit.rules.ClientCacheRule;
+import org.apache.geode.test.dunit.rules.DistributedRule;
+import org.apache.geode.test.junit.rules.serializable.SerializableTestName;
+
+@SuppressWarnings("serial")
+public class ClientServerTransactionDistributedTest implements Serializable {
+
+  private String hostName;
+  private String uniqueName;
+  private String regionName;
+  private VM server1;
+  private int port1;
+
+  @Rule
+  public DistributedRule distributedRule = new DistributedRule();
+
+  @Rule
+  public CacheRule cacheRule = new CacheRule();
+
+  @Rule
+  public ClientCacheRule clientCacheRule = new ClientCacheRule();
+
+  @Rule
+  public SerializableTestName testName = new SerializableTestName();
+
+  @Before
+  public void setUp() {
+server1 = getVM(0);
+hostName = getHostName();
+uniqueName = getClass().getSimpleName() + "_" + testName.getMethodName();
+regionName = uniqueName + "_region";
+  }
+
+  @Test
+  public void clientTransactionIDAboveIntegerMaxValue() {
+port1 = server1.invoke(() -> createServerRegion());
+
+// Test that transaction ID overflow to one
+TXManagerImpl.INITIAL_UNIQUE_ID_VALUE = Integer.MAX_VALUE;
+createClientRegion(true, port1);
+
+TXManagerImpl txManager =
+(TXManagerImpl) 
clientCacheRule.getClientCache().getCacheTransactionManager();
+
+txManager.begin();
+TXStateProxyImpl txStateProxy = (TXStateProxyImpl) txManager.getTXState();
+int transactionID = ((TXId) txStateProxy.getTransactionId()).getUniqId();
+
+int numOfOperations = 5;
+putData(numOfOperations);
+txManager.commit();
+
+server1.invoke(() -> verifyTransactionResult(numOfOperations));
+assertEquals(1, transactionID);
+  }
+
+  private void putData(int numberOfEntries) {
+Region region = clientCacheRule.getClientCache().getRegion(regionName);
+for (int key = 0; key < numberOfEntries; key++) {
+  String value = getValue(key);
+  region.put(key, value);
+}
+  }
+
+  private void verifyTransactionResult(int numberOfEntries) {
+Region region = cacheRule.getCache().getRegion(regionName);
+for (int i = 0; i < numberOfEntries; i++) {
+  LogService.getLogger().info("region get key {} value {} ", i, 
region.get(i));
+}
+for (int i = 0; i < numberOfEntries; i++) {
+  assertEquals("value" + i, region.get(i));
+}
+  }
+
+  private int createServerRegion() throws Exception {
+PartitionAttributesFactory factory = new PartitionAttributesFactory();
+PartitionAttributes partitionAttributes = factory.create(

[jira] [Commented] (GEODE-8846) Transaction commit fail when uniqueID integer overflow to negative value

[ASF GitHub Bot (Jira)]

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

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

jvarenina commented on a change in pull request #5927:
URL: https://github.com/apache/geode/pull/5927#discussion_r562461549



##
File path: 
geode-core/src/distributedTest/java/org/apache/geode/internal/cache/ClientServerTransactionDistributedTest.java
##
@@ -0,0 +1,152 @@
+/*
+ * 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;
+
+import static org.apache.geode.test.dunit.VM.getHostName;
+import static org.apache.geode.test.dunit.VM.getVM;
+import static org.junit.Assert.assertEquals;
+
+import java.io.Serializable;
+
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+
+import org.apache.geode.cache.PartitionAttributes;
+import org.apache.geode.cache.PartitionAttributesFactory;
+import org.apache.geode.cache.Region;
+import org.apache.geode.cache.RegionShortcut;
+import org.apache.geode.cache.client.ClientRegionFactory;
+import org.apache.geode.cache.client.ClientRegionShortcut;
+import org.apache.geode.cache.client.PoolFactory;
+import org.apache.geode.cache.client.PoolManager;
+import org.apache.geode.cache.client.internal.PoolImpl;
+import org.apache.geode.cache.server.CacheServer;
+import org.apache.geode.distributed.internal.ServerLocation;
+import org.apache.geode.logging.internal.log4j.api.LogService;
+import org.apache.geode.test.dunit.VM;
+import org.apache.geode.test.dunit.rules.CacheRule;
+import org.apache.geode.test.dunit.rules.ClientCacheRule;
+import org.apache.geode.test.dunit.rules.DistributedRule;
+import org.apache.geode.test.junit.rules.serializable.SerializableTestName;
+
+@SuppressWarnings("serial")
+public class ClientServerTransactionDistributedTest implements Serializable {
+
+  private String hostName;
+  private String uniqueName;
+  private String regionName;
+  private VM server1;
+  private int port1;
+
+  @Rule
+  public DistributedRule distributedRule = new DistributedRule();
+
+  @Rule
+  public CacheRule cacheRule = new CacheRule();
+
+  @Rule
+  public ClientCacheRule clientCacheRule = new ClientCacheRule();
+
+  @Rule
+  public SerializableTestName testName = new SerializableTestName();
+
+  @Before
+  public void setUp() {
+server1 = getVM(0);
+hostName = getHostName();
+uniqueName = getClass().getSimpleName() + "_" + testName.getMethodName();
+regionName = uniqueName + "_region";
+  }
+
+  @Test
+  public void clientTransactionIDAboveIntegerMaxValue() {
+port1 = server1.invoke(() -> createServerRegion());
+
+// Test that transaction ID overflow to one
+TXManagerImpl.INITIAL_UNIQUE_ID_VALUE = Integer.MAX_VALUE;
+createClientRegion(true, port1);
+
+TXManagerImpl txManager =
+(TXManagerImpl) 
clientCacheRule.getClientCache().getCacheTransactionManager();
+
+txManager.begin();
+TXStateProxyImpl txStateProxy = (TXStateProxyImpl) txManager.getTXState();
+int transactionID = ((TXId) txStateProxy.getTransactionId()).getUniqId();
+
+int numOfOperations = 5;
+putData(numOfOperations);
+txManager.commit();
+
+server1.invoke(() -> verifyTransactionResult(numOfOperations));
+assertEquals(1, transactionID);
+  }
+
+  private void putData(int numberOfEntries) {
+Region region = clientCacheRule.getClientCache().getRegion(regionName);
+for (int key = 0; key < numberOfEntries; key++) {
+  String value = getValue(key);
+  region.put(key, value);
+}
+  }
+
+  private void verifyTransactionResult(int numberOfEntries) {
+Region region = cacheRule.getCache().getRegion(regionName);
+for (int i = 0; i < numberOfEntries; i++) {
+  LogService.getLogger().info("region get key {} value {} ", i, 
region.get(i));
+}
+for (int i = 0; i < numberOfEntries; i++) {
+  assertEquals("value" + i, region.get(i));
+}
+  }
+
+  private int createServerRegion() throws Exception {
+PartitionAttributesFactory factory = new PartitionAttributesFactory();
+PartitionAttributes partitionAttributes = factory.create(

[jira] [Commented] (GEODE-8852) Add additional tests for Redis HVALS command

[ASF GitHub Bot (Jira)]

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

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

sabbey37 commented on a change in pull request #5931:
URL: https://github.com/apache/geode/pull/5931#discussion_r562668014



##
File path: 
geode-redis/src/distributedTest/java/org/apache/geode/redis/internal/executor/hash/HvalsDUnitTest.java
##
@@ -0,0 +1,95 @@
+/*
+ * 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.internal.executor.hash;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.util.List;
+import java.util.Random;
+
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.ClassRule;
+import org.junit.Test;
+import redis.clients.jedis.Jedis;
+
+import org.apache.geode.redis.ConcurrentLoopingThreads;
+import org.apache.geode.test.awaitility.GeodeAwaitility;
+import org.apache.geode.test.dunit.rules.MemberVM;
+import org.apache.geode.test.dunit.rules.RedisClusterStartupRule;
+
+public class HvalsDUnitTest {
+
+  @ClassRule
+  public static RedisClusterStartupRule clusterStartUp = new 
RedisClusterStartupRule(4);
+
+  private static final String LOCAL_HOST = "127.0.0.1";
+  private static final int JEDIS_TIMEOUT =
+  Math.toIntExact(GeodeAwaitility.getTimeout().toMillis());
+  private static Jedis jedis1;
+  private static Jedis jedis2;
+  private static Jedis jedis3;
+
+  @BeforeClass
+  public static void classSetup() {
+MemberVM locator = clusterStartUp.startLocatorVM(0);
+clusterStartUp.startRedisVM(1, locator.getPort());
+clusterStartUp.startRedisVM(2, locator.getPort());
+
+int redisServerPort1 = clusterStartUp.getRedisPort(1);
+int redisServerPort2 = clusterStartUp.getRedisPort(2);
+
+jedis1 = new Jedis(LOCAL_HOST, redisServerPort1, JEDIS_TIMEOUT);
+jedis2 = new Jedis(LOCAL_HOST, redisServerPort2, JEDIS_TIMEOUT);
+jedis3 = new Jedis(LOCAL_HOST, redisServerPort1, JEDIS_TIMEOUT);
+  }
+
+  @Before
+  public void testSetup() {
+jedis1.flushAll();
+  }
+
+  @Test
+  public void hvalsWorks_whileAlsoUpdatingHash() {
+String key = "key";
+int fieldCount = 100;
+int iterations = 1;
+Random rand = new Random();
+
+for (int i = 0; i < fieldCount; i++) {
+  jedis1.hset(key, "field-" + i, "" + i);
+}
+
+new ConcurrentLoopingThreads(iterations,
+(i) -> {
+  int x = rand.nextInt(fieldCount);
+  String field = "field-" + x;
+  String currentValue = jedis1.hget(key, field);
+  jedis1.hset(key, field, "" + (Long.parseLong(currentValue) + i));
+},
+(i) -> assertThat(jedis2.hvals(key)).hasSize(fieldCount),
+(i) -> assertThat(jedis3.hvals(key)).hasSize(fieldCount))
+.run();
+
+List values = jedis1.hvals(key);
+long finalTotal = values.stream().mapToLong(Long::valueOf).sum();
+
+long sumOfBothSequenceSums = (fieldCount / 2) * ((fieldCount - 1) - 0) +
+(iterations / 2) * ((iterations - 1) - 0);

Review comment:
   Yeah, definitely cool!  I guess it makes sense to leave it in that case, 
but change it to `+ 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


> Add additional tests for Redis HVALS command
> 
>
> Key: GEODE-8852
> URL: https://issues.apache.org/jira/browse/GEODE-8852
> Project: Geode
>  Issue Type: Test
>  Components: redis
>Reporter: Jens Deppe
>Priority: Major
>  Labels: pull-request-available
>
> Adding concurrency test as well as additional integration test



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

[jira] [Commented] (GEODE-8852) Add additional tests for Redis HVALS command

[ASF GitHub Bot (Jira)]

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

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

jdeppe-pivotal commented on a change in pull request #5931:
URL: https://github.com/apache/geode/pull/5931#discussion_r562681850



##
File path: 
geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/hash/AbstractHashesIntegrationTest.java
##
@@ -373,23 +373,32 @@ public void testHVals() {
 String key = "HVals_key";
 String field1 = "field_1";
 String field2 = "field_2";
-String value = "value";
+String value1 = "value_1";
+String value2 = "value_2";
 
-List list = jedis.hvals(key);
-assertThat(list == null || list.isEmpty()).isTrue();
+List list = jedis.hvals("non-existent-key");
+assertThat(list == null);
 
-Long result = jedis.hset(key, field1, value);
+Long result = jedis.hset(key, field1, value1);
 assertThat(result).isEqualTo(1);
 
-result = jedis.hset(key, field2, value);
+result = jedis.hset(key, field2, value2);
 assertThat(result).isEqualTo(1);
 list = jedis.hvals(key);
 
-assertThat(list).isNotNull();
-assertThat(list).isNotEmpty();
 assertThat(list).hasSize(2);
+assertThat(list).contains(value1, value2);
+  }
 
-assertThat(list).contains(value);
+  @Test
+  public void hvalsFailsForNonHash() {
+jedis.sadd("farm", "chicken");
+assertThatThrownBy(() -> jedis.hvals("farm"))
+.hasMessageContaining("WRONGTYPE");
+
+jedis.set("tractor", "John Deere");
+assertThatThrownBy(() -> jedis.hvals("tractor"))
+.hasMessageContaining("WRONGTYPE");

Review comment:
   Good call - done.





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 additional tests for Redis HVALS command
> 
>
> Key: GEODE-8852
> URL: https://issues.apache.org/jira/browse/GEODE-8852
> Project: Geode
>  Issue Type: Test
>  Components: redis
>Reporter: Jens Deppe
>Priority: Major
>  Labels: pull-request-available
>
> Adding concurrency test as well as additional integration test



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

[jira] [Commented] (GEODE-8852) Add additional tests for Redis HVALS command

[ASF GitHub Bot (Jira)]

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

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

jdeppe-pivotal commented on a change in pull request #5931:
URL: https://github.com/apache/geode/pull/5931#discussion_r562681975



##
File path: 
geode-redis/src/distributedTest/java/org/apache/geode/redis/internal/executor/hash/HvalsDUnitTest.java
##
@@ -0,0 +1,95 @@
+/*
+ * 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.internal.executor.hash;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.util.List;
+import java.util.Random;
+
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.ClassRule;
+import org.junit.Test;
+import redis.clients.jedis.Jedis;
+
+import org.apache.geode.redis.ConcurrentLoopingThreads;
+import org.apache.geode.test.awaitility.GeodeAwaitility;
+import org.apache.geode.test.dunit.rules.MemberVM;
+import org.apache.geode.test.dunit.rules.RedisClusterStartupRule;
+
+public class HvalsDUnitTest {
+
+  @ClassRule
+  public static RedisClusterStartupRule clusterStartUp = new 
RedisClusterStartupRule(4);
+
+  private static final String LOCAL_HOST = "127.0.0.1";
+  private static final int JEDIS_TIMEOUT =
+  Math.toIntExact(GeodeAwaitility.getTimeout().toMillis());
+  private static Jedis jedis1;
+  private static Jedis jedis2;
+  private static Jedis jedis3;
+
+  @BeforeClass
+  public static void classSetup() {
+MemberVM locator = clusterStartUp.startLocatorVM(0);
+clusterStartUp.startRedisVM(1, locator.getPort());
+clusterStartUp.startRedisVM(2, locator.getPort());
+
+int redisServerPort1 = clusterStartUp.getRedisPort(1);
+int redisServerPort2 = clusterStartUp.getRedisPort(2);
+
+jedis1 = new Jedis(LOCAL_HOST, redisServerPort1, JEDIS_TIMEOUT);
+jedis2 = new Jedis(LOCAL_HOST, redisServerPort2, JEDIS_TIMEOUT);
+jedis3 = new Jedis(LOCAL_HOST, redisServerPort1, JEDIS_TIMEOUT);
+  }
+
+  @Before
+  public void testSetup() {
+jedis1.flushAll();
+  }
+
+  @Test
+  public void hvalsWorks_whileAlsoUpdatingHash() {
+String key = "key";
+int fieldCount = 100;
+int iterations = 1;
+Random rand = new Random();
+
+for (int i = 0; i < fieldCount; i++) {
+  jedis1.hset(key, "field-" + i, "" + i);
+}
+
+new ConcurrentLoopingThreads(iterations,
+(i) -> {
+  int x = rand.nextInt(fieldCount);
+  String field = "field-" + x;
+  String currentValue = jedis1.hget(key, field);
+  jedis1.hset(key, field, "" + (Long.parseLong(currentValue) + i));
+},
+(i) -> assertThat(jedis2.hvals(key)).hasSize(fieldCount),
+(i) -> assertThat(jedis3.hvals(key)).hasSize(fieldCount))
+.run();
+
+List values = jedis1.hvals(key);
+long finalTotal = values.stream().mapToLong(Long::valueOf).sum();
+
+long sumOfBothSequenceSums = (fieldCount / 2) * ((fieldCount - 1) - 0) +
+(iterations / 2) * ((iterations - 1) - 0);

Review comment:
   Done





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 additional tests for Redis HVALS command
> 
>
> Key: GEODE-8852
> URL: https://issues.apache.org/jira/browse/GEODE-8852
> Project: Geode
>  Issue Type: Test
>  Components: redis
>Reporter: Jens Deppe
>Priority: Major
>  Labels: pull-request-available
>
> Adding concurrency test as well as additional integration test



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

[jira] [Commented] (GEODE-8852) Add additional tests for Redis HVALS command

[ASF GitHub Bot (Jira)]

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

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

jdeppe-pivotal commented on a change in pull request #5931:
URL: https://github.com/apache/geode/pull/5931#discussion_r562682489



##
File path: 
geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/hash/AbstractHashesIntegrationTest.java
##
@@ -373,23 +373,32 @@ public void testHVals() {
 String key = "HVals_key";
 String field1 = "field_1";
 String field2 = "field_2";
-String value = "value";
+String value1 = "value_1";
+String value2 = "value_2";
 
-List list = jedis.hvals(key);
-assertThat(list == null || list.isEmpty()).isTrue();
+List list = jedis.hvals("non-existent-key");
+assertThat(list == null);

Review comment:
   Ah - good catch. It didn't actually even occur to me that the 
`assertThat`, as it is here, does nothing :(. Now fixed.





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 additional tests for Redis HVALS command
> 
>
> Key: GEODE-8852
> URL: https://issues.apache.org/jira/browse/GEODE-8852
> Project: Geode
>  Issue Type: Test
>  Components: redis
>Reporter: Jens Deppe
>Priority: Major
>  Labels: pull-request-available
>
> Adding concurrency test as well as additional integration test



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

[jira] [Commented] (GEODE-8858) Unit/Integration tests for HSETNX command

[ASF GitHub Bot (Jira)]

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

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

sabbey37 commented on a change in pull request #5940:
URL: https://github.com/apache/geode/pull/5940#discussion_r562054830



##
File path: 
geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/hash/AbstractHashesIntegrationTest.java
##
@@ -365,6 +365,28 @@ public void testHSetNXExecutor() {
 
   }
 
+  @Test
+  public void hsetNX_shouldThrowErrorIfKeyIsWrongType() {
+String string_key = "String_Key";
+String set_key = "Set_Key";
+String field = "field";
+String value = "value";
+
+jedis.set(string_key, value);
+jedis.sadd(set_key, field);
+
+assertThatThrownBy(
+() -> jedis.hsetnx(string_key, field, "something else"))
+.isInstanceOf(JedisDataException.class)
+.hasMessageContaining("WRONGTYPE");

Review comment:
   It would be nice if we checked the full error message here.  We store 
the rest of it as a constant in `RedisConstants`, so it could be something like:
   ```
   .hasMessage("WRONGTYPE " + ERROR_WRONG_TYPE);
   ```
   
   I know it might seem like overkill, but I remember us having problems with 
other error messages not matching Redis's exactly.  There are other tests where 
we just check that the message contains `WRONGTYPE`, maybe we could change 
those as well.





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


> Unit/Integration tests for HSETNX command
> -
>
> Key: GEODE-8858
> URL: https://issues.apache.org/jira/browse/GEODE-8858
> Project: Geode
>  Issue Type: Test
>  Components: redis
>Reporter: Raymond Ingles
>Priority: Major
>  Labels: pull-request-available
>
> Flesh out unit/integration tests for Redis HSETNX command.



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

[jira] [Commented] (GEODE-8858) Unit/Integration tests for HSETNX command

[ASF GitHub Bot (Jira)]

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

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

sabbey37 commented on a change in pull request #5940:
URL: https://github.com/apache/geode/pull/5940#discussion_r562207431



##
File path: 
geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/hash/AbstractHashesIntegrationTest.java
##
@@ -365,6 +365,40 @@ public void testHSetNXExecutor() {
 
   }
 
+  @Test
+  public void hsetNX_shouldThrowErrorIfKeyIsWrongType() {
+String string_key = "String_Key";
+String set_key = "Set_Key";
+String field = "field";
+String value = "value";
+
+jedis.set(string_key, value);
+jedis.sadd(set_key, field);
+
+assertThatThrownBy(
+() -> jedis.hsetnx(string_key, field, "something else"))
+.isInstanceOf(JedisDataException.class)
+.hasMessageContaining("WRONGTYPE");
+assertThatThrownBy(
+() -> jedis.hsetnx(set_key, field, "something 
else")).isInstanceOf(JedisDataException.class)
+.hasMessageContaining("WRONGTYPE");
+
+jedis.del(string_key);
+jedis.del(set_key);
+  }
+
+  @Test
+  public void hsetnx_shouldThrowError_givenWrongNumberOfArguments() {
+assertThatThrownBy(() -> jedis.sendCommand(Protocol.Command.HSETNX))
+.hasMessageContaining("wrong number of arguments");
+assertThatThrownBy(() -> jedis.sendCommand(Protocol.Command.HSETNX, "1"))
+.hasMessageContaining("wrong number of arguments");
+assertThatThrownBy(() -> jedis.sendCommand(Protocol.Command.HSETNX, "1", 
"2"))
+.hasMessageContaining("wrong number of arguments");
+assertThatThrownBy(() -> jedis.sendCommand(Protocol.Command.HSETNX, "1", 
"2", "3", "4"))
+.hasMessageContaining("wrong number of arguments");
+  }

Review comment:
   This is great!  Thanks for adding these.  I know for most of the other 
commands (del, getset, exists, etc.) we check the error message more closely, 
like:
   
   ```
   .hasMessage("ERR wrong number of arguments for 'get' command");
   ```
   I'm not sure if it's necessary, but maybe we could do the same here to be 
consistent?  I realize there are a few areas where we're still just checking 
`wrong number of arguments`.  We could eventually change those in a different 
PR.





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


> Unit/Integration tests for HSETNX command
> -
>
> Key: GEODE-8858
> URL: https://issues.apache.org/jira/browse/GEODE-8858
> Project: Geode
>  Issue Type: Test
>  Components: redis
>Reporter: Raymond Ingles
>Priority: Major
>  Labels: pull-request-available
>
> Flesh out unit/integration tests for Redis HSETNX command.



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

[jira] [Commented] (GEODE-8865) Create additional dunit and integration tests for Redis HMGET

[ASF GitHub Bot (Jira)]

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

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

sabbey37 commented on a change in pull request #5945:
URL: https://github.com/apache/geode/pull/5945#discussion_r562700659



##
File path: 
geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/hash/AbstractHashesIntegrationTest.java
##
@@ -133,34 +149,54 @@ public void testHSet() {
   }
 
   @Test
-  public void testHMGetHDelHGetAllHVals() {
+  public void testHMGet_HDel_HGetAll_HVals() {
 String key = "key";
-Map hash = new HashMap();
+Map hash = new HashMap<>();
 for (int i = 0; i < 10; i++) {
   hash.put("field_" + i, "member_" + i);
 }
 jedis.hmset(key, hash);
+
 Set keys = hash.keySet();
 String[] keyArray = keys.toArray(new String[keys.size()]);
 List retList = jedis.hmget(key, keyArray);
 
-for (int i = 0; i < keys.size(); i++) {
-  assertThat(hash.get(keyArray[i])).isEqualTo(retList.get(i));
-}
+assertThat(retList).containsExactlyInAnyOrderElementsOf(hash.values());
 
 Map retMap = jedis.hgetAll(key);
 
 assertThat(retMap).containsExactlyInAnyOrderEntriesOf(hash);
 
 List retVals = jedis.hvals(key);
-Set retSet = new HashSet(retVals);
+Set retSet = new HashSet<>(retVals);
 
 assertThat(retSet.containsAll(hash.values())).isTrue();
 
 jedis.hdel(key, keyArray);
 assertThat(jedis.hlen(key)).isEqualTo(0);
   }
 
+  @Test
+  public void testHMGet_givenWrongNumberOfArguments() {
+assertThatThrownBy(() -> jedis.sendCommand(Protocol.Command.HMGET))
+.hasMessageContaining("wrong number of arguments");
+assertThatThrownBy(() -> jedis.sendCommand(Protocol.Command.HMGET, "1"))
+.hasMessageContaining("wrong number of arguments");
+  }
+
+  @Test
+  public void testHMGetErrorMessage_givenIncorrectDataType() {
+jedis.set("farm", "chicken");
+assertThatThrownBy(() -> jedis.hmget("farm", "chicken"))
+.isInstanceOf(JedisDataException.class)
+.hasMessageContaining("WRONGTYPE Operation against a key holding the 
wrong kind of value");
+
+jedis.sadd("zoo", "elephant");
+assertThatThrownBy(() -> jedis.hmget("zoo", "chicken"))
+.isInstanceOf(JedisDataException.class)
+.hasMessageContaining("WRONGTYPE Operation against a key holding the 
wrong kind of value");

Review comment:
   Same comment from earlier in the file: you could actually change this to 
.hasMessage instead of .hasMessageContaining in case anything ever gets 
appended to the beginning or end. We could also use the constant from 
RedisConstants here:
   
   .hasMessage("WRONGTYPE " + ERROR_WRONG_TYPE)
   so if the message ever changed for some reason it would be less work to 
update it throughout all the tests

##
File path: 
geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/hash/AbstractHashesIntegrationTest.java
##
@@ -111,6 +111,22 @@ public void testHMSet() {
 assertThat(jedis.hlen(key)).isEqualTo(hash.size());
   }
 
+  @Test
+  public void testHMSetErrorMessage_givenIncorrectDataType() {
+Map animalMap = new HashMap<>();
+animalMap.put("chicken", "eggs");
+
+jedis.set("farm", "chicken");
+assertThatThrownBy(() -> jedis.hmset("farm", animalMap))
+.isInstanceOf(JedisDataException.class)
+.hasMessageContaining("WRONGTYPE Operation against a key holding the 
wrong kind of value");
+
+jedis.sadd("zoo", "elephant");
+assertThatThrownBy(() -> jedis.hmset("zoo", animalMap))
+.isInstanceOf(JedisDataException.class)
+.hasMessageContaining("WRONGTYPE Operation against a key holding the 
wrong kind of value");

Review comment:
   Just realized you could actually change this to `.hasMessage` instead of 
`.hasMessageContaining` in case anything ever gets appended to the beginning or 
end.  We could also use the constant from `RedisConstants` here:
   ```
   .hasMessage("WRONGTYPE " + ERROR_WRONG_TYPE)
   ```
   
   so if the message ever changed for some reason it would be less work to 
update it throughout all the tests.

##
File path: 
geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/hash/AbstractHashesIntegrationTest.java
##
@@ -133,34 +149,54 @@ public void testHSet() {
   }
 
   @Test
-  public void testHMGetHDelHGetAllHVals() {
+  public void testHMGet_HDel_HGetAll_HVals() {
 String key = "key";
-Map hash = new HashMap();
+Map hash = new HashMap<>();
 for (int i = 0; i < 10; i++) {
   hash.put("field_" + i, "member_" + i);
 }
 jedis.hmset(key, hash);
+
 Set keys = hash.keySet();
 String[] keyArray = keys.toArray(new String[keys.size()]);
 List retList = jedis.hmget(key, keyArray);
 
-for (int i = 0; i < keys.size

[jira] [Commented] (GEODE-8852) Add additional tests for Redis HVALS command

[ASF GitHub Bot (Jira)]

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

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

sabbey37 commented on a change in pull request #5931:
URL: https://github.com/apache/geode/pull/5931#discussion_r562707462



##
File path: 
geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/hash/AbstractHashesIntegrationTest.java
##
@@ -373,23 +374,41 @@ public void testHVals() {
 String key = "HVals_key";
 String field1 = "field_1";
 String field2 = "field_2";
-String value = "value";
+String value1 = "value_1";
+String value2 = "value_2";
 
-List list = jedis.hvals(key);
-assertThat(list == null || list.isEmpty()).isTrue();
+List list = jedis.hvals("non-existent-key");
+assertThat(list).isEmpty();
 
-Long result = jedis.hset(key, field1, value);
+Long result = jedis.hset(key, field1, value1);
 assertThat(result).isEqualTo(1);
 
-result = jedis.hset(key, field2, value);
+result = jedis.hset(key, field2, value2);
 assertThat(result).isEqualTo(1);
 list = jedis.hvals(key);
 
-assertThat(list).isNotNull();
-assertThat(list).isNotEmpty();
 assertThat(list).hasSize(2);
+assertThat(list).contains(value1, value2);
+  }
 
-assertThat(list).contains(value);
+  @Test
+  public void hvalsFailsForNonHash() {
+jedis.sadd("farm", "chicken");
+assertThatThrownBy(() -> jedis.hvals("farm"))
+.hasMessage("WRONGTYPE " + RedisConstants.ERROR_WRONG_TYPE);
+
+jedis.set("tractor", "John Deere");
+assertThatThrownBy(() -> jedis.hvals("tractor"))
+.hasMessage("WRONGTYPE " + RedisConstants.ERROR_WRONG_TYPE);
+  }
+
+  @Test
+  public void hvalsFails_withIncorrectParameters() {
+assertThatThrownBy(() -> jedis.sendCommand(Protocol.Command.HVALS))
+.hasMessageContaining("wrong number of arguments");
+
+assertThatThrownBy(() -> jedis.sendCommand(Protocol.Command.HVALS, "1", 
"too-many"))
+.hasMessageContaining("wrong number of arguments");

Review comment:
   For most of the other commands (del, getset, exists, etc.) we check the 
error message more closely, like:
   ```
   .hasMessage("ERR wrong number of arguments for 'get' command");
   ```
   I'm not sure if it's necessary, but maybe we could do the same here to be 
consistent? I realize there are a few areas where we're still just checking 
wrong number of arguments. We could eventually change those in a different PR.





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 additional tests for Redis HVALS command
> 
>
> Key: GEODE-8852
> URL: https://issues.apache.org/jira/browse/GEODE-8852
> Project: Geode
>  Issue Type: Test
>  Components: redis
>Reporter: Jens Deppe
>Priority: Major
>  Labels: pull-request-available
>
> Adding concurrency test as well as additional integration test



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

[jira] [Commented] (GEODE-8852) Add additional tests for Redis HVALS command

[ASF GitHub Bot (Jira)]

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

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

jdeppe-pivotal commented on a change in pull request #5931:
URL: https://github.com/apache/geode/pull/5931#discussion_r562714941



##
File path: 
geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/hash/AbstractHashesIntegrationTest.java
##
@@ -373,23 +374,41 @@ public void testHVals() {
 String key = "HVals_key";
 String field1 = "field_1";
 String field2 = "field_2";
-String value = "value";
+String value1 = "value_1";
+String value2 = "value_2";
 
-List list = jedis.hvals(key);
-assertThat(list == null || list.isEmpty()).isTrue();
+List list = jedis.hvals("non-existent-key");
+assertThat(list).isEmpty();
 
-Long result = jedis.hset(key, field1, value);
+Long result = jedis.hset(key, field1, value1);
 assertThat(result).isEqualTo(1);
 
-result = jedis.hset(key, field2, value);
+result = jedis.hset(key, field2, value2);
 assertThat(result).isEqualTo(1);
 list = jedis.hvals(key);
 
-assertThat(list).isNotNull();
-assertThat(list).isNotEmpty();
 assertThat(list).hasSize(2);
+assertThat(list).contains(value1, value2);
+  }
 
-assertThat(list).contains(value);
+  @Test
+  public void hvalsFailsForNonHash() {
+jedis.sadd("farm", "chicken");
+assertThatThrownBy(() -> jedis.hvals("farm"))
+.hasMessage("WRONGTYPE " + RedisConstants.ERROR_WRONG_TYPE);
+
+jedis.set("tractor", "John Deere");
+assertThatThrownBy(() -> jedis.hvals("tractor"))
+.hasMessage("WRONGTYPE " + RedisConstants.ERROR_WRONG_TYPE);
+  }
+
+  @Test
+  public void hvalsFails_withIncorrectParameters() {
+assertThatThrownBy(() -> jedis.sendCommand(Protocol.Command.HVALS))
+.hasMessageContaining("wrong number of arguments");
+
+assertThatThrownBy(() -> jedis.sendCommand(Protocol.Command.HVALS, "1", 
"too-many"))
+.hasMessageContaining("wrong number of arguments");

Review comment:
   No problemo - done.





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 additional tests for Redis HVALS command
> 
>
> Key: GEODE-8852
> URL: https://issues.apache.org/jira/browse/GEODE-8852
> Project: Geode
>  Issue Type: Test
>  Components: redis
>Reporter: Jens Deppe
>Priority: Major
>  Labels: pull-request-available
>
> Adding concurrency test as well as additional integration test



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

[jira] [Commented] (GEODE-7017) CI failure: org.apache.geode.launchers.ServerStartupValueRecoveryNotificationTest > startupReportsOnlineOnlyAfterRedundancyRestored

[John Hutchison (Jira)]

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

John Hutchison commented on GEODE-7017:
---

seen in CI run :

org.apache.geode.launchers.ServerStartupValueRecoveryNotificationTest > 
startupReportsOnlineOnlyAfterRedundancyRestored FAILED
org.junit.ComparisonFailure: [Exit value from process started by 
[a3a70e5ecf4d838f: gfsh -e connect --locator=localhost[23137] -e start server 
--name=server1 --dir=/tmp/junit2110431257149045019/server1secondfolder 
--locators=localhost[23137]]] expected:<[0]> but was:<[1]>
at 
jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
at 
jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
at 
jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
at 
org.apache.geode.test.junit.rules.gfsh.GfshExecution.awaitTermination(GfshExecution.java:103)
at 
org.apache.geode.test.junit.rules.gfsh.GfshRule.execute(GfshRule.java:143)
at 
org.apache.geode.test.junit.rules.gfsh.GfshRule.execute(GfshRule.java:152)
at 
org.apache.geode.test.junit.rules.gfsh.GfshRule.execute(GfshRule.java:118)
at 
org.apache.geode.launchers.ServerStartupValueRecoveryNotificationTest.startupReportsOnlineOnlyAfterRedundancyRestored(ServerStartupValueRecoveryNotificationTest.java:146)

=-=-=-=-=-=-=-=-=-=-=-=-=-=-=  Test Results URI 
=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
http://files.apachegeode-ci.info/builds/apache-develop-main/1.14.0-build.0628/test-results/acceptanceTest/1611279518/
=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=

> CI failure: 
> org.apache.geode.launchers.ServerStartupValueRecoveryNotificationTest > 
> startupReportsOnlineOnlyAfterRedundancyRestored
> ---
>
> Key: GEODE-7017
> URL: https://issues.apache.org/jira/browse/GEODE-7017
> Project: Geode
>  Issue Type: Bug
>  Components: gfsh
>Affects Versions: 1.10.0
>Reporter: Anilkumar Gingade
>Assignee: Mark Hanson
>Priority: Major
> Attachments: acceptancetestfiles-OpenJDK11-1.14.0-build.0628.tgz
>
>
> {noformat}
> org.apache.geode.launchers.ServerStartupValueRecoveryNotificationTest > 
> startupReportsOnlineOnlyAfterRedundancyRestored FAILED
> org.junit.ComparisonFailure: expected:<[0]> but was:<[1]>
> at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native 
> Method)
> at 
> sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
> at 
> sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
> at 
> org.apache.geode.test.junit.rules.gfsh.GfshExecution.awaitTermination(GfshExecution.java:125)
> at 
> org.apache.geode.test.junit.rules.gfsh.GfshRule.execute(GfshRule.java:125)
> at 
> org.apache.geode.test.junit.rules.gfsh.GfshRule.execute(GfshRule.java:112)
> at 
> org.apache.geode.launchers.ServerStartupValueRecoveryNotificationTest.persistentRegionThatRequiresValueRecovery(ServerStartupValueRecoveryNotificationTest.java:120)
> {noformat}
> https://concourse.gemfire-ci.info/teams/main/pipelines/gemfire-develop-main/jobs/AcceptanceTestOpenJDK8/builds/797
> Test report artifacts from this job are available at:
> gs://gemfire-test-artifacts/builds/gemfire-develop-main/9.9.0-build.0258/test-artifacts/1564078711/acceptancetestfiles-OpenJDK8-9.9.0-build.0258.tgz



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

[jira] [Commented] (GEODE-8863) Redis: test hsetnx concurrency

[ASF GitHub Bot (Jira)]

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

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

sabbey37 commented on a change in pull request #5942:
URL: https://github.com/apache/geode/pull/5942#discussion_r562728647



##
File path: 
geode-redis/src/distributedTest/java/org/apache/geode/redis/internal/executor/hash/HSetNXDunitTest.java
##
@@ -0,0 +1,121 @@
+/*
+ * 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.internal.executor.hash;
+
+import static org.apache.geode.distributed.ConfigurationProperties.REDIS_PORT;
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.Future;
+
+import io.lettuce.core.ClientOptions;
+import io.lettuce.core.RedisClient;
+import io.lettuce.core.api.StatefulRedisConnection;
+import io.lettuce.core.api.sync.RedisCommands;
+import io.lettuce.core.resource.ClientResources;
+import org.junit.AfterClass;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.ClassRule;
+import org.junit.Test;
+
+import org.apache.geode.internal.AvailablePortHelper;
+import 
org.apache.geode.redis.session.springRedisTestApplication.config.DUnitSocketAddressResolver;
+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 HSetNXDunitTest {
+  @ClassRule
+  public static RedisClusterStartupRule cluster = new 
RedisClusterStartupRule();
+
+  @ClassRule
+  public static ExecutorServiceRule executor = new ExecutorServiceRule();
+
+  private static final int HASH_SIZE = 5;
+  private static MemberVM locator;
+  private static MemberVM server1;
+  private static MemberVM server2;
+  private static int[] redisPorts;
+  private static RedisCommands lettuce;
+  private static StatefulRedisConnection connection;
+  private static ClientResources resources;
+
+  @BeforeClass
+  public static void classSetup() {
+redisPorts = AvailablePortHelper.getRandomAvailableTCPPorts(3);
+
+String redisPort1 = "" + redisPorts[0];
+String redisPort2 = "" + redisPorts[1];
+
+locator = cluster.startLocatorVM(0);
+
+server1 = startRedisVM(1, redisPorts[0]);
+server2 = startRedisVM(2, redisPorts[1]);
+
+DUnitSocketAddressResolver dnsResolver =
+new DUnitSocketAddressResolver(new String[] {redisPort2, redisPort1});
+
+resources = ClientResources.builder()
+.socketAddressResolver(dnsResolver)
+.build();
+
+RedisClient redisClient = RedisClient.create(resources, 
"redis://localhost");
+redisClient.setOptions(ClientOptions.builder()
+.autoReconnect(true)
+.build());
+connection = redisClient.connect();
+lettuce = connection.sync();
+  }
+
+  @Before
+  public void testSetup() {
+lettuce.flushall();
+  }
+
+  @AfterClass
+  public static void tearDown() throws Exception {
+resources.shutdown().get();
+connection.close();
+server1.stop();
+server2.stop();
+  }
+
+  @Test
+  public void testHSETNXReturnsOneWhenKeyDoesNotExistAndZeroWhenItDoes()
+  throws ExecutionException, InterruptedException {
+String key = "HSETNX";
+
+for (int i = 0; i < 1000; i++) {
+  int local_i = i;
+
+  Future server_1_counter = executor.submit(
+  () -> lettuce.hsetnx(key, "field" + local_i, "value" + local_i));
+  Future server_2_counter = executor.submit(
+  () -> lettuce.hsetnx(key, "field" + local_i, "value" + local_i));
+
+  assertThat(server_1_counter.get() ^ server_2_counter.get()).isTrue();
+}
+  }

Review comment:
   This is really cool!





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


> Redis: test hsetnx concurrency 
> -

[jira] [Updated] (GEODE-7017) CI failure: org.apache.geode.launchers.ServerStartupValueRecoveryNotificationTest > startupReportsOnlineOnlyAfterRedundancyRestored

[John Hutchison (Jira)]

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

John Hutchison updated GEODE-7017:
--
Attachment: acceptancetestfiles-OpenJDK11-1.14.0-build.0628.tgz

> CI failure: 
> org.apache.geode.launchers.ServerStartupValueRecoveryNotificationTest > 
> startupReportsOnlineOnlyAfterRedundancyRestored
> ---
>
> Key: GEODE-7017
> URL: https://issues.apache.org/jira/browse/GEODE-7017
> Project: Geode
>  Issue Type: Bug
>  Components: gfsh
>Affects Versions: 1.10.0
>Reporter: Anilkumar Gingade
>Assignee: Mark Hanson
>Priority: Major
> Attachments: acceptancetestfiles-OpenJDK11-1.14.0-build.0628.tgz
>
>
> {noformat}
> org.apache.geode.launchers.ServerStartupValueRecoveryNotificationTest > 
> startupReportsOnlineOnlyAfterRedundancyRestored FAILED
> org.junit.ComparisonFailure: expected:<[0]> but was:<[1]>
> at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native 
> Method)
> at 
> sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
> at 
> sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
> at 
> org.apache.geode.test.junit.rules.gfsh.GfshExecution.awaitTermination(GfshExecution.java:125)
> at 
> org.apache.geode.test.junit.rules.gfsh.GfshRule.execute(GfshRule.java:125)
> at 
> org.apache.geode.test.junit.rules.gfsh.GfshRule.execute(GfshRule.java:112)
> at 
> org.apache.geode.launchers.ServerStartupValueRecoveryNotificationTest.persistentRegionThatRequiresValueRecovery(ServerStartupValueRecoveryNotificationTest.java:120)
> {noformat}
> https://concourse.gemfire-ci.info/teams/main/pipelines/gemfire-develop-main/jobs/AcceptanceTestOpenJDK8/builds/797
> Test report artifacts from this job are available at:
> gs://gemfire-test-artifacts/builds/gemfire-develop-main/9.9.0-build.0258/test-artifacts/1564078711/acceptancetestfiles-OpenJDK8-9.9.0-build.0258.tgz



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

[jira] [Commented] (GEODE-7017) CI failure: org.apache.geode.launchers.ServerStartupValueRecoveryNotificationTest > startupReportsOnlineOnlyAfterRedundancyRestored

[Geode Integration (Jira)]

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

Geode Integration commented on GEODE-7017:
--

Seen in [AcceptanceTestOpenJDK11 
#703|https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-develop-main/jobs/AcceptanceTestOpenJDK11/builds/703]
 ... see [test 
results|http://files.apachegeode-ci.info/builds/apache-develop-main/1.14.0-build.0628/test-results/acceptanceTest/1611279518/]
 or download 
[artifacts|http://files.apachegeode-ci.info/builds/apache-develop-main/1.14.0-build.0628/test-artifacts/1611279518/acceptancetestfiles-OpenJDK11-1.14.0-build.0628.tgz].

> CI failure: 
> org.apache.geode.launchers.ServerStartupValueRecoveryNotificationTest > 
> startupReportsOnlineOnlyAfterRedundancyRestored
> ---
>
> Key: GEODE-7017
> URL: https://issues.apache.org/jira/browse/GEODE-7017
> Project: Geode
>  Issue Type: Bug
>  Components: gfsh
>Affects Versions: 1.10.0
>Reporter: Anilkumar Gingade
>Assignee: Mark Hanson
>Priority: Major
> Attachments: acceptancetestfiles-OpenJDK11-1.14.0-build.0628.tgz
>
>
> {noformat}
> org.apache.geode.launchers.ServerStartupValueRecoveryNotificationTest > 
> startupReportsOnlineOnlyAfterRedundancyRestored FAILED
> org.junit.ComparisonFailure: expected:<[0]> but was:<[1]>
> at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native 
> Method)
> at 
> sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
> at 
> sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
> at 
> org.apache.geode.test.junit.rules.gfsh.GfshExecution.awaitTermination(GfshExecution.java:125)
> at 
> org.apache.geode.test.junit.rules.gfsh.GfshRule.execute(GfshRule.java:125)
> at 
> org.apache.geode.test.junit.rules.gfsh.GfshRule.execute(GfshRule.java:112)
> at 
> org.apache.geode.launchers.ServerStartupValueRecoveryNotificationTest.persistentRegionThatRequiresValueRecovery(ServerStartupValueRecoveryNotificationTest.java:120)
> {noformat}
> https://concourse.gemfire-ci.info/teams/main/pipelines/gemfire-develop-main/jobs/AcceptanceTestOpenJDK8/builds/797
> Test report artifacts from this job are available at:
> gs://gemfire-test-artifacts/builds/gemfire-develop-main/9.9.0-build.0258/test-artifacts/1564078711/acceptancetestfiles-OpenJDK8-9.9.0-build.0258.tgz



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

[jira] [Commented] (GEODE-8865) Create additional dunit and integration tests for Redis HMGET

[ASF GitHub Bot (Jira)]

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

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

jdeppe-pivotal commented on a change in pull request #5945:
URL: https://github.com/apache/geode/pull/5945#discussion_r562730365



##
File path: 
geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/hash/AbstractHashesIntegrationTest.java
##
@@ -111,6 +111,22 @@ public void testHMSet() {
 assertThat(jedis.hlen(key)).isEqualTo(hash.size());
   }
 
+  @Test
+  public void testHMSetErrorMessage_givenIncorrectDataType() {
+Map animalMap = new HashMap<>();
+animalMap.put("chicken", "eggs");
+
+jedis.set("farm", "chicken");
+assertThatThrownBy(() -> jedis.hmset("farm", animalMap))
+.isInstanceOf(JedisDataException.class)
+.hasMessageContaining("WRONGTYPE Operation against a key holding the 
wrong kind of value");
+
+jedis.sadd("zoo", "elephant");
+assertThatThrownBy(() -> jedis.hmset("zoo", animalMap))
+.isInstanceOf(JedisDataException.class)
+.hasMessageContaining("WRONGTYPE Operation against a key holding the 
wrong kind of value");

Review comment:
   I think I just did this in the HVALS PR





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


> Create additional dunit and integration tests for Redis HMGET
> -
>
> Key: GEODE-8865
> URL: https://issues.apache.org/jira/browse/GEODE-8865
> Project: Geode
>  Issue Type: Test
>  Components: redis
>Reporter: Jens Deppe
>Priority: Major
>  Labels: pull-request-available
>
> Write integration tests for the following command:
>  * HMGET
> *A.C.*
>  * NativeRedisAcceptanceTest file present to run our tests against native 
> Redis
>  * Tests are passing, _or_
>  * Stories in the backlog to fix the identified issues (with JIRA tickets) 
> and problem tests ignored



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

[jira] [Commented] (GEODE-8846) Transaction commit fail when uniqueID integer overflow to negative value

[ASF GitHub Bot (Jira)]

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

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

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



##
File path: 
geode-core/src/distributedTest/java/org/apache/geode/internal/cache/ClientServerTransactionDistributedTest.java
##
@@ -0,0 +1,152 @@
+/*
+ * 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;
+
+import static org.apache.geode.test.dunit.VM.getHostName;
+import static org.apache.geode.test.dunit.VM.getVM;
+import static org.junit.Assert.assertEquals;
+
+import java.io.Serializable;
+
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+
+import org.apache.geode.cache.PartitionAttributes;
+import org.apache.geode.cache.PartitionAttributesFactory;
+import org.apache.geode.cache.Region;
+import org.apache.geode.cache.RegionShortcut;
+import org.apache.geode.cache.client.ClientRegionFactory;
+import org.apache.geode.cache.client.ClientRegionShortcut;
+import org.apache.geode.cache.client.PoolFactory;
+import org.apache.geode.cache.client.PoolManager;
+import org.apache.geode.cache.client.internal.PoolImpl;
+import org.apache.geode.cache.server.CacheServer;
+import org.apache.geode.distributed.internal.ServerLocation;
+import org.apache.geode.logging.internal.log4j.api.LogService;
+import org.apache.geode.test.dunit.VM;
+import org.apache.geode.test.dunit.rules.CacheRule;
+import org.apache.geode.test.dunit.rules.ClientCacheRule;
+import org.apache.geode.test.dunit.rules.DistributedRule;
+import org.apache.geode.test.junit.rules.serializable.SerializableTestName;
+
+@SuppressWarnings("serial")
+public class ClientServerTransactionDistributedTest implements Serializable {
+
+  private String hostName;
+  private String uniqueName;
+  private String regionName;
+  private VM server1;
+  private int port1;

Review comment:
   Yeah, this is fine, thanks for addressing the feedback!





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


> Transaction commit fail when uniqueID integer overflow to negative value
> 
>
> Key: GEODE-8846
> URL: https://issues.apache.org/jira/browse/GEODE-8846
> Project: Geode
>  Issue Type: Bug
>  Components: client/server
>Affects Versions: 1.13.0, 1.13.1
>Reporter: Jakov Varenina
>Assignee: Jakov Varenina
>Priority: Major
>  Labels: pull-request-available
>
> When client increments *uniqId* above Integer.MAX_VALUE (2147483647) then due 
> to memory overflow the *uniqId* is set to negative value Integer.MIN_VALUE 
> (-2147483648). TransactionID (uniqId) is  sent to the server as a part of 
> transaction message.
> {code:java}
> public class TXManagerImpl implements CacheTransactionManager, 
> MembershipListener {
> ...
>   // The unique transaction ID for this Manager
>   private final AtomicInteger uniqId;
> 
> TXId id = new TXId(this.distributionMgrId, this.uniqId.incrementAndGet());
> 
> {code}
> Currently server will interpret any negative value of transactionID (uniqID) 
> as non-transactional traffic. Please notice that getTransactionID() actually 
> retrieves uniqID value that is set by client.
> {code:java}
> /**
>  * checks to see if this thread needs to masquerade as a transactional 
> thread. clients after
>  * GFE_66 should be able to start a transaction.
>  *
>  * @return true if thread should masquerade as a transactional thread.
>  */
> protected boolean shouldMasqueradeForTx(Message clientMessage,
> ServerConnection serverConnection) {
>   return 
> serverConnection.getClientVersion().isNotOlderThan(K

[jira] [Commented] (GEODE-8837) Establish GFE_81 as the oldest supported client version.

[ASF GitHub Bot (Jira)]

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

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

Bill commented on a change in pull request #5905:
URL: https://github.com/apache/geode/pull/5905#discussion_r562790958



##
File path: 
geode-serialization/src/main/java/org/apache/geode/internal/serialization/KnownVersion.java
##
@@ -367,61 +377,48 @@ public static String unsupportedVersionMessage(final 
short ordinal) {
   }
 
   public String getMethodSuffix() {
-return this.methodSuffix;
+return methodSuffix;
   }
 
   public String getName() {
-return this.name;
+return name;
   }
 
-  public short getMajorVersion() {
-return this.majorVersion;
+  public short getMajor() {
+return major;
   }
 
-  public short getMinorVersion() {
-return this.minorVersion;
+  public short getMinor() {
+return minor;
   }
 
   public short getRelease() {
-return this.release;
+return release;
   }
 
   public short getPatch() {
-return this.patch;
+return patch;
   }
 
-  /**
-   * Returns a string representation for this Version.
-   *
-   * @return the name of this operation.
-   */
   @Override
   public String toString() {
-return this.productName + " " + this.name;
-  }
-
-  public byte[] toBytes() {
-byte[] bytes = new byte[2];
-bytes[0] = (byte) (ordinal() >> 8);
-bytes[1] = (byte) ordinal();
-return bytes;
+return productName + " " + name;
   }
 
   public static Iterable getAllVersions() {
-return Arrays.asList(VALUES).stream().filter(x -> x != null && x != 
TEST_VERSION)
+return Arrays.stream(VALUES).filter(x -> x != null && x != TEST_VERSION)
 .collect(Collectors.toList());
   }
 
   /**
* package-protected for use by Versioning factory
*/
-  static KnownVersion getKnownVersionOrDefault(final short ordinal,
-  final KnownVersion defaultKnownVersion) {
+  static KnownVersion getKnownVersionOrDefault(final short ordinal) {

Review comment:
   I'm very glad you didn't remove it. Indeed it is a key part of the 
abstraction barrier.





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


> Establish GFE_81 as the oldest supported client version.
> 
>
> Key: GEODE-8837
> URL: https://issues.apache.org/jira/browse/GEODE-8837
> Project: Geode
>  Issue Type: Improvement
>  Components: client/server, core, cq, functions
>Reporter: Jacob Barrett
>Priority: Major
>  Labels: pull-request-available
>
> Remove support for versions older than GFE_81 in backwards compatibility 
> checks and command tables.
> Remove the unused and unreleased GFE_82 ordinal for consistency.



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

[jira] [Commented] (GEODE-8772) Avoid using default ports in tests [PERMANENT]

[ASF GitHub Bot (Jira)]

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

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

demery-pivotal merged pull request #5935:
URL: https://github.com/apache/geode/pull/5935


   



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


> Avoid using default ports in tests [PERMANENT]
> --
>
> Key: GEODE-8772
> URL: https://issues.apache.org/jira/browse/GEODE-8772
> Project: Geode
>  Issue Type: Test
>  Components: tests
>Affects Versions: 1.14.0
>Reporter: Dale Emery
>Assignee: Dale Emery
>Priority: Major
>  Labels: GeodeOperationAPI, pull-request-available
>
> Many distributed tests and upgrade tests (and sometimes others) launch 
> members with default ports, especially for JMX (1099) and HTTP service 
> (7070). When run in parallel outside of docker, these tests often fail 
> because the default port is already in use in another test.
> Except when specifically testing the product's use of the defaults, every 
> test should assign ports known to be available. For many tests, we can 
> accomplish this by changing the test framework to assign available ports. 
> Other tests may require changes in the test code.



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

[jira] [Commented] (GEODE-8837) Establish GFE_81 as the oldest supported client version.

[ASF GitHub Bot (Jira)]

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

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

Bill commented on a change in pull request #5905:
URL: https://github.com/apache/geode/pull/5905#discussion_r562792290



##
File path: 
geode-core/src/distributedTest/java/org/apache/geode/internal/BackwardCompatibilitySerializationDUnitTest.java
##
@@ -160,7 +149,7 @@ public void testFromDataFromLowerVersionToHigher() throws 
Exception {
   public void testAllMessages() throws Exception {
 // list of msgs not created using reflection
 // taken from DSFIDFactory.create()
-ArrayList constdsfids = new ArrayList();
+ArrayList constdsfids = new ArrayList<>();

Review comment:
   My comment was based on a quick read of the comment in the test method. 
That sent me over to `DSFIDFactory.create` where I found more cases than were 
accounted for in the test. Made me think those were added after the test and 
that we forgot to add them to the test.





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


> Establish GFE_81 as the oldest supported client version.
> 
>
> Key: GEODE-8837
> URL: https://issues.apache.org/jira/browse/GEODE-8837
> Project: Geode
>  Issue Type: Improvement
>  Components: client/server, core, cq, functions
>Reporter: Jacob Barrett
>Priority: Major
>  Labels: pull-request-available
>
> Remove support for versions older than GFE_81 in backwards compatibility 
> checks and command tables.
> Remove the unused and unreleased GFE_82 ordinal for consistency.



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

[jira] [Commented] (GEODE-8772) Avoid using default ports in tests [PERMANENT]

[ASF subversion and git services (Jira)]

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

ASF subversion and git services commented on GEODE-8772:


Commit 964051b7029d73ee4e6bed96f30e0fd3ee23bb79 in geode's branch 
refs/heads/develop from Dale Emery
[ https://gitbox.apache.org/repos/asf?p=geode.git;h=964051b ]

GEODE-8772: Make tests assign ports (#5935)

* GEODE-8772: Make tests assign ports

Some tests in ManagementRequestLoggingDistributedTest and
RestAPIsAndInterOpsDUnitTest were using the default JMX port (1099).
Some tests in TombstoneDUnitTest were using the default cache server
port (40404). When these tests run in parallel outside of docker, they
can fail to bind these ports if another test has already bound them.

These tests now assign ports via AvailablePortHelper.

Authored-by: Dale Emery 

* Make LocatorTestBase assign JMX port

* Restore use of jmx port "0" in REST API tests

Fixing an inadvertent change of a non-default port.

* Remove unused code

> Avoid using default ports in tests [PERMANENT]
> --
>
> Key: GEODE-8772
> URL: https://issues.apache.org/jira/browse/GEODE-8772
> Project: Geode
>  Issue Type: Test
>  Components: tests
>Affects Versions: 1.14.0
>Reporter: Dale Emery
>Assignee: Dale Emery
>Priority: Major
>  Labels: GeodeOperationAPI, pull-request-available
>
> Many distributed tests and upgrade tests (and sometimes others) launch 
> members with default ports, especially for JMX (1099) and HTTP service 
> (7070). When run in parallel outside of docker, these tests often fail 
> because the default port is already in use in another test.
> Except when specifically testing the product's use of the defaults, every 
> test should assign ports known to be available. For many tests, we can 
> accomplish this by changing the test framework to assign available ports. 
> Other tests may require changes in the test code.



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

[jira] [Commented] (GEODE-8772) Avoid using default ports in tests [PERMANENT]

[ASF subversion and git services (Jira)]

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

ASF subversion and git services commented on GEODE-8772:


Commit 964051b7029d73ee4e6bed96f30e0fd3ee23bb79 in geode's branch 
refs/heads/develop from Dale Emery
[ https://gitbox.apache.org/repos/asf?p=geode.git;h=964051b ]

GEODE-8772: Make tests assign ports (#5935)

* GEODE-8772: Make tests assign ports

Some tests in ManagementRequestLoggingDistributedTest and
RestAPIsAndInterOpsDUnitTest were using the default JMX port (1099).
Some tests in TombstoneDUnitTest were using the default cache server
port (40404). When these tests run in parallel outside of docker, they
can fail to bind these ports if another test has already bound them.

These tests now assign ports via AvailablePortHelper.

Authored-by: Dale Emery 

* Make LocatorTestBase assign JMX port

* Restore use of jmx port "0" in REST API tests

Fixing an inadvertent change of a non-default port.

* Remove unused code

> Avoid using default ports in tests [PERMANENT]
> --
>
> Key: GEODE-8772
> URL: https://issues.apache.org/jira/browse/GEODE-8772
> Project: Geode
>  Issue Type: Test
>  Components: tests
>Affects Versions: 1.14.0
>Reporter: Dale Emery
>Assignee: Dale Emery
>Priority: Major
>  Labels: GeodeOperationAPI, pull-request-available
>
> Many distributed tests and upgrade tests (and sometimes others) launch 
> members with default ports, especially for JMX (1099) and HTTP service 
> (7070). When run in parallel outside of docker, these tests often fail 
> because the default port is already in use in another test.
> Except when specifically testing the product's use of the defaults, every 
> test should assign ports known to be available. For many tests, we can 
> accomplish this by changing the test framework to assign available ports. 
> Other tests may require changes in the test code.



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

[jira] [Commented] (GEODE-8837) Establish GFE_81 as the oldest supported client version.

[ASF GitHub Bot (Jira)]

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

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

Bill commented on a change in pull request #5905:
URL: https://github.com/apache/geode/pull/5905#discussion_r562792988



##
File path: 
geode-core/src/main/java/org/apache/geode/cache/client/internal/ClientSideHandshakeImpl.java
##
@@ -233,7 +230,7 @@ public ServerQueueStatus handshakeWithServer(Connection 
conn, ServerLocation loc
 
   member = readServerMember(dis);
 
-  serverQStatus = new ServerQueueStatus(endpointType, queueSize, member);
+  ServerQueueStatus serverQStatus = new ServerQueueStatus(endpointType, 
queueSize, member);

Review comment:
   I have to make at least one `final` comment on every PR. Also this one 
just jumped out at me.





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


> Establish GFE_81 as the oldest supported client version.
> 
>
> Key: GEODE-8837
> URL: https://issues.apache.org/jira/browse/GEODE-8837
> Project: Geode
>  Issue Type: Improvement
>  Components: client/server, core, cq, functions
>Reporter: Jacob Barrett
>Priority: Major
>  Labels: pull-request-available
>
> Remove support for versions older than GFE_81 in backwards compatibility 
> checks and command tables.
> Remove the unused and unreleased GFE_82 ordinal for consistency.



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

[jira] [Commented] (GEODE-8852) Add additional tests for Redis HVALS command

[ASF GitHub Bot (Jira)]

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

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

jdeppe-pivotal merged pull request #5931:
URL: https://github.com/apache/geode/pull/5931


   



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 additional tests for Redis HVALS command
> 
>
> Key: GEODE-8852
> URL: https://issues.apache.org/jira/browse/GEODE-8852
> Project: Geode
>  Issue Type: Test
>  Components: redis
>Reporter: Jens Deppe
>Priority: Major
>  Labels: pull-request-available
>
> Adding concurrency test as well as additional integration test



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

[jira] [Commented] (GEODE-8852) Add additional tests for Redis HVALS command

[ASF subversion and git services (Jira)]

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

ASF subversion and git services commented on GEODE-8852:


Commit 85d340e4d1c863158758c5dc0f73cc276b6010c4 in geode's branch 
refs/heads/develop from Jens Deppe
[ https://gitbox.apache.org/repos/asf?p=geode.git;h=85d340e ]

GEODE-8852: Add additional tests for Redis HVALS command (#5931)

* Also make error response assertions more exact

Co-authored-by: Ray Ingles 

> Add additional tests for Redis HVALS command
> 
>
> Key: GEODE-8852
> URL: https://issues.apache.org/jira/browse/GEODE-8852
> Project: Geode
>  Issue Type: Test
>  Components: redis
>Reporter: Jens Deppe
>Priority: Major
>  Labels: pull-request-available
>
> Adding concurrency test as well as additional integration test



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

[jira] [Commented] (GEODE-8824) fix CODEOWNERS rules

[ASF GitHub Bot (Jira)]

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

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

upthewaterspout commented on pull request #5934:
URL: https://github.com/apache/geode/pull/5934#issuecomment-765581675


   Since it looks like you are restricting the list of owners for CODEOWNERS 
and .asf.yml in this PR, should this have a dev list discussion?



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


> fix CODEOWNERS rules
> 
>
> Key: GEODE-8824
> URL: https://issues.apache.org/jira/browse/GEODE-8824
> Project: Geode
>  Issue Type: Bug
>  Components: github
>Reporter: Owen Nichols
>Assignee: Owen Nichols
>Priority: Major
>  Labels: pull-request-available
>
> We are finding some quirks in how GitHub parses the rules in CODEOWNERS.  It 
> seems to not support paths containing {{/**/}} and also a few rules were not 
> ordered correctly.



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

[jira] [Commented] (GEODE-8772) Avoid using default ports in tests [PERMANENT]

[ASF GitHub Bot (Jira)]

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

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

mhansonp commented on a change in pull request #5935:
URL: https://github.com/apache/geode/pull/5935#discussion_r562801891



##
File path: 
geode-assembly/src/distributedTest/java/org/apache/geode/rest/internal/web/controllers/RestAPIsAndInterOpsDUnitTest.java
##
@@ -168,6 +168,8 @@ private CacheServer 
createRegionAndStartCacheServer(String[] regions, Cache cach
   }
 
   private int startManager(final String locators, final String[] regions) 
throws IOException {
+int httpPort = getRandomAvailableTCPPort();

Review comment:
   this seems like it could be final...





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


> Avoid using default ports in tests [PERMANENT]
> --
>
> Key: GEODE-8772
> URL: https://issues.apache.org/jira/browse/GEODE-8772
> Project: Geode
>  Issue Type: Test
>  Components: tests
>Affects Versions: 1.14.0
>Reporter: Dale Emery
>Assignee: Dale Emery
>Priority: Major
>  Labels: GeodeOperationAPI, pull-request-available
>
> Many distributed tests and upgrade tests (and sometimes others) launch 
> members with default ports, especially for JMX (1099) and HTTP service 
> (7070). When run in parallel outside of docker, these tests often fail 
> because the default port is already in use in another test.
> Except when specifically testing the product's use of the defaults, every 
> test should assign ports known to be available. For many tests, we can 
> accomplish this by changing the test framework to assign available ports. 
> Other tests may require changes in the test code.



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

[jira] [Commented] (GEODE-8824) fix CODEOWNERS rules

[ASF GitHub Bot (Jira)]

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

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

onichols-pivotal commented on pull request #5934:
URL: https://github.com/apache/geode/pull/5934#issuecomment-765588723


   > Since it looks like you are restricting the list of owners for CODEOWNERS 
and .asf.yml in this PR, should this have a dev list discussion?
   
   That seems reasonable.  The question was previously asked on the dev list 
"who owns the CODEOWNERS file"  but never answered...
   
   CODEOWNERS is currently ownerless since the syntax Robert tried didn't work. 
 I'd like to be tagged on any modification just to review that changes are 
syntactically sound, minimum of 2 owners is maintained, formatting is 
consistent, and all owners are committers.  I have no desire to interfere with 
any actual changes in ownership.  But I would also be fine with keeping 
CODEOWNERS ownerless if that is perceived as the only democratic solution.



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


> fix CODEOWNERS rules
> 
>
> Key: GEODE-8824
> URL: https://issues.apache.org/jira/browse/GEODE-8824
> Project: Geode
>  Issue Type: Bug
>  Components: github
>Reporter: Owen Nichols
>Assignee: Owen Nichols
>Priority: Major
>  Labels: pull-request-available
>
> We are finding some quirks in how GitHub parses the rules in CODEOWNERS.  It 
> seems to not support paths containing {{/**/}} and also a few rules were not 
> ordered correctly.



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

[jira] [Commented] (GEODE-8837) Establish GFE_81 as the oldest supported client version.

[ASF GitHub Bot (Jira)]

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

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

Bill commented on a change in pull request #5905:
URL: https://github.com/apache/geode/pull/5905#discussion_r562817499



##
File path: 
geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/CommandInitializer.java
##
@@ -111,37 +89,76 @@
  * @since GemFire 5.7
  */
 
-public class CommandInitializer {
+public class CommandInitializer implements CommandRegistry {
 
-  @Immutable
-  static final Map> ALL_COMMANDS = 
initializeAllCommands();
+  @Deprecated
+  @MakeNotStatic
+  static final CommandInitializer instance = new CommandInitializer();
 
   /**
-   * Register a new command with the system.
+   * Gets legacy singleton instance.
+   *
+   * @deprecated Efforts should be made to get and instance from the cache or 
other object.
*
-   * @param messageType - An ordinal for this message. This must be something 
defined in MessageType
-   *that has not already been allocated to a different command.
-   * @param versionToNewCommand The command to register, for different 
versions. The key is the
-   *earliest version for which this command class is valid (starting 
with GFE_57). The value
-   *is the command object for clients starting with that version.
+   * @return legacy singleton instance. Instance is not immutable.
*/
-  public static void registerCommand(int messageType,
+  @Deprecated
+  public static CommandInitializer getDefaultInstance() {
+return instance;
+  }
+
+  final Map> 
unmodifiableRegisteredCommands;
+  final LinkedHashMap> 
modifiableRegisteredCommands;

Review comment:
   Thanks @pivotal-jbarrett.





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


> Establish GFE_81 as the oldest supported client version.
> 
>
> Key: GEODE-8837
> URL: https://issues.apache.org/jira/browse/GEODE-8837
> Project: Geode
>  Issue Type: Improvement
>  Components: client/server, core, cq, functions
>Reporter: Jacob Barrett
>Priority: Major
>  Labels: pull-request-available
>
> Remove support for versions older than GFE_81 in backwards compatibility 
> checks and command tables.
> Remove the unused and unreleased GFE_82 ordinal for consistency.



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

[jira] [Commented] (GEODE-8837) Establish GFE_81 as the oldest supported client version.

[ASF GitHub Bot (Jira)]

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

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

Bill commented on a change in pull request #5905:
URL: https://github.com/apache/geode/pull/5905#discussion_r562790958



##
File path: 
geode-serialization/src/main/java/org/apache/geode/internal/serialization/KnownVersion.java
##
@@ -367,61 +377,48 @@ public static String unsupportedVersionMessage(final 
short ordinal) {
   }
 
   public String getMethodSuffix() {
-return this.methodSuffix;
+return methodSuffix;
   }
 
   public String getName() {
-return this.name;
+return name;
   }
 
-  public short getMajorVersion() {
-return this.majorVersion;
+  public short getMajor() {
+return major;
   }
 
-  public short getMinorVersion() {
-return this.minorVersion;
+  public short getMinor() {
+return minor;
   }
 
   public short getRelease() {
-return this.release;
+return release;
   }
 
   public short getPatch() {
-return this.patch;
+return patch;
   }
 
-  /**
-   * Returns a string representation for this Version.
-   *
-   * @return the name of this operation.
-   */
   @Override
   public String toString() {
-return this.productName + " " + this.name;
-  }
-
-  public byte[] toBytes() {
-byte[] bytes = new byte[2];
-bytes[0] = (byte) (ordinal() >> 8);
-bytes[1] = (byte) ordinal();
-return bytes;
+return productName + " " + name;
   }
 
   public static Iterable getAllVersions() {
-return Arrays.asList(VALUES).stream().filter(x -> x != null && x != 
TEST_VERSION)
+return Arrays.stream(VALUES).filter(x -> x != null && x != TEST_VERSION)
 .collect(Collectors.toList());
   }
 
   /**
* package-protected for use by Versioning factory
*/
-  static KnownVersion getKnownVersionOrDefault(final short ordinal,
-  final KnownVersion defaultKnownVersion) {
+  static KnownVersion getKnownVersionOrDefault(final short ordinal) {

Review comment:
   I'm very glad you didn't remove it. Indeed it is a key part of the 
abstraction barrier.
   
   I'll offer one other argument for the old version of the method: in addition 
to the form matching the form of `Map.getOrDefault()` the old version of this 
method matches `Versioning.getKnownVersionOrDefault(Version,KnownVersion)`.
   
   I'm a fan of the consistency with `java.util.Map` and between 
`Versioning.getKnownVersionOrDefault` and 
`KnownVersion.getKnownVersionOrDefault`.





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


> Establish GFE_81 as the oldest supported client version.
> 
>
> Key: GEODE-8837
> URL: https://issues.apache.org/jira/browse/GEODE-8837
> Project: Geode
>  Issue Type: Improvement
>  Components: client/server, core, cq, functions
>Reporter: Jacob Barrett
>Priority: Major
>  Labels: pull-request-available
>
> Remove support for versions older than GFE_81 in backwards compatibility 
> checks and command tables.
> Remove the unused and unreleased GFE_82 ordinal for consistency.



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

[jira] [Commented] (GEODE-8837) Establish GFE_81 as the oldest supported client version.

[ASF GitHub Bot (Jira)]

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

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

Bill commented on a change in pull request #5905:
URL: https://github.com/apache/geode/pull/5905#discussion_r562790958



##
File path: 
geode-serialization/src/main/java/org/apache/geode/internal/serialization/KnownVersion.java
##
@@ -367,61 +377,48 @@ public static String unsupportedVersionMessage(final 
short ordinal) {
   }
 
   public String getMethodSuffix() {
-return this.methodSuffix;
+return methodSuffix;
   }
 
   public String getName() {
-return this.name;
+return name;
   }
 
-  public short getMajorVersion() {
-return this.majorVersion;
+  public short getMajor() {
+return major;
   }
 
-  public short getMinorVersion() {
-return this.minorVersion;
+  public short getMinor() {
+return minor;
   }
 
   public short getRelease() {
-return this.release;
+return release;
   }
 
   public short getPatch() {
-return this.patch;
+return patch;
   }
 
-  /**
-   * Returns a string representation for this Version.
-   *
-   * @return the name of this operation.
-   */
   @Override
   public String toString() {
-return this.productName + " " + this.name;
-  }
-
-  public byte[] toBytes() {
-byte[] bytes = new byte[2];
-bytes[0] = (byte) (ordinal() >> 8);
-bytes[1] = (byte) ordinal();
-return bytes;
+return productName + " " + name;
   }
 
   public static Iterable getAllVersions() {
-return Arrays.asList(VALUES).stream().filter(x -> x != null && x != 
TEST_VERSION)
+return Arrays.stream(VALUES).filter(x -> x != null && x != TEST_VERSION)
 .collect(Collectors.toList());
   }
 
   /**
* package-protected for use by Versioning factory
*/
-  static KnownVersion getKnownVersionOrDefault(final short ordinal,
-  final KnownVersion defaultKnownVersion) {
+  static KnownVersion getKnownVersionOrDefault(final short ordinal) {

Review comment:
   I'm very glad you didn't remove it. Indeed it is a key part of the 
abstraction barrier.
   
   I'll offer one other argument for my original method signature: in addition 
to the form matching the form of `Map.getOrDefault()` the old signature of this 
method matches the signature of 
`Versioning.getKnownVersionOrDefault(Version,KnownVersion)`.
   
   I'm a fan of the consistency with `java.util.Map` and also a fan of 
consistency between `Versioning.getKnownVersionOrDefault` and 
`KnownVersion.getKnownVersionOrDefault`.





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


> Establish GFE_81 as the oldest supported client version.
> 
>
> Key: GEODE-8837
> URL: https://issues.apache.org/jira/browse/GEODE-8837
> Project: Geode
>  Issue Type: Improvement
>  Components: client/server, core, cq, functions
>Reporter: Jacob Barrett
>Priority: Major
>  Labels: pull-request-available
>
> Remove support for versions older than GFE_81 in backwards compatibility 
> checks and command tables.
> Remove the unused and unreleased GFE_82 ordinal for consistency.



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

[jira] [Reopened] (GEODE-7016) CI failure: ServerStartupRedundancyRecoveryNotificationTest > startupReportsOnlineOnlyAfterRedundancyRestored FAILED

[John Hutchison (Jira)]

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

John Hutchison reopened GEODE-7016:
---

> CI failure: ServerStartupRedundancyRecoveryNotificationTest > 
> startupReportsOnlineOnlyAfterRedundancyRestored FAILED
> 
>
> Key: GEODE-7016
> URL: https://issues.apache.org/jira/browse/GEODE-7016
> Project: Geode
>  Issue Type: Bug
>  Components: gfsh
>Affects Versions: 1.10.0
>Reporter: Anilkumar Gingade
>Assignee: Kirk Lund
>Priority: Major
>  Labels: pull-request-available
> Fix For: 1.14.0
>
>
> {noformat}
> org.apache.geode.launchers.ServerStartupRedundancyRecoveryNotificationTest > 
> startupReportsOnlineOnlyAfterRedundancyRestored FAILED
> org.junit.ComparisonFailure: expected:<[0]> but was:<[1]>
> at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native 
> Method)
> at 
> sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
> at 
> sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
> at 
> org.apache.geode.test.junit.rules.gfsh.GfshExecution.awaitTermination(GfshExecution.java:125)
> at 
> org.apache.geode.test.junit.rules.gfsh.GfshRule.execute(GfshRule.java:125)
> at 
> org.apache.geode.test.junit.rules.gfsh.GfshRule.execute(GfshRule.java:112)
> at 
> org.apache.geode.launchers.ServerStartupRedundancyRecoveryNotificationTest.startupReportsOnlineOnlyAfterRedundancyRestored(ServerStartupRedundancyRecoveryNotificationTest.java:142)
> org.junit.ComparisonFailure: expected:<[0]> but was:<[1]>
> at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native 
> Method)
> at 
> sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
> at 
> sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
> at 
> org.apache.geode.test.junit.rules.gfsh.GfshExecution.awaitTermination(GfshExecution.java:125)
> at 
> org.apache.geode.test.junit.rules.gfsh.GfshRule.execute(GfshRule.java:125)
> at 
> org.apache.geode.test.junit.rules.gfsh.GfshRule.execute(GfshRule.java:112)
> at 
> org.apache.geode.launchers.ServerStartupRedundancyRecoveryNotificationTest.stopAllMembers(ServerStartupRedundancyRecoveryNotificationTest.java:128)
> {noformat}
> https://concourse.gemfire-ci.info/teams/main/pipelines/gemfire-develop-main/jobs/AcceptanceTestOpenJDK8/builds/797
> Test report artifacts from this job are available at:
> gs://gemfire-test-artifacts/builds/gemfire-develop-main/9.9.0-build.0258/test-artifacts/1564078711/acceptancetestfiles-OpenJDK8-9.9.0-build.0258.tgz



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

[jira] [Updated] (GEODE-7016) CI failure: ServerStartupRedundancyRecoveryNotificationTest > startupReportsOnlineOnlyAfterRedundancyRestored FAILED

[John Hutchison (Jira)]

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

John Hutchison updated GEODE-7016:
--
Attachment: acceptancetestfiles-OpenJDK11-1.14.0-build.0628 (1).tgz

> CI failure: ServerStartupRedundancyRecoveryNotificationTest > 
> startupReportsOnlineOnlyAfterRedundancyRestored FAILED
> 
>
> Key: GEODE-7016
> URL: https://issues.apache.org/jira/browse/GEODE-7016
> Project: Geode
>  Issue Type: Bug
>  Components: gfsh
>Affects Versions: 1.10.0
>Reporter: Anilkumar Gingade
>Assignee: Kirk Lund
>Priority: Major
>  Labels: pull-request-available
> Fix For: 1.14.0
>
> Attachments: acceptancetestfiles-OpenJDK11-1.14.0-build.0628 (1).tgz
>
>
> {noformat}
> org.apache.geode.launchers.ServerStartupRedundancyRecoveryNotificationTest > 
> startupReportsOnlineOnlyAfterRedundancyRestored FAILED
> org.junit.ComparisonFailure: expected:<[0]> but was:<[1]>
> at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native 
> Method)
> at 
> sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
> at 
> sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
> at 
> org.apache.geode.test.junit.rules.gfsh.GfshExecution.awaitTermination(GfshExecution.java:125)
> at 
> org.apache.geode.test.junit.rules.gfsh.GfshRule.execute(GfshRule.java:125)
> at 
> org.apache.geode.test.junit.rules.gfsh.GfshRule.execute(GfshRule.java:112)
> at 
> org.apache.geode.launchers.ServerStartupRedundancyRecoveryNotificationTest.startupReportsOnlineOnlyAfterRedundancyRestored(ServerStartupRedundancyRecoveryNotificationTest.java:142)
> org.junit.ComparisonFailure: expected:<[0]> but was:<[1]>
> at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native 
> Method)
> at 
> sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
> at 
> sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
> at 
> org.apache.geode.test.junit.rules.gfsh.GfshExecution.awaitTermination(GfshExecution.java:125)
> at 
> org.apache.geode.test.junit.rules.gfsh.GfshRule.execute(GfshRule.java:125)
> at 
> org.apache.geode.test.junit.rules.gfsh.GfshRule.execute(GfshRule.java:112)
> at 
> org.apache.geode.launchers.ServerStartupRedundancyRecoveryNotificationTest.stopAllMembers(ServerStartupRedundancyRecoveryNotificationTest.java:128)
> {noformat}
> https://concourse.gemfire-ci.info/teams/main/pipelines/gemfire-develop-main/jobs/AcceptanceTestOpenJDK8/builds/797
> Test report artifacts from this job are available at:
> gs://gemfire-test-artifacts/builds/gemfire-develop-main/9.9.0-build.0258/test-artifacts/1564078711/acceptancetestfiles-OpenJDK8-9.9.0-build.0258.tgz



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

[jira] [Updated] (GEODE-7016) CI failure: ServerStartupRedundancyRecoveryNotificationTest > startupReportsOnlineOnlyAfterRedundancyRestored FAILED

[John Hutchison (Jira)]

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

John Hutchison updated GEODE-7016:
--
Attachment: acceptancetestfiles-OpenJDK11-1.14.0-build.0628 (2).tgz

> CI failure: ServerStartupRedundancyRecoveryNotificationTest > 
> startupReportsOnlineOnlyAfterRedundancyRestored FAILED
> 
>
> Key: GEODE-7016
> URL: https://issues.apache.org/jira/browse/GEODE-7016
> Project: Geode
>  Issue Type: Bug
>  Components: gfsh
>Affects Versions: 1.10.0
>Reporter: Anilkumar Gingade
>Assignee: Kirk Lund
>Priority: Major
>  Labels: pull-request-available
> Fix For: 1.14.0
>
> Attachments: acceptancetestfiles-OpenJDK11-1.14.0-build.0628 (1).tgz, 
> acceptancetestfiles-OpenJDK11-1.14.0-build.0628 (2).tgz
>
>
> {noformat}
> org.apache.geode.launchers.ServerStartupRedundancyRecoveryNotificationTest > 
> startupReportsOnlineOnlyAfterRedundancyRestored FAILED
> org.junit.ComparisonFailure: expected:<[0]> but was:<[1]>
> at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native 
> Method)
> at 
> sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
> at 
> sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
> at 
> org.apache.geode.test.junit.rules.gfsh.GfshExecution.awaitTermination(GfshExecution.java:125)
> at 
> org.apache.geode.test.junit.rules.gfsh.GfshRule.execute(GfshRule.java:125)
> at 
> org.apache.geode.test.junit.rules.gfsh.GfshRule.execute(GfshRule.java:112)
> at 
> org.apache.geode.launchers.ServerStartupRedundancyRecoveryNotificationTest.startupReportsOnlineOnlyAfterRedundancyRestored(ServerStartupRedundancyRecoveryNotificationTest.java:142)
> org.junit.ComparisonFailure: expected:<[0]> but was:<[1]>
> at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native 
> Method)
> at 
> sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
> at 
> sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
> at 
> org.apache.geode.test.junit.rules.gfsh.GfshExecution.awaitTermination(GfshExecution.java:125)
> at 
> org.apache.geode.test.junit.rules.gfsh.GfshRule.execute(GfshRule.java:125)
> at 
> org.apache.geode.test.junit.rules.gfsh.GfshRule.execute(GfshRule.java:112)
> at 
> org.apache.geode.launchers.ServerStartupRedundancyRecoveryNotificationTest.stopAllMembers(ServerStartupRedundancyRecoveryNotificationTest.java:128)
> {noformat}
> https://concourse.gemfire-ci.info/teams/main/pipelines/gemfire-develop-main/jobs/AcceptanceTestOpenJDK8/builds/797
> Test report artifacts from this job are available at:
> gs://gemfire-test-artifacts/builds/gemfire-develop-main/9.9.0-build.0258/test-artifacts/1564078711/acceptancetestfiles-OpenJDK8-9.9.0-build.0258.tgz



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

[jira] [Commented] (GEODE-7016) CI failure: ServerStartupRedundancyRecoveryNotificationTest > startupReportsOnlineOnlyAfterRedundancyRestored FAILED

[John Hutchison (Jira)]

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

John Hutchison commented on GEODE-7016:
---

seen in CI Run -  ticket re-opened.  

org.apache.geode.launchers.ServerStartupValueRecoveryNotificationTest > 
startupReportsOnlineOnlyAfterRedundancyRestored FAILED
org.junit.ComparisonFailure: [Exit value from process started by 
[a3a70e5ecf4d838f: gfsh -e connect --locator=localhost[23137] -e start server 
--name=server1 --dir=/tmp/junit2110431257149045019/server1secondfolder 
--locators=localhost[23137]]] expected:<[0]> but was:<[1]>
at 
jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
at 
jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
at 
jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
at 
org.apache.geode.test.junit.rules.gfsh.GfshExecution.awaitTermination(GfshExecution.java:103)
at 
org.apache.geode.test.junit.rules.gfsh.GfshRule.execute(GfshRule.java:143)
at 
org.apache.geode.test.junit.rules.gfsh.GfshRule.execute(GfshRule.java:152)
at 
org.apache.geode.test.junit.rules.gfsh.GfshRule.execute(GfshRule.java:118)
at 
org.apache.geode.launchers.ServerStartupValueRecoveryNotificationTest.startupReportsOnlineOnlyAfterRedundancyRestored(ServerStartupValueRecoveryNotificationTest.java:146)


=-=-=-=-=-=-=-=-=-=-=-=-=-=-=  Test Results URI 
=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
http://files.apachegeode-ci.info/builds/apache-develop-main/1.14.0-build.0628/test-results/acceptanceTest/1611279518/
=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=


 [^acceptancetestfiles-OpenJDK11-1.14.0-build.0628 (2).tgz] 

> CI failure: ServerStartupRedundancyRecoveryNotificationTest > 
> startupReportsOnlineOnlyAfterRedundancyRestored FAILED
> 
>
> Key: GEODE-7016
> URL: https://issues.apache.org/jira/browse/GEODE-7016
> Project: Geode
>  Issue Type: Bug
>  Components: gfsh
>Affects Versions: 1.10.0
>Reporter: Anilkumar Gingade
>Assignee: Kirk Lund
>Priority: Major
>  Labels: pull-request-available
> Fix For: 1.14.0
>
> Attachments: acceptancetestfiles-OpenJDK11-1.14.0-build.0628 (1).tgz, 
> acceptancetestfiles-OpenJDK11-1.14.0-build.0628 (2).tgz
>
>
> {noformat}
> org.apache.geode.launchers.ServerStartupRedundancyRecoveryNotificationTest > 
> startupReportsOnlineOnlyAfterRedundancyRestored FAILED
> org.junit.ComparisonFailure: expected:<[0]> but was:<[1]>
> at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native 
> Method)
> at 
> sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
> at 
> sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
> at 
> org.apache.geode.test.junit.rules.gfsh.GfshExecution.awaitTermination(GfshExecution.java:125)
> at 
> org.apache.geode.test.junit.rules.gfsh.GfshRule.execute(GfshRule.java:125)
> at 
> org.apache.geode.test.junit.rules.gfsh.GfshRule.execute(GfshRule.java:112)
> at 
> org.apache.geode.launchers.ServerStartupRedundancyRecoveryNotificationTest.startupReportsOnlineOnlyAfterRedundancyRestored(ServerStartupRedundancyRecoveryNotificationTest.java:142)
> org.junit.ComparisonFailure: expected:<[0]> but was:<[1]>
> at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native 
> Method)
> at 
> sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
> at 
> sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
> at 
> org.apache.geode.test.junit.rules.gfsh.GfshExecution.awaitTermination(GfshExecution.java:125)
> at 
> org.apache.geode.test.junit.rules.gfsh.GfshRule.execute(GfshRule.java:125)
> at 
> org.apache.geode.test.junit.rules.gfsh.GfshRule.execute(GfshRule.java:112)
> at 
> org.apache.geode.launchers.ServerStartupRedundancyRecoveryNotificationTest.stopAllMembers(ServerStartupRedundancyRecoveryNotificationTest.java:128)
> {noformat}
> https://concourse.gemfire-ci.info/teams/main/pipelines/gemfire-develop-main/jobs/AcceptanceTestOpenJDK8/builds/797
> Test report artifacts from this job are available at:
> gs://gemfire-test-artifacts/builds/gemfire-develop-main/9.9.0-build.0258/test-artifacts/1564078711/acceptancetestfiles-OpenJDK8-9.9.0-build.0258.tgz



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

[jira] [Commented] (GEODE-7016) CI failure: ServerStartupRedundancyRecoveryNotificationTest > startupReportsOnlineOnlyAfterRedundancyRestored FAILED

[Geode Integration (Jira)]

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

Geode Integration commented on GEODE-7016:
--

Seen in [WindowsUnitTestOpenJDK11 
#686|https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-develop-main/jobs/WindowsUnitTestOpenJDK11/builds/686]
 ... see [test 
results|http://files.apachegeode-ci.info/builds/apache-develop-main/1.14.0-build.0630/test-results/test/1611340430/]
 or download 
[artifacts|http://files.apachegeode-ci.info/builds/apache-develop-main/1.14.0-build.0630/test-artifacts/1611340430/windows-unittestfiles-OpenJDK11-1.14.0-build.0630.tgz].

> CI failure: ServerStartupRedundancyRecoveryNotificationTest > 
> startupReportsOnlineOnlyAfterRedundancyRestored FAILED
> 
>
> Key: GEODE-7016
> URL: https://issues.apache.org/jira/browse/GEODE-7016
> Project: Geode
>  Issue Type: Bug
>  Components: gfsh
>Affects Versions: 1.10.0
>Reporter: Anilkumar Gingade
>Assignee: Kirk Lund
>Priority: Major
>  Labels: pull-request-available
> Fix For: 1.14.0
>
> Attachments: acceptancetestfiles-OpenJDK11-1.14.0-build.0628 (1).tgz, 
> acceptancetestfiles-OpenJDK11-1.14.0-build.0628 (2).tgz
>
>
> {noformat}
> org.apache.geode.launchers.ServerStartupRedundancyRecoveryNotificationTest > 
> startupReportsOnlineOnlyAfterRedundancyRestored FAILED
> org.junit.ComparisonFailure: expected:<[0]> but was:<[1]>
> at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native 
> Method)
> at 
> sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
> at 
> sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
> at 
> org.apache.geode.test.junit.rules.gfsh.GfshExecution.awaitTermination(GfshExecution.java:125)
> at 
> org.apache.geode.test.junit.rules.gfsh.GfshRule.execute(GfshRule.java:125)
> at 
> org.apache.geode.test.junit.rules.gfsh.GfshRule.execute(GfshRule.java:112)
> at 
> org.apache.geode.launchers.ServerStartupRedundancyRecoveryNotificationTest.startupReportsOnlineOnlyAfterRedundancyRestored(ServerStartupRedundancyRecoveryNotificationTest.java:142)
> org.junit.ComparisonFailure: expected:<[0]> but was:<[1]>
> at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native 
> Method)
> at 
> sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
> at 
> sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
> at 
> org.apache.geode.test.junit.rules.gfsh.GfshExecution.awaitTermination(GfshExecution.java:125)
> at 
> org.apache.geode.test.junit.rules.gfsh.GfshRule.execute(GfshRule.java:125)
> at 
> org.apache.geode.test.junit.rules.gfsh.GfshRule.execute(GfshRule.java:112)
> at 
> org.apache.geode.launchers.ServerStartupRedundancyRecoveryNotificationTest.stopAllMembers(ServerStartupRedundancyRecoveryNotificationTest.java:128)
> {noformat}
> https://concourse.gemfire-ci.info/teams/main/pipelines/gemfire-develop-main/jobs/AcceptanceTestOpenJDK8/builds/797
> Test report artifacts from this job are available at:
> gs://gemfire-test-artifacts/builds/gemfire-develop-main/9.9.0-build.0258/test-artifacts/1564078711/acceptancetestfiles-OpenJDK8-9.9.0-build.0258.tgz



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

[jira] [Commented] (GEODE-8824) fix CODEOWNERS rules

[ASF GitHub Bot (Jira)]

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

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

onichols-pivotal merged pull request #5934:
URL: https://github.com/apache/geode/pull/5934


   



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


> fix CODEOWNERS rules
> 
>
> Key: GEODE-8824
> URL: https://issues.apache.org/jira/browse/GEODE-8824
> Project: Geode
>  Issue Type: Bug
>  Components: github
>Reporter: Owen Nichols
>Assignee: Owen Nichols
>Priority: Major
>  Labels: pull-request-available
>
> We are finding some quirks in how GitHub parses the rules in CODEOWNERS.  It 
> seems to not support paths containing {{/**/}} and also a few rules were not 
> ordered correctly.



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

[jira] [Resolved] (GEODE-8824) fix CODEOWNERS rules

[Owen Nichols (Jira)]

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

Owen Nichols resolved GEODE-8824.
-
Fix Version/s: 1.14.0
   Resolution: Fixed

> fix CODEOWNERS rules
> 
>
> Key: GEODE-8824
> URL: https://issues.apache.org/jira/browse/GEODE-8824
> Project: Geode
>  Issue Type: Bug
>  Components: github
>Reporter: Owen Nichols
>Assignee: Owen Nichols
>Priority: Major
>  Labels: pull-request-available
> Fix For: 1.14.0
>
>
> We are finding some quirks in how GitHub parses the rules in CODEOWNERS.  It 
> seems to not support paths containing {{/**/}} and also a few rules were not 
> ordered correctly.



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

[jira] [Commented] (GEODE-8824) fix CODEOWNERS rules

[ASF subversion and git services (Jira)]

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

ASF subversion and git services commented on GEODE-8824:


Commit ff41e497bf0703a94b2778c13eb81283b8577ce7 in geode's branch 
refs/heads/develop from Owen Nichols
[ https://gitbox.apache.org/repos/asf?p=geode.git;h=ff41e49 ]

GEODE-8824: fix some rule overlap and unintended singleton codeowners (#5934)

* fix some rule overlap and unintended singleton codeowners
* fix whitespace/alignment and change tabs to spaces
* since Apache doesn't allow ownership to be assigned to 'all committers' leave 
ownership of CODEOWNERS ownerless as the only fair democratic analog of 'all'
* cherry-pick #5944

> fix CODEOWNERS rules
> 
>
> Key: GEODE-8824
> URL: https://issues.apache.org/jira/browse/GEODE-8824
> Project: Geode
>  Issue Type: Bug
>  Components: github
>Reporter: Owen Nichols
>Assignee: Owen Nichols
>Priority: Major
>  Labels: pull-request-available
> Fix For: 1.14.0
>
>
> We are finding some quirks in how GitHub parses the rules in CODEOWNERS.  It 
> seems to not support paths containing {{/**/}} and also a few rules were not 
> ordered correctly.



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

[jira] [Commented] (GEODE-8855) Redis Integration Test: add tests for all commands to redisStatsIntegrationTest

[ASF GitHub Bot (Jira)]

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

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

nonbinaryprogrammer commented on a change in pull request #5937:
URL: https://github.com/apache/geode/pull/5937#discussion_r562864838



##
File path: 
geode-redis/src/main/java/org/apache/geode/redis/internal/data/RedisHashCommandsFunctionExecutor.java
##
@@ -102,14 +102,14 @@ public int hstrlen(ByteArrayWrapper key, ByteArrayWrapper 
field) {
   @Override
   public long hincrby(ByteArrayWrapper key, ByteArrayWrapper field, long 
increment) {
 return stripedExecute(key,
-() -> getRedisHash(key, true)
+() -> getRedisHash(key, false)

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


> Redis Integration Test: add tests for all commands to 
> redisStatsIntegrationTest
> ---
>
> Key: GEODE-8855
> URL: https://issues.apache.org/jira/browse/GEODE-8855
> Project: Geode
>  Issue Type: Test
>  Components: redis
>Reporter: Helena Bales
>Priority: Major
>  Labels: pull-request-available
>
> add tests for the unsupported commands and the rest of the untested commands. 
> organize the tests. make the tests run against redis and native redis. remove 
> duplicate tests from the hitsmisses test.



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

[jira] [Created] (GEODE-8869) Tomcat6SessionsTest CI Failure

[John Hutchison (Jira)]

John Hutchison created GEODE-8869:
-

 Summary: Tomcat6SessionsTest CI Failure
 Key: GEODE-8869
 URL: https://issues.apache.org/jira/browse/GEODE-8869
 Project: Geode
  Issue Type: New Feature
Reporter: John Hutchison


org.apache.geode.modules.session.Tomcat6SessionsTest > classMethod FAILED
LifecycleException:  Protocol handler initialization failed: 
java.net.BindException: Address already in use: NET_Bind :49863
at 
org.apache.catalina.connector.Connector.initialize(Connector.java:1125)
at org.apache.catalina.startup.Embedded.start(Embedded.java:830)
at 
org.apache.geode.modules.session.EmbeddedTomcat.startContainer(EmbeddedTomcat.java:98)
at 
org.apache.geode.modules.session.AbstractSessionsTest.setupServer(AbstractSessionsTest.java:72)
at 
org.apache.geode.modules.session.Tomcat6SessionsTest.setupClass(Tomcat6SessionsTest.java:29)

=-=-=-=-=-=-=-=-=-=-=-=-=-=-=  Test Results URI 
=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
http://files.apachegeode-ci.info/builds/apache-develop-main/1.14.0-build.0630/test-results/integrationTest/1611347047/
=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=




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

[jira] [Commented] (GEODE-8869) Tomcat6SessionsTest CI Failure

[Geode Integration (Jira)]

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

Geode Integration commented on GEODE-8869:
--

Seen in [WindowsIntegrationTestOpenJDK11 
#672|https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-develop-main/jobs/WindowsIntegrationTestOpenJDK11/builds/672]
 ... see [test 
results|http://files.apachegeode-ci.info/builds/apache-develop-main/1.14.0-build.0630/test-results/integrationTest/1611347047/]
 or download 
[artifacts|http://files.apachegeode-ci.info/builds/apache-develop-main/1.14.0-build.0630/test-artifacts/1611347047/windows-integrationtestfiles-OpenJDK11-1.14.0-build.0630.tgz].

> Tomcat6SessionsTest CI Failure
> --
>
> Key: GEODE-8869
> URL: https://issues.apache.org/jira/browse/GEODE-8869
> Project: Geode
>  Issue Type: New Feature
>Reporter: John Hutchison
>Priority: Major
>
> org.apache.geode.modules.session.Tomcat6SessionsTest > classMethod FAILED
> LifecycleException:  Protocol handler initialization failed: 
> java.net.BindException: Address already in use: NET_Bind :49863
> at 
> org.apache.catalina.connector.Connector.initialize(Connector.java:1125)
> at org.apache.catalina.startup.Embedded.start(Embedded.java:830)
> at 
> org.apache.geode.modules.session.EmbeddedTomcat.startContainer(EmbeddedTomcat.java:98)
> at 
> org.apache.geode.modules.session.AbstractSessionsTest.setupServer(AbstractSessionsTest.java:72)
> at 
> org.apache.geode.modules.session.Tomcat6SessionsTest.setupClass(Tomcat6SessionsTest.java:29)
> =-=-=-=-=-=-=-=-=-=-=-=-=-=-=  Test Results URI 
> =-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
> http://files.apachegeode-ci.info/builds/apache-develop-main/1.14.0-build.0630/test-results/integrationTest/1611347047/
> =-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=



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

[jira] [Commented] (GEODE-8869) Tomcat6SessionsTest CI Failure

[John Hutchison (Jira)]

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

John Hutchison commented on GEODE-8869:
---

test artifacts too big to upload.  Contact author if unable to download

http://files.apachegeode-ci.info/builds/apache-develop-main/1.14.0-build.0630/test-artifacts/1611347047/windows-integrationtestfiles-OpenJDK11-1.14.0-build.0630.tgz

> Tomcat6SessionsTest CI Failure
> --
>
> Key: GEODE-8869
> URL: https://issues.apache.org/jira/browse/GEODE-8869
> Project: Geode
>  Issue Type: New Feature
>Reporter: John Hutchison
>Priority: Major
>
> org.apache.geode.modules.session.Tomcat6SessionsTest > classMethod FAILED
> LifecycleException:  Protocol handler initialization failed: 
> java.net.BindException: Address already in use: NET_Bind :49863
> at 
> org.apache.catalina.connector.Connector.initialize(Connector.java:1125)
> at org.apache.catalina.startup.Embedded.start(Embedded.java:830)
> at 
> org.apache.geode.modules.session.EmbeddedTomcat.startContainer(EmbeddedTomcat.java:98)
> at 
> org.apache.geode.modules.session.AbstractSessionsTest.setupServer(AbstractSessionsTest.java:72)
> at 
> org.apache.geode.modules.session.Tomcat6SessionsTest.setupClass(Tomcat6SessionsTest.java:29)
> =-=-=-=-=-=-=-=-=-=-=-=-=-=-=  Test Results URI 
> =-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
> http://files.apachegeode-ci.info/builds/apache-develop-main/1.14.0-build.0630/test-results/integrationTest/1611347047/
> =-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=



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

[jira] [Commented] (GEODE-8858) Unit/Integration tests for HSETNX command

[ASF GitHub Bot (Jira)]

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

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

jdeppe-pivotal commented on a change in pull request #5940:
URL: https://github.com/apache/geode/pull/5940#discussion_r562907161



##
File path: 
geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/hash/AbstractHashesIntegrationTest.java
##
@@ -365,6 +365,40 @@ public void testHSetNXExecutor() {
 
   }
 
+  @Test
+  public void hsetNX_shouldThrowErrorIfKeyIsWrongType() {
+String string_key = "String_Key";
+String set_key = "Set_Key";
+String field = "field";
+String value = "value";
+
+jedis.set(string_key, value);
+jedis.sadd(set_key, field);
+
+assertThatThrownBy(
+() -> jedis.hsetnx(string_key, field, "something else"))
+.isInstanceOf(JedisDataException.class)
+.hasMessageContaining("WRONGTYPE");
+assertThatThrownBy(
+() -> jedis.hsetnx(set_key, field, "something 
else")).isInstanceOf(JedisDataException.class)
+.hasMessageContaining("WRONGTYPE");
+
+jedis.del(string_key);
+jedis.del(set_key);
+  }
+
+  @Test
+  public void hsetnx_shouldThrowError_givenWrongNumberOfArguments() {
+assertThatThrownBy(() -> jedis.sendCommand(Protocol.Command.HSETNX))
+.hasMessageContaining("wrong number of arguments");
+assertThatThrownBy(() -> jedis.sendCommand(Protocol.Command.HSETNX, "1"))
+.hasMessageContaining("wrong number of arguments");
+assertThatThrownBy(() -> jedis.sendCommand(Protocol.Command.HSETNX, "1", 
"2"))
+.hasMessageContaining("wrong number of arguments");
+assertThatThrownBy(() -> jedis.sendCommand(Protocol.Command.HSETNX, "1", 
"2", "3", "4"))
+.hasMessageContaining("wrong number of arguments");
+  }

Review comment:
   I've already made these changes in another PR.





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


> Unit/Integration tests for HSETNX command
> -
>
> Key: GEODE-8858
> URL: https://issues.apache.org/jira/browse/GEODE-8858
> Project: Geode
>  Issue Type: Test
>  Components: redis
>Reporter: Raymond Ingles
>Priority: Major
>  Labels: pull-request-available
>
> Flesh out unit/integration tests for Redis HSETNX command.



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

[jira] [Commented] (GEODE-8858) Unit/Integration tests for HSETNX command

[ASF GitHub Bot (Jira)]

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

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

jdeppe-pivotal commented on a change in pull request #5940:
URL: https://github.com/apache/geode/pull/5940#discussion_r562907161



##
File path: 
geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/hash/AbstractHashesIntegrationTest.java
##
@@ -365,6 +365,40 @@ public void testHSetNXExecutor() {
 
   }
 
+  @Test
+  public void hsetNX_shouldThrowErrorIfKeyIsWrongType() {
+String string_key = "String_Key";
+String set_key = "Set_Key";
+String field = "field";
+String value = "value";
+
+jedis.set(string_key, value);
+jedis.sadd(set_key, field);
+
+assertThatThrownBy(
+() -> jedis.hsetnx(string_key, field, "something else"))
+.isInstanceOf(JedisDataException.class)
+.hasMessageContaining("WRONGTYPE");
+assertThatThrownBy(
+() -> jedis.hsetnx(set_key, field, "something 
else")).isInstanceOf(JedisDataException.class)
+.hasMessageContaining("WRONGTYPE");
+
+jedis.del(string_key);
+jedis.del(set_key);
+  }
+
+  @Test
+  public void hsetnx_shouldThrowError_givenWrongNumberOfArguments() {
+assertThatThrownBy(() -> jedis.sendCommand(Protocol.Command.HSETNX))
+.hasMessageContaining("wrong number of arguments");
+assertThatThrownBy(() -> jedis.sendCommand(Protocol.Command.HSETNX, "1"))
+.hasMessageContaining("wrong number of arguments");
+assertThatThrownBy(() -> jedis.sendCommand(Protocol.Command.HSETNX, "1", 
"2"))
+.hasMessageContaining("wrong number of arguments");
+assertThatThrownBy(() -> jedis.sendCommand(Protocol.Command.HSETNX, "1", 
"2", "3", "4"))
+.hasMessageContaining("wrong number of arguments");
+  }

Review comment:
   I've already made these changes in another PR.





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


> Unit/Integration tests for HSETNX command
> -
>
> Key: GEODE-8858
> URL: https://issues.apache.org/jira/browse/GEODE-8858
> Project: Geode
>  Issue Type: Test
>  Components: redis
>Reporter: Raymond Ingles
>Priority: Major
>  Labels: pull-request-available
>
> Flesh out unit/integration tests for Redis HSETNX command.



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

[jira] [Commented] (GEODE-8855) Redis Integration Test: add tests for all commands to redisStatsIntegrationTest

[ASF GitHub Bot (Jira)]

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

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

nonbinaryprogrammer merged pull request #5937:
URL: https://github.com/apache/geode/pull/5937


   



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


> Redis Integration Test: add tests for all commands to 
> redisStatsIntegrationTest
> ---
>
> Key: GEODE-8855
> URL: https://issues.apache.org/jira/browse/GEODE-8855
> Project: Geode
>  Issue Type: Test
>  Components: redis
>Reporter: Helena Bales
>Priority: Major
>  Labels: pull-request-available
>
> add tests for the unsupported commands and the rest of the untested commands. 
> organize the tests. make the tests run against redis and native redis. remove 
> duplicate tests from the hitsmisses test.



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

[jira] [Commented] (GEODE-8855) Redis Integration Test: add tests for all commands to redisStatsIntegrationTest

[ASF subversion and git services (Jira)]

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

ASF subversion and git services commented on GEODE-8855:


Commit abddba5165ba9f0dcd72d7ac424ca67e2cfe7c45 in geode's branch 
refs/heads/develop from Hale Bales
[ https://gitbox.apache.org/repos/asf?p=geode.git;h=abddba5 ]

GEODE-8855: fix hitmiss and stats integration tests (#5937)

GEODE-8855: improve RedisInfoStatsIntegrationTest

- organize tests
- remove tests that don't belong in this class
- fix a formatting issue in RedisCommandType
- make the RedisInfoStatsIntegrationTest run against our redis and native redis
- test all the stats in info
- add and use getExposedPort() so tests work in docker
- make hincrby behavior match that of native
redis

Co-authored-by: Jens Deppe 

> Redis Integration Test: add tests for all commands to 
> redisStatsIntegrationTest
> ---
>
> Key: GEODE-8855
> URL: https://issues.apache.org/jira/browse/GEODE-8855
> Project: Geode
>  Issue Type: Test
>  Components: redis
>Reporter: Helena Bales
>Priority: Major
>  Labels: pull-request-available
>
> add tests for the unsupported commands and the rest of the untested commands. 
> organize the tests. make the tests run against redis and native redis. remove 
> duplicate tests from the hitsmisses test.



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

[jira] [Commented] (GEODE-8855) Redis Integration Test: add tests for all commands to redisStatsIntegrationTest

[ASF subversion and git services (Jira)]

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

ASF subversion and git services commented on GEODE-8855:


Commit abddba5165ba9f0dcd72d7ac424ca67e2cfe7c45 in geode's branch 
refs/heads/develop from Hale Bales
[ https://gitbox.apache.org/repos/asf?p=geode.git;h=abddba5 ]

GEODE-8855: fix hitmiss and stats integration tests (#5937)

GEODE-8855: improve RedisInfoStatsIntegrationTest

- organize tests
- remove tests that don't belong in this class
- fix a formatting issue in RedisCommandType
- make the RedisInfoStatsIntegrationTest run against our redis and native redis
- test all the stats in info
- add and use getExposedPort() so tests work in docker
- make hincrby behavior match that of native
redis

Co-authored-by: Jens Deppe 

> Redis Integration Test: add tests for all commands to 
> redisStatsIntegrationTest
> ---
>
> Key: GEODE-8855
> URL: https://issues.apache.org/jira/browse/GEODE-8855
> Project: Geode
>  Issue Type: Test
>  Components: redis
>Reporter: Helena Bales
>Priority: Major
>  Labels: pull-request-available
>
> add tests for the unsupported commands and the rest of the untested commands. 
> organize the tests. make the tests run against redis and native redis. remove 
> duplicate tests from the hitsmisses test.



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

[jira] [Commented] (GEODE-8858) Unit/Integration tests for HSETNX command

[ASF GitHub Bot (Jira)]

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

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

sabbey37 commented on a change in pull request #5940:
URL: https://github.com/apache/geode/pull/5940#discussion_r562918840



##
File path: 
geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/hash/AbstractHashesIntegrationTest.java
##
@@ -365,6 +365,40 @@ public void testHSetNXExecutor() {
 
   }
 
+  @Test
+  public void hsetNX_shouldThrowErrorIfKeyIsWrongType() {
+String string_key = "String_Key";
+String set_key = "Set_Key";
+String field = "field";
+String value = "value";
+
+jedis.set(string_key, value);
+jedis.sadd(set_key, field);
+
+assertThatThrownBy(
+() -> jedis.hsetnx(string_key, field, "something else"))
+.isInstanceOf(JedisDataException.class)
+.hasMessageContaining("WRONGTYPE");
+assertThatThrownBy(
+() -> jedis.hsetnx(set_key, field, "something 
else")).isInstanceOf(JedisDataException.class)
+.hasMessageContaining("WRONGTYPE");
+
+jedis.del(string_key);
+jedis.del(set_key);
+  }
+
+  @Test
+  public void hsetnx_shouldThrowError_givenWrongNumberOfArguments() {
+assertThatThrownBy(() -> jedis.sendCommand(Protocol.Command.HSETNX))
+.hasMessageContaining("wrong number of arguments");
+assertThatThrownBy(() -> jedis.sendCommand(Protocol.Command.HSETNX, "1"))
+.hasMessageContaining("wrong number of arguments");
+assertThatThrownBy(() -> jedis.sendCommand(Protocol.Command.HSETNX, "1", 
"2"))
+.hasMessageContaining("wrong number of arguments");
+assertThatThrownBy(() -> jedis.sendCommand(Protocol.Command.HSETNX, "1", 
"2", "3", "4"))
+.hasMessageContaining("wrong number of arguments");
+  }

Review comment:
   @jdeppe-pivotal says he'll fix it in another PR.





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


> Unit/Integration tests for HSETNX command
> -
>
> Key: GEODE-8858
> URL: https://issues.apache.org/jira/browse/GEODE-8858
> Project: Geode
>  Issue Type: Test
>  Components: redis
>Reporter: Raymond Ingles
>Priority: Major
>  Labels: pull-request-available
>
> Flesh out unit/integration tests for Redis HSETNX command.



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

[jira] [Commented] (GEODE-8858) Unit/Integration tests for HSETNX command

[ASF GitHub Bot (Jira)]

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

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

sabbey37 merged pull request #5940:
URL: https://github.com/apache/geode/pull/5940


   



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


> Unit/Integration tests for HSETNX command
> -
>
> Key: GEODE-8858
> URL: https://issues.apache.org/jira/browse/GEODE-8858
> Project: Geode
>  Issue Type: Test
>  Components: redis
>Reporter: Raymond Ingles
>Priority: Major
>  Labels: pull-request-available
>
> Flesh out unit/integration tests for Redis HSETNX command.



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

[jira] [Commented] (GEODE-8858) Unit/Integration tests for HSETNX command

[ASF subversion and git services (Jira)]

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

ASF subversion and git services commented on GEODE-8858:


Commit 4a3f0394ccaf0f070ae5feb41856aa1a8057ca7a in geode's branch 
refs/heads/develop from Ray Ingles
[ https://gitbox.apache.org/repos/asf?p=geode.git;h=4a3f039 ]

GEODE-8858: unit/integration tests for HSETNX (#5940)



> Unit/Integration tests for HSETNX command
> -
>
> Key: GEODE-8858
> URL: https://issues.apache.org/jira/browse/GEODE-8858
> Project: Geode
>  Issue Type: Test
>  Components: redis
>Reporter: Raymond Ingles
>Priority: Major
>  Labels: pull-request-available
>
> Flesh out unit/integration tests for Redis HSETNX command.



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

[jira] [Commented] (GEODE-8837) Establish GFE_81 as the oldest supported client version.

[ASF GitHub Bot (Jira)]

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

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

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



##
File path: 
geode-serialization/src/main/java/org/apache/geode/internal/serialization/KnownVersion.java
##
@@ -367,61 +377,48 @@ public static String unsupportedVersionMessage(final 
short ordinal) {
   }
 
   public String getMethodSuffix() {
-return this.methodSuffix;
+return methodSuffix;
   }
 
   public String getName() {
-return this.name;
+return name;
   }
 
-  public short getMajorVersion() {
-return this.majorVersion;
+  public short getMajor() {
+return major;
   }
 
-  public short getMinorVersion() {
-return this.minorVersion;
+  public short getMinor() {
+return minor;
   }
 
   public short getRelease() {
-return this.release;
+return release;
   }
 
   public short getPatch() {
-return this.patch;
+return patch;
   }
 
-  /**
-   * Returns a string representation for this Version.
-   *
-   * @return the name of this operation.
-   */
   @Override
   public String toString() {
-return this.productName + " " + this.name;
-  }
-
-  public byte[] toBytes() {
-byte[] bytes = new byte[2];
-bytes[0] = (byte) (ordinal() >> 8);
-bytes[1] = (byte) ordinal();
-return bytes;
+return productName + " " + name;
   }
 
   public static Iterable getAllVersions() {
-return Arrays.asList(VALUES).stream().filter(x -> x != null && x != 
TEST_VERSION)
+return Arrays.stream(VALUES).filter(x -> x != null && x != TEST_VERSION)
 .collect(Collectors.toList());
   }
 
   /**
* package-protected for use by Versioning factory
*/
-  static KnownVersion getKnownVersionOrDefault(final short ordinal,
-  final KnownVersion defaultKnownVersion) {
+  static KnownVersion getKnownVersionOrDefault(final short ordinal) {

Review comment:
   I am not in favor of adding parameters that are unused especially when 
the use is currently in a single place and likely not going to expand beyond 
that place, given it is package private. The caller never set anything other 
than `null` meaning that if there was no known value it wanted `null` back. I 
changed the method to do what the single use caller wanted and documented the 
behavior.





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


> Establish GFE_81 as the oldest supported client version.
> 
>
> Key: GEODE-8837
> URL: https://issues.apache.org/jira/browse/GEODE-8837
> Project: Geode
>  Issue Type: Improvement
>  Components: client/server, core, cq, functions
>Reporter: Jacob Barrett
>Priority: Major
>  Labels: pull-request-available
>
> Remove support for versions older than GFE_81 in backwards compatibility 
> checks and command tables.
> Remove the unused and unreleased GFE_82 ordinal for consistency.



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

[jira] [Commented] (GEODE-8837) Establish GFE_81 as the oldest supported client version.

[ASF GitHub Bot (Jira)]

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

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

Bill commented on a change in pull request #5905:
URL: https://github.com/apache/geode/pull/5905#discussion_r562953278



##
File path: 
geode-serialization/src/main/java/org/apache/geode/internal/serialization/KnownVersion.java
##
@@ -367,61 +377,48 @@ public static String unsupportedVersionMessage(final 
short ordinal) {
   }
 
   public String getMethodSuffix() {
-return this.methodSuffix;
+return methodSuffix;
   }
 
   public String getName() {
-return this.name;
+return name;
   }
 
-  public short getMajorVersion() {
-return this.majorVersion;
+  public short getMajor() {
+return major;
   }
 
-  public short getMinorVersion() {
-return this.minorVersion;
+  public short getMinor() {
+return minor;
   }
 
   public short getRelease() {
-return this.release;
+return release;
   }
 
   public short getPatch() {
-return this.patch;
+return patch;
   }
 
-  /**
-   * Returns a string representation for this Version.
-   *
-   * @return the name of this operation.
-   */
   @Override
   public String toString() {
-return this.productName + " " + this.name;
-  }
-
-  public byte[] toBytes() {
-byte[] bytes = new byte[2];
-bytes[0] = (byte) (ordinal() >> 8);
-bytes[1] = (byte) ordinal();
-return bytes;
+return productName + " " + name;
   }
 
   public static Iterable getAllVersions() {
-return Arrays.asList(VALUES).stream().filter(x -> x != null && x != 
TEST_VERSION)
+return Arrays.stream(VALUES).filter(x -> x != null && x != TEST_VERSION)
 .collect(Collectors.toList());
   }
 
   /**
* package-protected for use by Versioning factory
*/
-  static KnownVersion getKnownVersionOrDefault(final short ordinal,
-  final KnownVersion defaultKnownVersion) {
+  static KnownVersion getKnownVersionOrDefault(final short ordinal) {

Review comment:
   ok so you're gonna rename 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


> Establish GFE_81 as the oldest supported client version.
> 
>
> Key: GEODE-8837
> URL: https://issues.apache.org/jira/browse/GEODE-8837
> Project: Geode
>  Issue Type: Improvement
>  Components: client/server, core, cq, functions
>Reporter: Jacob Barrett
>Priority: Major
>  Labels: pull-request-available
>
> Remove support for versions older than GFE_81 in backwards compatibility 
> checks and command tables.
> Remove the unused and unreleased GFE_82 ordinal for consistency.



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

[jira] [Commented] (GEODE-8837) Establish GFE_81 as the oldest supported client version.

[ASF GitHub Bot (Jira)]

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

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

Bill commented on a change in pull request #5905:
URL: https://github.com/apache/geode/pull/5905#discussion_r562955796



##
File path: 
geode-core/src/distributedTest/java/org/apache/geode/internal/BackwardCompatibilitySerializationDUnitTest.java
##
@@ -160,7 +149,7 @@ public void testFromDataFromLowerVersionToHigher() throws 
Exception {
   public void testAllMessages() throws Exception {
 // list of msgs not created using reflection
 // taken from DSFIDFactory.create()
-ArrayList constdsfids = new ArrayList();
+ArrayList constdsfids = new ArrayList<>();

Review comment:
   I'm willing to call this out of scope.





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


> Establish GFE_81 as the oldest supported client version.
> 
>
> Key: GEODE-8837
> URL: https://issues.apache.org/jira/browse/GEODE-8837
> Project: Geode
>  Issue Type: Improvement
>  Components: client/server, core, cq, functions
>Reporter: Jacob Barrett
>Priority: Major
>  Labels: pull-request-available
>
> Remove support for versions older than GFE_81 in backwards compatibility 
> checks and command tables.
> Remove the unused and unreleased GFE_82 ordinal for consistency.



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

[jira] [Commented] (GEODE-8837) Establish GFE_81 as the oldest supported client version.

[ASF GitHub Bot (Jira)]

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

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

Bill commented on a change in pull request #5905:
URL: https://github.com/apache/geode/pull/5905#discussion_r562956354



##
File path: 
geode-core/src/main/java/org/apache/geode/cache/client/internal/ClientSideHandshakeImpl.java
##
@@ -336,20 +333,17 @@ public ServerQueueStatus 
handshakeWithSubscriptionFeed(Socket sock, boolean isPr
   if (currentClientVersion.isOlderThan(KnownVersion.GFE_61)) {
 return new ServerQueueStatus(endpointType, queueSize, member);
   }
-  HashMap instantiatorMap = DataSerializer.readHashMap(dis);
-  for (Iterator itr = instantiatorMap.entrySet().iterator(); 
itr.hasNext();) {
-Map.Entry instantiator = (Map.Entry) itr.next();
-Integer id = (Integer) instantiator.getKey();
-ArrayList instantiatorArguments = (ArrayList) instantiator.getValue();
-InternalInstantiator.register((String) instantiatorArguments.get(0),
-(String) instantiatorArguments.get(1), id, false);
+  final Map> instantiatorMap = 
DataSerializer.readHashMap(dis);
+  for (final Map.Entry> entry : 
instantiatorMap.entrySet()) {

Review comment:
   just a suggestion





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


> Establish GFE_81 as the oldest supported client version.
> 
>
> Key: GEODE-8837
> URL: https://issues.apache.org/jira/browse/GEODE-8837
> Project: Geode
>  Issue Type: Improvement
>  Components: client/server, core, cq, functions
>Reporter: Jacob Barrett
>Priority: Major
>  Labels: pull-request-available
>
> Remove support for versions older than GFE_81 in backwards compatibility 
> checks and command tables.
> Remove the unused and unreleased GFE_82 ordinal for consistency.



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

[jira] [Assigned] (GEODE-8858) Unit/Integration tests for HSETNX command

[Raymond Ingles (Jira)]

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

Raymond Ingles reassigned GEODE-8858:
-

Assignee: Raymond Ingles

> Unit/Integration tests for HSETNX command
> -
>
> Key: GEODE-8858
> URL: https://issues.apache.org/jira/browse/GEODE-8858
> Project: Geode
>  Issue Type: Test
>  Components: redis
>Reporter: Raymond Ingles
>Assignee: Raymond Ingles
>Priority: Major
>  Labels: pull-request-available
>
> Flesh out unit/integration tests for Redis HSETNX command.



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

[jira] [Resolved] (GEODE-8858) Unit/Integration tests for HSETNX command

[Raymond Ingles (Jira)]

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

Raymond Ingles resolved GEODE-8858.
---
Fix Version/s: 1.14.0
   Resolution: Fixed

> Unit/Integration tests for HSETNX command
> -
>
> Key: GEODE-8858
> URL: https://issues.apache.org/jira/browse/GEODE-8858
> Project: Geode
>  Issue Type: Test
>  Components: redis
>Reporter: Raymond Ingles
>Assignee: Raymond Ingles
>Priority: Major
>  Labels: pull-request-available
> Fix For: 1.14.0
>
>
> Flesh out unit/integration tests for Redis HSETNX command.



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

[jira] [Assigned] (GEODE-8852) Add additional tests for Redis HVALS command

[Raymond Ingles (Jira)]

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

Raymond Ingles reassigned GEODE-8852:
-

Assignee: Raymond Ingles

> Add additional tests for Redis HVALS command
> 
>
> Key: GEODE-8852
> URL: https://issues.apache.org/jira/browse/GEODE-8852
> Project: Geode
>  Issue Type: Test
>  Components: redis
>Reporter: Jens Deppe
>Assignee: Raymond Ingles
>Priority: Major
>  Labels: pull-request-available
>
> Adding concurrency test as well as additional integration test



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

[jira] [Resolved] (GEODE-8852) Add additional tests for Redis HVALS command

[Raymond Ingles (Jira)]

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

Raymond Ingles resolved GEODE-8852.
---
Fix Version/s: 1.14.0
   Resolution: Fixed

> Add additional tests for Redis HVALS command
> 
>
> Key: GEODE-8852
> URL: https://issues.apache.org/jira/browse/GEODE-8852
> Project: Geode
>  Issue Type: Test
>  Components: redis
>Reporter: Jens Deppe
>Assignee: Raymond Ingles
>Priority: Major
>  Labels: pull-request-available
> Fix For: 1.14.0
>
>
> Adding concurrency test as well as additional integration test



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

[jira] [Commented] (GEODE-8837) Establish GFE_81 as the oldest supported client version.

[ASF GitHub Bot (Jira)]

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

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

Bill commented on a change in pull request #5905:
URL: https://github.com/apache/geode/pull/5905#discussion_r562960661



##
File path: 
geode-serialization/src/main/java/org/apache/geode/internal/serialization/KnownVersion.java
##
@@ -367,61 +377,48 @@ public static String unsupportedVersionMessage(final 
short ordinal) {
   }
 
   public String getMethodSuffix() {
-return this.methodSuffix;
+return methodSuffix;
   }
 
   public String getName() {
-return this.name;
+return name;
   }
 
-  public short getMajorVersion() {
-return this.majorVersion;
+  public short getMajor() {
+return major;
   }
 
-  public short getMinorVersion() {
-return this.minorVersion;
+  public short getMinor() {
+return minor;
   }
 
   public short getRelease() {
-return this.release;
+return release;
   }
 
   public short getPatch() {
-return this.patch;
+return patch;
   }
 
-  /**
-   * Returns a string representation for this Version.
-   *
-   * @return the name of this operation.
-   */
   @Override
   public String toString() {
-return this.productName + " " + this.name;
-  }
-
-  public byte[] toBytes() {
-byte[] bytes = new byte[2];
-bytes[0] = (byte) (ordinal() >> 8);
-bytes[1] = (byte) ordinal();
-return bytes;
+return productName + " " + name;
   }
 
   public static Iterable getAllVersions() {
-return Arrays.asList(VALUES).stream().filter(x -> x != null && x != 
TEST_VERSION)
+return Arrays.stream(VALUES).filter(x -> x != null && x != TEST_VERSION)
 .collect(Collectors.toList());
   }
 
   /**
* package-protected for use by Versioning factory
*/
-  static KnownVersion getKnownVersionOrDefault(final short ordinal,
-  final KnownVersion defaultKnownVersion) {
+  static KnownVersion getKnownVersionOrDefault(final short ordinal) {

Review comment:
   I see you renamed it to `getKnownVersion`. I would have preferred it if 
you had taken my suggestion: `getKnownVersionOrNull` to make it clear to the 
caller that a null could come back. While we don't do that much in Geode, I did 
try hard to eliminate nulls everywhere I could in the versioning code when I 
did the rework last year. This change introduces a null in a null-less bastion. 
   
   I'm going to accept this change anyway in the interest of harmony. Oh well.





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


> Establish GFE_81 as the oldest supported client version.
> 
>
> Key: GEODE-8837
> URL: https://issues.apache.org/jira/browse/GEODE-8837
> Project: Geode
>  Issue Type: Improvement
>  Components: client/server, core, cq, functions
>Reporter: Jacob Barrett
>Priority: Major
>  Labels: pull-request-available
>
> Remove support for versions older than GFE_81 in backwards compatibility 
> checks and command tables.
> Remove the unused and unreleased GFE_82 ordinal for consistency.



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

[jira] [Commented] (GEODE-8863) Redis: test hsetnx concurrency

[ASF GitHub Bot (Jira)]

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

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

onichols-pivotal commented on pull request #5942:
URL: https://github.com/apache/geode/pull/5942#issuecomment-765738706


   if desired to clearly see the diff before merging this, you could squash 
then rebase to latest develop



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


> Redis: test hsetnx concurrency 
> ---
>
> Key: GEODE-8863
> URL: https://issues.apache.org/jira/browse/GEODE-8863
> Project: Geode
>  Issue Type: Test
>  Components: redis
>Reporter: Helena Bales
>Assignee: Helena Bales
>Priority: Major
>  Labels: pull-request-available
>
> Write dunit tests, to launch multi-node clusters, which test multiple 
> concurrent clients accessing different servers for the following command:
> HSETNX
> A.C.
> Tests are passing, and README/redis_api_for_geode.html.md.erb updated to 
> make command "supported", or
> Stories in the backlog to fix the identified issues (with JIRA tickets) 
> and problem tests ignored



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

[jira] [Commented] (GEODE-8837) Establish GFE_81 as the oldest supported client version.

[ASF GitHub Bot (Jira)]

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

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

pivotal-jbarrett merged pull request #5905:
URL: https://github.com/apache/geode/pull/5905


   



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


> Establish GFE_81 as the oldest supported client version.
> 
>
> Key: GEODE-8837
> URL: https://issues.apache.org/jira/browse/GEODE-8837
> Project: Geode
>  Issue Type: Improvement
>  Components: client/server, core, cq, functions
>Reporter: Jacob Barrett
>Priority: Major
>  Labels: pull-request-available
>
> Remove support for versions older than GFE_81 in backwards compatibility 
> checks and command tables.
> Remove the unused and unreleased GFE_82 ordinal for consistency.



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

[jira] [Commented] (GEODE-8837) Establish GFE_81 as the oldest supported client version.

[ASF subversion and git services (Jira)]

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

ASF subversion and git services commented on GEODE-8837:


Commit 854456c81ca7b9545eba252b6fa075318bb33af8 in geode's branch 
refs/heads/develop from Jacob Barrett
[ https://gitbox.apache.org/repos/asf?p=geode.git;h=854456c ]

GEODE-8837: Establishes GFE_81 as the oldest supported client. (#5905)

* Deprecate old Versions.
* Remove old gossip from TcpClient.
* Removes old commands from command map.
* Removes GFE_56 completely as example.
* Removes obsolete ContainsKey as example.
* Removes obsolete Commands.
* Extracts CommandRegistry interface for testing.
* Adds exception if commands fail to register.
* Tests that CQ commands can register.
* Removes obsolete CQ command.
* Refactor CommandInitializer for more immutability and thread safety.
* Removed unreleased GFE_82 version.


> Establish GFE_81 as the oldest supported client version.
> 
>
> Key: GEODE-8837
> URL: https://issues.apache.org/jira/browse/GEODE-8837
> Project: Geode
>  Issue Type: Improvement
>  Components: client/server, core, cq, functions
>Reporter: Jacob Barrett
>Priority: Major
>  Labels: pull-request-available
>
> Remove support for versions older than GFE_81 in backwards compatibility 
> checks and command tables.
> Remove the unused and unreleased GFE_82 ordinal for consistency.



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

[jira] [Updated] (GEODE-8837) Establish GFE_81 as the oldest supported client version.

[Jacob Barrett (Jira)]

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

Jacob Barrett updated GEODE-8837:
-
Fix Version/s: 1.14.0

> Establish GFE_81 as the oldest supported client version.
> 
>
> Key: GEODE-8837
> URL: https://issues.apache.org/jira/browse/GEODE-8837
> Project: Geode
>  Issue Type: Improvement
>  Components: client/server, core, cq, functions
>Reporter: Jacob Barrett
>Priority: Major
>  Labels: pull-request-available
> Fix For: 1.14.0
>
>
> Remove support for versions older than GFE_81 in backwards compatibility 
> checks and command tables.
> Remove the unused and unreleased GFE_82 ordinal for consistency.



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

[jira] [Resolved] (GEODE-8837) Establish GFE_81 as the oldest supported client version.

[Jacob Barrett (Jira)]

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

Jacob Barrett resolved GEODE-8837.
--
Resolution: Fixed

> Establish GFE_81 as the oldest supported client version.
> 
>
> Key: GEODE-8837
> URL: https://issues.apache.org/jira/browse/GEODE-8837
> Project: Geode
>  Issue Type: Improvement
>  Components: client/server, core, cq, functions
>Reporter: Jacob Barrett
>Priority: Major
>  Labels: pull-request-available
> Fix For: 1.14.0
>
>
> Remove support for versions older than GFE_81 in backwards compatibility 
> checks and command tables.
> Remove the unused and unreleased GFE_82 ordinal for consistency.



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

[jira] [Created] (GEODE-8870) Remove obsolete version compatibility code

[Jacob Barrett (Jira)]

Jacob Barrett created GEODE-8870:


 Summary: Remove obsolete version compatibility code
 Key: GEODE-8870
 URL: https://issues.apache.org/jira/browse/GEODE-8870
 Project: Geode
  Issue Type: Improvement
Reporter: Jacob Barrett


As a followup to GEODE-8837 remove all obsolete backwards compatibility code.

This ticket will catch all changes to remove the obsolete code.



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

[jira] [Assigned] (GEODE-8870) Remove obsolete version compatibility code

[Jacob Barrett (Jira)]

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

Jacob Barrett reassigned GEODE-8870:


Assignee: Jacob Barrett

> Remove obsolete version compatibility code
> --
>
> Key: GEODE-8870
> URL: https://issues.apache.org/jira/browse/GEODE-8870
> Project: Geode
>  Issue Type: Improvement
>Reporter: Jacob Barrett
>Assignee: Jacob Barrett
>Priority: Major
>
> As a followup to GEODE-8837 remove all obsolete backwards compatibility code.
> This ticket will catch all changes to remove the obsolete code.



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

[jira] [Created] (GEODE-8871) Parse server replies to PUT and CONTAINS_KEY messages in gnmsg

[Blake Bender (Jira)]

Blake Bender created GEODE-8871:
---

 Summary: Parse server replies to PUT and CONTAINS_KEY messages in 
gnmsg
 Key: GEODE-8871
 URL: https://issues.apache.org/jira/browse/GEODE-8871
 Project: Geode
  Issue Type: Improvement
  Components: native client
Reporter: Blake Bender


As a developer, I would like to be able to see information returned from the 
server in response to common message for debugging purposes.   PUT and 
CONTAINS_KEY messages are a decent place to start, and correspond to certain 
flavors of REPLY and RESPONSE server messages, respectively.  Seeing the 
information in these server messages may be helpful in gnmsg output.



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

[jira] [Assigned] (GEODE-8871) Parse server replies to PUT and CONTAINS_KEY messages in gnmsg

[Blake Bender (Jira)]

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

Blake Bender reassigned GEODE-8871:
---

Assignee: Blake Bender

> Parse server replies to PUT and CONTAINS_KEY messages in gnmsg
> --
>
> Key: GEODE-8871
> URL: https://issues.apache.org/jira/browse/GEODE-8871
> Project: Geode
>  Issue Type: Improvement
>  Components: native client
>Reporter: Blake Bender
>Assignee: Blake Bender
>Priority: Major
>
> As a developer, I would like to be able to see information returned from the 
> server in response to common message for debugging purposes.   PUT and 
> CONTAINS_KEY messages are a decent place to start, and correspond to certain 
> flavors of REPLY and RESPONSE server messages, respectively.  Seeing the 
> information in these server messages may be helpful in gnmsg output.



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

[jira] [Commented] (GEODE-8870) Remove obsolete version compatibility code

[ASF GitHub Bot (Jira)]

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

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

lgtm-com[bot] commented on pull request #5947:
URL: https://github.com/apache/geode/pull/5947#issuecomment-765804285


   This pull request **introduces 2 alerts** when merging 
da6d3986dcd6552398b5e405ba1240c3159444fa into 
854456c81ca7b9545eba252b6fa075318bb33af8 - [view on 
LGTM.com](https://lgtm.com/projects/g/apache/geode/rev/pr-4ea3aa465551e076ef1ff29a0a0d3e22ae7bb046)
   
   **new alerts:**
   
   * 1 for Useless comparison test
   * 1 for Container size compared to zero



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 obsolete version compatibility code
> --
>
> Key: GEODE-8870
> URL: https://issues.apache.org/jira/browse/GEODE-8870
> Project: Geode
>  Issue Type: Improvement
>Reporter: Jacob Barrett
>Assignee: Jacob Barrett
>Priority: Major
>
> As a followup to GEODE-8837 remove all obsolete backwards compatibility code.
> This ticket will catch all changes to remove the obsolete code.



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

[jira] [Updated] (GEODE-8870) Remove obsolete version compatibility code

[ASF GitHub Bot (Jira)]

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

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

> Remove obsolete version compatibility code
> --
>
> Key: GEODE-8870
> URL: https://issues.apache.org/jira/browse/GEODE-8870
> Project: Geode
>  Issue Type: Improvement
>Reporter: Jacob Barrett
>Assignee: Jacob Barrett
>Priority: Major
>  Labels: pull-request-available
>
> As a followup to GEODE-8837 remove all obsolete backwards compatibility code.
> This ticket will catch all changes to remove the obsolete code.



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

[jira] [Commented] (GEODE-8207) Enforce No Unknown Pragmas as Error

[ASF GitHub Bot (Jira)]

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

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

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


   Since https://github.com/apache/geode-native/pull/608 got rid of this 
warning on clang, we can get rid of it on windows.  Though let me know if I 
really should make a new jira instead of reusing the old one



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


> Enforce No Unknown Pragmas as Error
> ---
>
> Key: GEODE-8207
> URL: https://issues.apache.org/jira/browse/GEODE-8207
> Project: Geode
>  Issue Type: Improvement
>  Components: native client
>Reporter: Michael Oleske
>Priority: Major
> Fix For: 1.14.0
>
>
> Given I compile the code without exempting no-unknown-pragmas
> Then it should compile
> Note - was marked as a todo



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

[jira] [Updated] (GEODE-8207) Enforce No Unknown Pragmas as Error

[ASF GitHub Bot (Jira)]

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

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

> Enforce No Unknown Pragmas as Error
> ---
>
> Key: GEODE-8207
> URL: https://issues.apache.org/jira/browse/GEODE-8207
> Project: Geode
>  Issue Type: Improvement
>  Components: native client
>Reporter: Michael Oleske
>Priority: Major
>  Labels: pull-request-available
> Fix For: 1.14.0
>
>
> Given I compile the code without exempting no-unknown-pragmas
> Then it should compile
> Note - was marked as a todo



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

[jira] [Commented] (GEODE-8543) Client disconnects from server due to exception on other server

[ASF GitHub Bot (Jira)]

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

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

codecov-io edited a comment on pull request #705:
URL: https://github.com/apache/geode-native/pull/705#issuecomment-754919087


   # [Codecov](https://codecov.io/gh/apache/geode-native/pull/705?src=pr&el=h1) 
Report
   > Merging 
[#705](https://codecov.io/gh/apache/geode-native/pull/705?src=pr&el=desc) 
(12d7a95) into 
[develop](https://codecov.io/gh/apache/geode-native/commit/4dd2ceb9ef0dd081039bad76cdb78a3a6cc0a2fd?el=desc)
 (4dd2ceb) will **decrease** coverage by `0.02%`.
   > The diff coverage is `100.00%`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/geode-native/pull/705/graphs/tree.svg?width=650&height=150&src=pr&token=plpAqoqGag)](https://codecov.io/gh/apache/geode-native/pull/705?src=pr&el=tree)
   
   ```diff
   @@ Coverage Diff @@
   ##   develop #705  +/-   ##
   ===
   - Coverage74.09%   74.06%   -0.03% 
   ===
 Files  642  642  
 Lines5081250812  
   ===
   - Hits 3764737633  -14 
   - Misses   1316513179  +14 
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/geode-native/pull/705?src=pr&el=tree) | 
Coverage Δ | |
   |---|---|---|
   | 
[cppcache/src/ThinClientBaseDM.cpp](https://codecov.io/gh/apache/geode-native/pull/705/diff?src=pr&el=tree#diff-Y3BwY2FjaGUvc3JjL1RoaW5DbGllbnRCYXNlRE0uY3Bw)
 | `68.33% <100.00%> (-0.56%)` | :arrow_down: |
   | 
[...test/testThinClientPoolExecuteHAFunctionPrSHOP.cpp](https://codecov.io/gh/apache/geode-native/pull/705/diff?src=pr&el=tree#diff-Y3BwY2FjaGUvaW50ZWdyYXRpb24tdGVzdC90ZXN0VGhpbkNsaWVudFBvb2xFeGVjdXRlSEFGdW5jdGlvblByU0hPUC5jcHA=)
 | `91.20% <0.00%> (-3.71%)` | :arrow_down: |
   | 
[cppcache/src/ClientMetadataService.cpp](https://codecov.io/gh/apache/geode-native/pull/705/diff?src=pr&el=tree#diff-Y3BwY2FjaGUvc3JjL0NsaWVudE1ldGFkYXRhU2VydmljZS5jcHA=)
 | `62.92% <0.00%> (-2.75%)` | :arrow_down: |
   | 
[cppcache/src/ExecutionImpl.cpp](https://codecov.io/gh/apache/geode-native/pull/705/diff?src=pr&el=tree#diff-Y3BwY2FjaGUvc3JjL0V4ZWN1dGlvbkltcGwuY3Bw)
 | `69.92% <0.00%> (-0.40%)` | :arrow_down: |
   | 
[cppcache/src/ThinClientRedundancyManager.cpp](https://codecov.io/gh/apache/geode-native/pull/705/diff?src=pr&el=tree#diff-Y3BwY2FjaGUvc3JjL1RoaW5DbGllbnRSZWR1bmRhbmN5TWFuYWdlci5jcHA=)
 | `75.74% <0.00%> (-0.16%)` | :arrow_down: |
   | 
[cppcache/src/ThinClientRegion.cpp](https://codecov.io/gh/apache/geode-native/pull/705/diff?src=pr&el=tree#diff-Y3BwY2FjaGUvc3JjL1RoaW5DbGllbnRSZWdpb24uY3Bw)
 | `56.28% <0.00%> (-0.06%)` | :arrow_down: |
   | 
[cppcache/src/ThinClientPoolDM.cpp](https://codecov.io/gh/apache/geode-native/pull/705/diff?src=pr&el=tree#diff-Y3BwY2FjaGUvc3JjL1RoaW5DbGllbnRQb29sRE0uY3Bw)
 | `77.38% <0.00%> (+0.07%)` | :arrow_up: |
   | 
[cppcache/src/ThinClientLocatorHelper.cpp](https://codecov.io/gh/apache/geode-native/pull/705/diff?src=pr&el=tree#diff-Y3BwY2FjaGUvc3JjL1RoaW5DbGllbnRMb2NhdG9ySGVscGVyLmNwcA==)
 | `93.57% <0.00%> (+0.71%)` | :arrow_up: |
   | 
[cppcache/src/TcrEndpoint.cpp](https://codecov.io/gh/apache/geode-native/pull/705/diff?src=pr&el=tree#diff-Y3BwY2FjaGUvc3JjL1RjckVuZHBvaW50LmNwcA==)
 | `56.47% <0.00%> (+0.98%)` | :arrow_up: |
   | ... and [1 
more](https://codecov.io/gh/apache/geode-native/pull/705/diff?src=pr&el=tree-more)
 | |
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/geode-native/pull/705?src=pr&el=continue).
   > **Legend** - [Click here to learn 
more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute  (impact)`, `ø = not affected`, `? = missing data`
   > Powered by 
[Codecov](https://codecov.io/gh/apache/geode-native/pull/705?src=pr&el=footer). 
Last update 
[4dd2ceb...12d7a95](https://codecov.io/gh/apache/geode-native/pull/705?src=pr&el=lastupdated).
 Read the [comment docs](https://docs.codecov.io/docs/pull-request-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


> Client disconnects from server due to exception on other server
> ---
>
> Key: GEODE-8543
> URL: https://issues.apache.org/jira/browse/GEODE-8543
> Project: Geode
>  Issue Type: Bug
>  Components: native client
>Reporter: Alberto Busta

[jira] [Commented] (GEODE-8845) Add Missing public Declarations for .NET Cacheables

[ASF GitHub Bot (Jira)]

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

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

codecov-io edited a comment on pull request #724:
URL: https://github.com/apache/geode-native/pull/724#issuecomment-762443413


   # [Codecov](https://codecov.io/gh/apache/geode-native/pull/724?src=pr&el=h1) 
Report
   > Merging 
[#724](https://codecov.io/gh/apache/geode-native/pull/724?src=pr&el=desc) 
(e40034c) into 
[develop](https://codecov.io/gh/apache/geode-native/commit/c1886061ff94e328d4675206b34c3a4868510600?el=desc)
 (c188606) will **decrease** coverage by `0.06%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/geode-native/pull/724/graphs/tree.svg?width=650&height=150&src=pr&token=plpAqoqGag)](https://codecov.io/gh/apache/geode-native/pull/724?src=pr&el=tree)
   
   ```diff
   @@ Coverage Diff @@
   ##   develop #724  +/-   ##
   ===
   - Coverage74.08%   74.02%   -0.07% 
   ===
 Files  645  645  
 Lines5085850858  
   ===
   - Hits 3768037646  -34 
   - Misses   1317813212  +34 
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/geode-native/pull/724?src=pr&el=tree) | 
Coverage Δ | |
   |---|---|---|
   | 
[cppcache/src/ReadWriteLock.cpp](https://codecov.io/gh/apache/geode-native/pull/724/diff?src=pr&el=tree#diff-Y3BwY2FjaGUvc3JjL1JlYWRXcml0ZUxvY2suY3Bw)
 | `81.25% <0.00%> (-18.75%)` | :arrow_down: |
   | 
[cppcache/src/TcrEndpoint.cpp](https://codecov.io/gh/apache/geode-native/pull/724/diff?src=pr&el=tree#diff-Y3BwY2FjaGUvc3JjL1RjckVuZHBvaW50LmNwcA==)
 | `54.36% <0.00%> (-2.82%)` | :arrow_down: |
   | 
[cppcache/src/TcrConnection.cpp](https://codecov.io/gh/apache/geode-native/pull/724/diff?src=pr&el=tree#diff-Y3BwY2FjaGUvc3JjL1RjckNvbm5lY3Rpb24uY3Bw)
 | `71.11% <0.00%> (-1.67%)` | :arrow_down: |
   | 
[cppcache/src/ClientMetadata.cpp](https://codecov.io/gh/apache/geode-native/pull/724/diff?src=pr&el=tree#diff-Y3BwY2FjaGUvc3JjL0NsaWVudE1ldGFkYXRhLmNwcA==)
 | `65.16% <0.00%> (-0.57%)` | :arrow_down: |
   | 
[cppcache/src/ThinClientPoolDM.cpp](https://codecov.io/gh/apache/geode-native/pull/724/diff?src=pr&el=tree#diff-Y3BwY2FjaGUvc3JjL1RoaW5DbGllbnRQb29sRE0uY3Bw)
 | `76.31% <0.00%> (-0.43%)` | :arrow_down: |
   | 
[cppcache/src/RemoteQuery.cpp](https://codecov.io/gh/apache/geode-native/pull/724/diff?src=pr&el=tree#diff-Y3BwY2FjaGUvc3JjL1JlbW90ZVF1ZXJ5LmNwcA==)
 | `76.59% <0.00%> (+1.06%)` | :arrow_up: |
   | 
[...tion-test/testThinClientRemoteQueryFailoverPdx.cpp](https://codecov.io/gh/apache/geode-native/pull/724/diff?src=pr&el=tree#diff-Y3BwY2FjaGUvaW50ZWdyYXRpb24tdGVzdC90ZXN0VGhpbkNsaWVudFJlbW90ZVF1ZXJ5RmFpbG92ZXJQZHguY3Bw)
 | `89.51% <0.00%> (+4.03%)` | :arrow_up: |
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/geode-native/pull/724?src=pr&el=continue).
   > **Legend** - [Click here to learn 
more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute  (impact)`, `ø = not affected`, `? = missing data`
   > Powered by 
[Codecov](https://codecov.io/gh/apache/geode-native/pull/724?src=pr&el=footer). 
Last update 
[c188606...e40034c](https://codecov.io/gh/apache/geode-native/pull/724?src=pr&el=lastupdated).
 Read the [comment docs](https://docs.codecov.io/docs/pull-request-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


> Add Missing public Declarations for .NET Cacheables
> ---
>
> Key: GEODE-8845
> URL: https://issues.apache.org/jira/browse/GEODE-8845
> Project: Geode
>  Issue Type: Bug
>  Components: native client
>Reporter: Michael Martell
>Priority: Minor
>  Labels: pull-request-available
>
> The .NET APIs provide Cacheable types (e.g. CacheableString) for the standard 
> .NET types. However, several of them are missing the public declarations, 
> making them inaccessible. 
> These classes provide a hashCode() function that matches the corresponding 
> hashCode on the server side. It would be nice to be able to leverage these 
> functions when using these types in a key class. I discovered this when 
> implementing a new ClassAsKey example.



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

[jira] [Commented] (GEODE-8830) Exceptions not correctly re-thrown in the tx manager

[ASF GitHub Bot (Jira)]

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

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

codecov-io edited a comment on pull request #720:
URL: https://github.com/apache/geode-native/pull/720#issuecomment-760355579


   # [Codecov](https://codecov.io/gh/apache/geode-native/pull/720?src=pr&el=h1) 
Report
   > Merging 
[#720](https://codecov.io/gh/apache/geode-native/pull/720?src=pr&el=desc) 
(4ca9432) into 
[develop](https://codecov.io/gh/apache/geode-native/commit/ec823095d0efe74e025d773b7bffefadc9261f70?el=desc)
 (ec82309) will **decrease** coverage by `0.04%`.
   > The diff coverage is `100.00%`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/geode-native/pull/720/graphs/tree.svg?width=650&height=150&src=pr&token=plpAqoqGag)](https://codecov.io/gh/apache/geode-native/pull/720?src=pr&el=tree)
   
   ```diff
   @@ Coverage Diff @@
   ##   develop #720  +/-   ##
   ===
   - Coverage75.05%   75.00%   -0.05% 
   ===
 Files  645  645  
 Lines4977949779  
   ===
   - Hits 3736037336  -24 
   - Misses   1241912443  +24 
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/geode-native/pull/720?src=pr&el=tree) | 
Coverage Δ | |
   |---|---|---|
   | 
[...che/src/InternalCacheTransactionManager2PCImpl.cpp](https://codecov.io/gh/apache/geode-native/pull/720/diff?src=pr&el=tree#diff-Y3BwY2FjaGUvc3JjL0ludGVybmFsQ2FjaGVUcmFuc2FjdGlvbk1hbmFnZXIyUENJbXBsLmNwcA==)
 | `74.11% <100.00%> (+11.76%)` | :arrow_up: |
   | 
[cppcache/src/ReadWriteLock.cpp](https://codecov.io/gh/apache/geode-native/pull/720/diff?src=pr&el=tree#diff-Y3BwY2FjaGUvc3JjL1JlYWRXcml0ZUxvY2suY3Bw)
 | `81.25% <0.00%> (-18.75%)` | :arrow_down: |
   | 
[...tion-test/testThinClientRemoteQueryFailoverPdx.cpp](https://codecov.io/gh/apache/geode-native/pull/720/diff?src=pr&el=tree#diff-Y3BwY2FjaGUvaW50ZWdyYXRpb24tdGVzdC90ZXN0VGhpbkNsaWVudFJlbW90ZVF1ZXJ5RmFpbG92ZXJQZHguY3Bw)
 | `85.48% <0.00%> (-4.04%)` | :arrow_down: |
   | 
[cppcache/src/ThinClientBaseDM.cpp](https://codecov.io/gh/apache/geode-native/pull/720/diff?src=pr&el=tree#diff-Y3BwY2FjaGUvc3JjL1RoaW5DbGllbnRCYXNlRE0uY3Bw)
 | `69.09% <0.00%> (-2.43%)` | :arrow_down: |
   | 
[cppcache/src/PdxTypeRegistry.cpp](https://codecov.io/gh/apache/geode-native/pull/720/diff?src=pr&el=tree#diff-Y3BwY2FjaGUvc3JjL1BkeFR5cGVSZWdpc3RyeS5jcHA=)
 | `79.10% <0.00%> (-2.24%)` | :arrow_down: |
   | 
[cppcache/src/TcrMessage.hpp](https://codecov.io/gh/apache/geode-native/pull/720/diff?src=pr&el=tree#diff-Y3BwY2FjaGUvc3JjL1Rjck1lc3NhZ2UuaHBw)
 | `98.00% <0.00%> (-2.00%)` | :arrow_down: |
   | 
[cppcache/src/CacheTransactionManagerImpl.cpp](https://codecov.io/gh/apache/geode-native/pull/720/diff?src=pr&el=tree#diff-Y3BwY2FjaGUvc3JjL0NhY2hlVHJhbnNhY3Rpb25NYW5hZ2VySW1wbC5jcHA=)
 | `81.72% <0.00%> (-1.62%)` | :arrow_down: |
   | 
[cppcache/src/RemoteQuery.cpp](https://codecov.io/gh/apache/geode-native/pull/720/diff?src=pr&el=tree#diff-Y3BwY2FjaGUvc3JjL1JlbW90ZVF1ZXJ5LmNwcA==)
 | `78.02% <0.00%> (-1.10%)` | :arrow_down: |
   | 
[cppcache/src/ThinClientPoolDM.cpp](https://codecov.io/gh/apache/geode-native/pull/720/diff?src=pr&el=tree#diff-Y3BwY2FjaGUvc3JjL1RoaW5DbGllbnRQb29sRE0uY3Bw)
 | `76.98% <0.00%> (-0.98%)` | :arrow_down: |
   | 
[cppcache/src/LRUEntriesMap.cpp](https://codecov.io/gh/apache/geode-native/pull/720/diff?src=pr&el=tree#diff-Y3BwY2FjaGUvc3JjL0xSVUVudHJpZXNNYXAuY3Bw)
 | `60.17% <0.00%> (-0.44%)` | :arrow_down: |
   | ... and [7 
more](https://codecov.io/gh/apache/geode-native/pull/720/diff?src=pr&el=tree-more)
 | |
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/geode-native/pull/720?src=pr&el=continue).
   > **Legend** - [Click here to learn 
more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute  (impact)`, `ø = not affected`, `? = missing data`
   > Powered by 
[Codecov](https://codecov.io/gh/apache/geode-native/pull/720?src=pr&el=footer). 
Last update 
[ec82309...4ca9432](https://codecov.io/gh/apache/geode-native/pull/720?src=pr&el=lastupdated).
 Read the [comment docs](https://docs.codecov.io/docs/pull-request-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


> Exceptions not correctly re-thrown in the tx manager
> 
>
> Key: GEODE-8830
>