[GitHub] [geode-native] jvarenina opened a new pull request #656: GEODE-8530: Fix for coredump during tx commit
jvarenina opened a new pull request #656: URL: https://github.com/apache/geode-native/pull/656 Client is fixed in a way that ignores all regions enlisted within transaction that client doesn't have configured. This behavior is aligned with the java client, since same thing is done there. 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
[GitHub] [geode] kohlmu-pivotal closed pull request #5390: ClassLoader isolation
kohlmu-pivotal closed pull request #5390: URL: https://github.com/apache/geode/pull/5390 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
[GitHub] [geode] Bill commented on pull request #5545: GEODE-8522: Switching a exception log back to debug
Bill commented on pull request #5545:
URL: https://github.com/apache/geode/pull/5545#issuecomment-698654480
@upthewaterspout would this be an improvement:
```
logger.info("Unable to contact a locator: " +
problem.getMessage());
```
That way we pass along the content of the exception but do not print a stack
trace. But we keep it at info level.
Thinking a bit more: the causes and ramifications of the `IOException` seem
very different from the `ClassNotFoundException` exception. I would expect the
former but not the latter in your normal startup case.
If we think the stack trace would be useful in the `ClassNotFoundException`
case we could handle that in a separate catch block.
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
[GitHub] [geode] dschneider-pivotal merged pull request #5544: GEODE-8515: Redis PING should respond appropriately when called from within a SUBSCRIBE/PSUBSCRIBE
dschneider-pivotal merged pull request #5544: URL: https://github.com/apache/geode/pull/5544 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
[GitHub] [geode] upthewaterspout commented on pull request #5545: GEODE-8522: Switching a exception log back to debug
upthewaterspout commented on pull request #5545: URL: https://github.com/apache/geode/pull/5545#issuecomment-698644756 @Bill - Regarding `locator-wait-time`, `locator-wait-time` used to be something that only took effect on servers, not locators. That changed fairly recently. It's still safe to start up multiple locators that all refer to each other without locator-wait-time - provided they can actually reach each other on startup! We have had some issues in K8s environments because there is race between when a locator tries to contact another locator and when the K8s DNS makes the locator name available. Good catch on those docs! Yeah, I don't see that string in the products since jgroups was removed in 2015. This particular log message *used* to be `debug` - you and I flipped it to info in this change - 53f1e1a81c3b58989a835d37f94466eb3dfc752f. I don't mind it being an info message - but I think we shouldn't be logging a stack trace in that case. Maybe just an info message that we failed to contact a particular locator. Let me know if you'd like me to make that change. Either way, I'll create a separate docs PR after we have this figured out. 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
[GitHub] [geode] Bill commented on pull request #5537: Bump archunit from 0.12.0 to 0.14.1
Bill commented on pull request #5537: URL: https://github.com/apache/geode/pull/5537#issuecomment-698639378 whacked another mole 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
[GitHub] [geode] Bill commented on pull request #5537: Bump archunit from 0.12.0 to 0.14.1
Bill commented on pull request #5537: URL: https://github.com/apache/geode/pull/5537#issuecomment-698631892 I pushed three commits that I think address all the issues @kirklund mentioned in comments above. A better longer-term fix would be to define "layers" for each of our, ahem, layers. For now I just added allowances for the annotations package in the `geode-commons` "module". 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
[GitHub] [geode] jhutchison opened a new pull request #5551: Explore pipeling
jhutchison opened a new pull request #5551: URL: https://github.com/apache/geode/pull/5551 Thank you for submitting a contribution to Apache Geode. In order to streamline the review of the contribution we ask you to ensure the following steps have been taken: ### For all changes: - [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message? - [ ] Has your PR been rebased against the latest commit within the target branch (typically `develop`)? - [ ] Is your initial contribution a single, squashed commit? - [ ] Does `gradlew build` run cleanly? - [ ] Have you written or updated unit tests to verify your changes? - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? ### Note: Please ensure that once the PR is submitted, check Concourse for build issues and submit an update to your PR as soon as possible. If you need help, please send an email to d...@geode.apache.org. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] kirklund commented on pull request #5537: Bump archunit from 0.12.0 to 0.14.1
kirklund commented on pull request #5537: URL: https://github.com/apache/geode/pull/5537#issuecomment-69865 IntegrationTest failures: ``` > Task :geode-membership:integrationTest org.apache.geode.distributed.internal.membership.api.MembershipAPIArchUnitTest > membershipAPIDoesntDependOnMembershipORCore FAILED java.lang.AssertionError: Architecture Violation [Priority: MEDIUM] - Rule 'classes that reside in a package 'org.apache.geode.distributed.internal.membership.api' should only depend on classes that reside in a package 'org.apache.geode.distributed.internal.membership.api' or not reside in a package 'org.apache.geode..' or reside in a package 'org.apache.geode.internal.serialization..' or reside in a package 'org.apache.geode.distributed.internal.tcpserver..' or type org.apache.geode.distributed.internal.membership.gms.MembershipBuilderImpl or type org.apache.geode.distributed.internal.membership.gms.MembershipLocatorBuilderImpl or type org.apache.geode.distributed.internal.membership.gms.MemberDataBuilderImpl or type org.apache.geode.distributed.internal.membership.gms.MemberIdentifierImpl or type org.apache.geode.internal.inet.LocalHostUtil' was violated (2 times): Field is annotated with in (MembershipConfig.java:0) Field is annotated with in (Message.java:0) ``` ``` > Task :geode-membership:integrationTest org.apache.geode.distributed.internal.membership.MembershipDependenciesJUnitTest > membershipDoesntDependOnCoreProvisional FAILED java.lang.AssertionError: Architecture Violation [Priority: MEDIUM] - Rule 'classes that reside in a package 'org.apache.geode.distributed.internal.membership.gms..' should only depend on classes that reside in a package 'org.apache.geode.distributed.internal.membership.gms..' or reside in a package 'org.apache.geode.distributed.internal.membership.api..' or reside in a package 'org.apache.geode.internal.serialization..' or reside in a package 'org.apache.geode.logging.internal..' or reside in a package 'org.apache.geode.distributed.internal.tcpserver..' or reside in a package 'org.apache.geode.internal.inet..' or reside in a package 'org.apache.geode.internal.lang..' or not reside in a package 'org.apache.geode..' or type org.apache.geode.internal.AvailablePortHelper or reside in a package 'org.apache.geode.test..'' was violated (14 times): Field is annotated with in (GMSMembership.java:0) Field is annotated with in (GMSMembershipView.java:0) Field is annotated with in (MemberIdentifierImpl.java:0) Field is annotated with in (GMSJoinLeave.java:0) Field is annotated with in (GMSJoinLeave.java:0) Method is annotated with in (GMSMembership.java:0) Method is annotated with in (MemberIdentifierImpl.java:0) Method is annotated with in (Services.java:0) Method is annotated with in (GMSLocator.java:0) Method is annotated with in (GMSLocator.java:0) Method is annotated with in (GMSLocator.java:0) Method is annotated with in (MembershipLocatorImpl.java:0) Method is annotated with in (GMSJoinLeave.java:0) Method is annotated with in (GMSJoinLeave.java:0) ``` ``` > Task :geode-membership:integrationTest FAILED timeout exceeded ``` 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
[GitHub] [geode] Bill commented on pull request #5545: GEODE-8522: Switching a exception log back to debug
Bill commented on pull request #5545: URL: https://github.com/apache/geode/pull/5545#issuecomment-698610478 @upthewaterspout so when two locators are starting up simultaneously I suppose `locatorClient.requestToServer()` is throwing `IOException` (because some other locator on `locators` list hasn't started yet.) Your comments imply that starting two locators at once is "normal". [Looking at the docs](https://geode.apache.org/docs/guide/12/configuring/running/starting_up_shutting_down.html), I suppose it is, provided you specify `locator-wait-time` (so as to avoid split-brain.) Oddly, the docs mention: > …an info-level message > > GemFire startup was unable to contact a locator. Waiting for one to start. Configured locators are frodo[12345],pippin[12345]. I don't find that message text anywhere in the Geode source today (searched for "unable to contact a locator" and "Configured locators are"). It makes me think that the docs might talking about (an old version) of the very message that this PR is changing (from info level down to debug level.) The logged message, on the `develop` branch and in this PR (unchanged) is: > Exception thrown when contacting a locator If that's right (that the docs are talking about the message modified by this PR), then we need a doc change to go with this code change. 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
[GitHub] [geode] kirklund commented on pull request #5537: Bump archunit from 0.12.0 to 0.14.1
kirklund commented on pull request #5537: URL: https://github.com/apache/geode/pull/5537#issuecomment-698610435 UnitTest failures: ``` > Task :geode-serialization:test org.apache.geode.internal.serialization.SerializationDependenciesJUnitTest > serializationDoesntDependOnCoreProvisional FAILED java.lang.AssertionError: Architecture Violation [Priority: MEDIUM] - Rule 'classes that reside in a package 'org.apache.geode.internal.serialization..' should only depend on classes that reside in a package 'org.apache.geode.internal.serialization..' or not reside in a package 'org.apache.geode..' or reside in a package 'org.apache.geode.test..'' was violated (45 times): Class is annotated with in (KnownVersion.java:0) Field is annotated with in (DscodeHelper.java:0) Field is annotated with in (KnownVersion.java:0) Field is annotated with in (KnownVersion.java:0) Field is annotated with in (KnownVersion.java:0) Field is annotated with in (KnownVersion.java:0) Field is annotated with in (KnownVersion.java:0) Field is annotated with in (KnownVersion.java:0) Field is annotated with in (KnownVersion.java:0) Field is annotated with in (KnownVersion.java:0) Field is annotated with in (KnownVersion.java:0) Field is annotated with in (KnownVersion.java:0) Field is annotated with in (KnownVersion.java:0) Field is annotated with in (KnownVersion.java:0) Field is annotated with in (KnownVersion.java:0) Field is annotated with in (KnownVersion.java:0) Field is annotated with in (KnownVersion.java:0) Field is annotated with in (KnownVersion.java:0) Field is annotated with in (KnownVersion.java:0) Field is annotated with in (KnownVersion.java:0) Field is annotated with in (KnownVersion.java:0) Field is annotated with in (KnownVersion.java:0) Field is annotated with in (KnownVersion.java:0) Field is annotated with in (KnownVersion.java:0) Field is annotated with in (KnownVersion.java:0) Field is annotated with in (KnownVersion.java:0) Field is annotated with in (KnownVersion.java:0) Field is annotated with in (KnownVersion.java:0) Field is annotated with in (KnownVersion.java:0) Field is annotated with in (KnownVersion.java:0) Field is annotated with in (KnownVersion.java:0) Field is annotated with in (KnownVersion.java:0) Field is annotated with in (KnownVersion.java:0) Field is annotated with in (KnownVersion.java:0) Field is annotated with in (KnownVersion.java:0) Field is annotated with in (KnownVersion.java:0) Field is annotated with in (KnownVersion.java:0) Field is annotated with in (KnownVersion.java:0) Field is annotated with in (KnownVersion.java:0) Field is annotated with in (KnownVersion.java:0) Field is annotated with in (KnownVersion.java:0) Field is annotated with in (KnownVersion.java:0) Field is annotated with in (StaticSerialization.java:0) Field is annotated with in (DSFIDSerializerImpl.java:0) Field is annotated with in (DSFIDSerializerImpl.java:0) > Task :geode-serialization:test FAILED ``` ``` > Task :geode-tcp-server:test org.apache.geode.distributed.internal.tcpserver.TcpServerDependenciesTest > membershipDoesntDependOnCoreProvisional FAILED java.lang.AssertionError: Architecture Violation [Priority: MEDIUM] - Rule 'classes that reside in a package 'org.apache.geode.distributed.internal.tcpserver..' should only depend on classes that reside in a package 'org.apache.geode.distributed.internal.tcpserver..' or reside in a package 'org.apache.geode.internal.serialization..' or reside in a package 'org.apache.geode.logging.internal.log4j.api..' or reside in a package 'org.apache.geode.logging.internal.executors..' or not reside in a package 'org.apache.geode..' or reside in a package 'org.apache.geode.test..'' was violated (2 times): Field is annotated with in (TcpServer.java:0) Field is annotated with in (TcpSocketFactory.java:0) 16 tests completed, 1 failed > Task :geode-tcp-server:test FAILED ``` 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
[GitHub] [geode] kirklund merged pull request #5538: Bump junit from 4.12 to 4.13
kirklund merged pull request #5538: URL: https://github.com/apache/geode/pull/5538 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
[GitHub] [geode-native] alb3rtobr commented on pull request #648: GEODE-8480: Add txmanager check in tx example
alb3rtobr commented on pull request #648: URL: https://github.com/apache/geode-native/pull/648#issuecomment-698608233 There you are a test that shows the problem. I have divided the code in two commits. In the first one, the test fails with the following error: ``` $ ctest -R TransactionsTest -j1 Test project /home/alb3rtobr/CLionProjects/Nordix/geode-native/build/cppcache/integration/test Start 61: TransactionsTest.ExceptionWhenRollingBackTx 1/1 Test #61: TransactionsTest.ExceptionWhenRollingBackTx ...***Exception: Child aborted 21.41 sec 0% tests passed, 1 tests failed out of 1 Total Test time (real) = 23.72 sec The following tests FAILED: 61 - TransactionsTest.ExceptionWhenRollingBackTx (Child aborted) Errors while running CTest ``` But after changing the code to call `transactionManager->exists()` before executing the rollback (the second commit), the test case works fine. 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
[GitHub] [geode] kirklund edited a comment on pull request #5539: Bump assertj from 3.15.0 to 3.17.2
kirklund edited a comment on pull request #5539: URL: https://github.com/apache/geode/pull/5539#issuecomment-698608120 Build failed due to warnings compiling geode-management: ``` > Task :geode-management:compileTestJava FAILED /home/geode/geode/geode-management/src/test/java/org/apache/geode/management/configuration/DeploymentTest.java:67: warning: [deprecation] isEqualToComparingFieldByField(Object) in AbstractObjectAssert has been deprecated assertThat(newValue).isEqualToComparingFieldByField(deployment); ^ where SELF,ACTUAL are type-variables: SELF extends AbstractObjectAssert declared in class AbstractObjectAssert ACTUAL extends Object declared in class AbstractObjectAssert error: warnings found and -Werror specified /home/geode/geode/geode-management/src/test/java/org/apache/geode/management/configuration/ClassNameTest.java:106: warning: [deprecation] isEqualToComparingFieldByField(Object) in AbstractObjectAssert has been deprecated assertThat(className).isEqualToComparingFieldByField(className1); ^ where SELF,ACTUAL are type-variables: SELF extends AbstractObjectAssert declared in class AbstractObjectAssert ACTUAL extends Object declared in class AbstractObjectAssert /home/geode/geode/geode-management/src/test/java/org/apache/geode/management/configuration/ClassNameTest.java:112: warning: [deprecation] isEqualToComparingFieldByField(Object) in AbstractObjectAssert has been deprecated assertThat(className2).isEqualToComparingFieldByField(className); ^ where SELF,ACTUAL are type-variables: SELF extends AbstractObjectAssert declared in class AbstractObjectAssert ACTUAL extends Object declared in class AbstractObjectAssert /home/geode/geode/geode-management/src/test/java/org/apache/geode/management/runtime/DeploymentInfoTest.java:51: warning: [deprecation] isEqualToComparingFieldByField(Object) in AbstractObjectAssert has been deprecated assertThat(newValue).isEqualToComparingFieldByField(deploymentInfo); ^ where SELF,ACTUAL are type-variables: SELF extends AbstractObjectAssert declared in class AbstractObjectAssert ACTUAL extends Object declared in class AbstractObjectAssert 1 error 4 warnings ``` 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
[GitHub] [geode] kirklund commented on pull request #5539: Bump assertj from 3.15.0 to 3.17.2
kirklund commented on pull request #5539: URL: https://github.com/apache/geode/pull/5539#issuecomment-698608120 ``` > Task :geode-management:compileTestJava FAILED /home/geode/geode/geode-management/src/test/java/org/apache/geode/management/configuration/DeploymentTest.java:67: warning: [deprecation] isEqualToComparingFieldByField(Object) in AbstractObjectAssert has been deprecated assertThat(newValue).isEqualToComparingFieldByField(deployment); ^ where SELF,ACTUAL are type-variables: SELF extends AbstractObjectAssert declared in class AbstractObjectAssert ACTUAL extends Object declared in class AbstractObjectAssert error: warnings found and -Werror specified /home/geode/geode/geode-management/src/test/java/org/apache/geode/management/configuration/ClassNameTest.java:106: warning: [deprecation] isEqualToComparingFieldByField(Object) in AbstractObjectAssert has been deprecated assertThat(className).isEqualToComparingFieldByField(className1); ^ where SELF,ACTUAL are type-variables: SELF extends AbstractObjectAssert declared in class AbstractObjectAssert ACTUAL extends Object declared in class AbstractObjectAssert /home/geode/geode/geode-management/src/test/java/org/apache/geode/management/configuration/ClassNameTest.java:112: warning: [deprecation] isEqualToComparingFieldByField(Object) in AbstractObjectAssert has been deprecated assertThat(className2).isEqualToComparingFieldByField(className); ^ where SELF,ACTUAL are type-variables: SELF extends AbstractObjectAssert declared in class AbstractObjectAssert ACTUAL extends Object declared in class AbstractObjectAssert /home/geode/geode/geode-management/src/test/java/org/apache/geode/management/runtime/DeploymentInfoTest.java:51: warning: [deprecation] isEqualToComparingFieldByField(Object) in AbstractObjectAssert has been deprecated assertThat(newValue).isEqualToComparingFieldByField(deploymentInfo); ^ where SELF,ACTUAL are type-variables: SELF extends AbstractObjectAssert declared in class AbstractObjectAssert ACTUAL extends Object declared in class AbstractObjectAssert 1 error 4 warnings ``` 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
[GitHub] [geode] jinmeiliao merged pull request #5536: GEODE-8520: GCStatsMonitor should sum up all the GC stats to get the …
jinmeiliao merged pull request #5536: URL: https://github.com/apache/geode/pull/5536 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
[GitHub] [geode] Bill commented on pull request #5545: GEODE-8522: Switching a exception log back to debug
Bill commented on pull request #5545: URL: https://github.com/apache/geode/pull/5545#issuecomment-698604220 Looks like CI failure is likely https://issues.apache.org/jira/browse/GEODE-8191 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
[GitHub] [geode] sabbeyPivotal commented on pull request #5533: GEODE-8498: Redis messages written to Netty channel sometimes delivered out of order
sabbeyPivotal commented on pull request #5533: URL: https://github.com/apache/geode/pull/5533#issuecomment-698593658 https://github.com/apache/geode/pull/5550 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
[GitHub] [geode] jchen21 commented on a change in pull request #5512: GEODE-7671: Add testing for GII with clear
jchen21 commented on a change in pull request #5512:
URL: https://github.com/apache/geode/pull/5512#discussion_r494557648
##
File path:
geode-core/src/distributedTest/java/org/apache/geode/internal/cache/ClearGIIDUnitTest.java
##
@@ -0,0 +1,543 @@
+/*
+ * 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.cache.RegionShortcut.PARTITION;
+import static org.apache.geode.cache.RegionShortcut.REPLICATE;
+import static
org.apache.geode.internal.cache.InitialImageOperation.getGIITestHookForCheckingPurpose;
+import static org.apache.geode.test.dunit.rules.ClusterStartupRule.getCache;
+import static
org.apache.geode.test.dunit.rules.ClusterStartupRule.getClientCache;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.Assert.fail;
+
+import java.io.Serializable;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Properties;
+import java.util.concurrent.CountDownLatch;
+
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+import org.apache.geode.cache.Cache;
+import org.apache.geode.cache.CacheException;
+import org.apache.geode.cache.PartitionAttributesFactory;
+import org.apache.geode.cache.PartitionedRegionPartialClearException;
+import org.apache.geode.cache.Region;
+import org.apache.geode.cache.RegionFactory;
+import org.apache.geode.cache.RegionShortcut;
+import org.apache.geode.cache.client.ClientRegionShortcut;
+import org.apache.geode.cache.query.Index;
+import org.apache.geode.cache.query.IndexStatistics;
+import org.apache.geode.cache.query.QueryService;
+import org.apache.geode.distributed.internal.ClusterDistributionManager;
+import org.apache.geode.distributed.internal.DistributionMessage;
+import org.apache.geode.distributed.internal.DistributionMessageObserver;
+import org.apache.geode.test.awaitility.GeodeAwaitility;
+import org.apache.geode.test.dunit.Assert;
+import org.apache.geode.test.dunit.AsyncInvocation;
+import org.apache.geode.test.dunit.SerializableRunnable;
+import org.apache.geode.test.dunit.WaitCriterion;
+import org.apache.geode.test.dunit.rules.ClientVM;
+import org.apache.geode.test.dunit.rules.ClusterStartupRule;
+import org.apache.geode.test.dunit.rules.MemberVM;
+
+
+@RunWith(Parameterized.class)
+public class ClearGIIDUnitTest implements Serializable {
+
+
+ protected static final String REGION_NAME = "testPR";
+ protected static final String INDEX_NAME = "testIndex";
+ protected static final int TOTAL_BUCKET_NUM = 10;
+ protected static final int REDUNDANT_COPIES = 1;
+ protected static final int DATA_SIZE = 100;
+ protected static final int NUM_SERVERS = 2;
+
+ @Parameterized.Parameter(0)
+ public RegionShortcut regionShortcut;
+
+ protected int locatorPort;
+ protected MemberVM locator;
+ protected MemberVM[] dataStores;
+ protected ClientVM client;
+
+ private static final Logger logger = LogManager.getLogger();
+
+ @Parameterized.Parameters
+ public static Collection getRegionShortcuts() {
+List params = new ArrayList<>();
+params.add(new Object[] {PARTITION});
+params.add(new Object[] {REPLICATE});
Review comment:
It is not necessary to test `REPLICATE` region in this test.
##
File path:
geode-core/src/distributedTest/java/org/apache/geode/internal/cache/ClearGIIDUnitTest.java
##
@@ -0,0 +1,543 @@
+/*
+ * 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
+
[GitHub] [geode] jhutchison opened a new pull request #5550: GEODE-8498: make AbstractSubscription write to channel synchronously
jhutchison opened a new pull request #5550: URL: https://github.com/apache/geode/pull/5550 Adds punsubscriptionlatch countdown WIP pipeline experiment without syncUninterupptibly GEODE-8498 keep published messages to same client in order Thank you for submitting a contribution to Apache Geode. In order to streamline the review of the contribution we ask you to ensure the following steps have been taken: ### For all changes: - [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message? - [ ] Has your PR been rebased against the latest commit within the target branch (typically `develop`)? - [ ] Is your initial contribution a single, squashed commit? - [ ] Does `gradlew build` run cleanly? - [ ] Have you written or updated unit tests to verify your changes? - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? ### Note: Please ensure that once the PR is submitted, check Concourse for build issues and submit an update to your PR as soon as possible. If you need help, please send an email to d...@geode.apache.org. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] davebarnes97 opened a new pull request #5549: GEODE-8533: Docs - compaction-threshold mechanism description are wrong
davebarnes97 opened a new pull request #5549: URL: https://github.com/apache/geode/pull/5549 Questions for reviewers: - Have the explanations satisfactorily stated that the compaction-threshold is a percentage of live data (non-garbage), as opposed to a percentage of garbage? - Is it clear that the threshold is activated when the live data percentage drops below the threshold, as opposed to exceeding it? - Is the modified formula for calculating required disk space correct? Thanks! 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
[GitHub] [geode] sabbeyPivotal closed pull request #5533: GEODE-8498: Redis messages written to Netty channel sometimes delivered out of order
sabbeyPivotal closed pull request #5533: URL: https://github.com/apache/geode/pull/5533 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
[GitHub] [geode] dschneider-pivotal commented on pull request #5538: Bump junit from 4.12 to 4.13
dschneider-pivotal commented on pull request #5538: URL: https://github.com/apache/geode/pull/5538#issuecomment-698578270 @onichols-pivotal can you review this This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] sabbeyPivotal commented on a change in pull request #5544: GEODE-8515: Redis PING should respond appropriately when called from within a SUBSCRIBE/PSUBSCRIBE
sabbeyPivotal commented on a change in pull request #5544:
URL: https://github.com/apache/geode/pull/5544#discussion_r494535974
##
File path:
geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/pubsub/SubscriptionsIntegrationTest.java
##
@@ -39,9 +38,8 @@
public static ExecutorServiceRule executor = new ExecutorServiceRule();
@Test
- @Ignore("GEODE-8515")
public void pingWhileSubscribed() {
-Jedis client = new Jedis("localhost", server.getPort());
+Jedis client = new Jedis("localhost", server.getPort(), 10);
Review comment:
Thank you for calling this out. I had a long timeout for debugging and
forgot to remove 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
[GitHub] [geode] sabbeyPivotal commented on a change in pull request #5544: GEODE-8515: Redis PING should respond appropriately when called from within a SUBSCRIBE/PSUBSCRIBE
sabbeyPivotal commented on a change in pull request #5544:
URL: https://github.com/apache/geode/pull/5544#discussion_r494536127
##
File path:
geode-redis/src/main/java/org/apache/geode/redis/internal/executor/connection/PingExecutor.java
##
@@ -31,9 +32,17 @@ public RedisResponse executeCommand(Command command,
ExecutionHandlerContext context) {
List commandElems = command.getProcessedCommand();
byte[] result = PING_RESPONSE.getBytes();
+byte[] subscribeResult = "".getBytes();
if (commandElems.size() > 1) {
result = commandElems.get(1);
+ subscribeResult = result;
}
+
+if
(!context.getPubSub().findSubscribedChannels(context.getClient()).isEmpty()) {
Review comment:
I refactored it. Check it out and let me know what you think.
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
[GitHub] [geode] dschneider-pivotal commented on a change in pull request #5544: GEODE-8515: Redis PING should respond appropriately when called from within a SUBSCRIBE/PSUBSCRIBE
dschneider-pivotal commented on a change in pull request #5544:
URL: https://github.com/apache/geode/pull/5544#discussion_r494487359
##
File path:
geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/pubsub/SubscriptionsIntegrationTest.java
##
@@ -39,9 +38,8 @@
public static ExecutorServiceRule executor = new ExecutorServiceRule();
@Test
- @Ignore("GEODE-8515")
public void pingWhileSubscribed() {
-Jedis client = new Jedis("localhost", server.getPort());
+Jedis client = new Jedis("localhost", server.getPort(), 10);
Review comment:
why a long timeout on this one but not on
pingWithArgumentWhileSubscribed?
##
File path:
geode-redis/src/main/java/org/apache/geode/redis/internal/executor/connection/PingExecutor.java
##
@@ -31,9 +32,17 @@ public RedisResponse executeCommand(Command command,
ExecutionHandlerContext context) {
List commandElems = command.getProcessedCommand();
byte[] result = PING_RESPONSE.getBytes();
+byte[] subscribeResult = "".getBytes();
if (commandElems.size() > 1) {
result = commandElems.get(1);
+ subscribeResult = result;
}
+
+if
(!context.getPubSub().findSubscribedChannels(context.getClient()).isEmpty()) {
Review comment:
consider refactoring this so that we only compute the RedisResponse for
subscribe if we find a client and then otherwise with (an else) compute the
RedisResponse when not subscribed. I don't like all the logic we have
initializing two different results and then we only end up using one or the
other. I'd prefer that we only have state at the top that would be shared by
either branch (for example commandElems). Once the RedisResponse is done we can
then fall through to a command return.
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
[GitHub] [geode] mhansonp commented on a change in pull request #5536: GEODE-8520: GCStatsMonitor should sum up all the GC stats to get the …
mhansonp commented on a change in pull request #5536:
URL: https://github.com/apache/geode/pull/5536#discussion_r494472324
##
File path:
geode-core/src/test/java/org/apache/geode/management/bean/stats/MemberLevelStatsTest.java
##
@@ -339,6 +342,41 @@ public void testRegionCounters() {
assertThat(memberMBeanBridge.getTotalPrimaryBucketCount()).isZero();
}
+ @Test
+ public void testVMStats() {
+Statistics[] realStats =
statisticsManager.findStatisticsByType(VMStats50.getGCType());
+long[] totals = modifyStatsAndReturnTotalCountAndTime(10, 2500, realStats);
+memberMBeanBridge.addVMStats(statSampler.getVMStats());
+
assertThat(memberMBeanBridge.getGarbageCollectionCount()).isEqualTo(totals[0]);
+
assertThat(memberMBeanBridge.getGarbageCollectionTime()).isEqualTo(totals[1]);
+
+long[] newTotals = modifyStatsAndReturnTotalCountAndTime(20, 3500,
realStats);
+sampleStats();
+
assertThat(memberMBeanBridge.getGarbageCollectionCount()).isEqualTo(newTotals[0]);
+
assertThat(memberMBeanBridge.getGarbageCollectionTime()).isEqualTo(newTotals[1]);
+ }
+
+ private long[] modifyStatsAndReturnTotalCountAndTime(
+ long baseCount, long baseTime,
+ Statistics[] modifiedStats) {
+long[] totalCountAndTime = {0, 0};
+for (Statistics gcStat : modifiedStats) {
+ StatisticDescriptor[] statistics = gcStat.getType().getStatistics();
Review comment:
Thanks!
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
[GitHub] [geode] demery-pivotal commented on a change in pull request #5536: GEODE-8520: GCStatsMonitor should sum up all the GC stats to get the …
demery-pivotal commented on a change in pull request #5536:
URL: https://github.com/apache/geode/pull/5536#discussion_r494449040
##
File path:
geode-core/src/test/java/org/apache/geode/management/internal/beans/stats/GCStatsMonitorTest.java
##
@@ -33,16 +45,94 @@
@Before
public void setUp() {
-gcStatsMonitor = new GCStatsMonitor(testName.getMethodName());
+ValueMonitor valueMonitor = mock(ValueMonitor.class);
+gcStatsMonitor = new GCStatsMonitor(testName.getMethodName(),
valueMonitor);
+
assertThat(gcStatsMonitor).isNotNull();
-assertThat(gcStatsMonitor.getStatistic("collections")).isEqualTo(0L);
-assertThat(gcStatsMonitor.getStatistic("collectionTime")).isEqualTo(0L);
+assertThat(gcStatsMonitor.getCollections()).isEqualTo(0L);
+assertThat(gcStatsMonitor.getCollectionTime()).isEqualTo(0L);
}
@Test
public void getStatisticShouldReturnZeroForUnknownStatistics() {
assertThat(gcStatsMonitor.getStatistic("unknownStatistic")).isEqualTo(0);
}
+ @Test
+ public void addStatsToMonitor() throws Exception {
+Statistics stats = mock(Statistics.class);
+when(stats.getUniqueId()).thenReturn(11L);
+StatisticDescriptor d1 = mock(StatisticDescriptor.class);
+when(d1.getName()).thenReturn(StatsKey.VM_GC_STATS_COLLECTIONS);
+StatisticDescriptor d2 = mock(StatisticDescriptor.class);
+when(d2.getName()).thenReturn(StatsKey.VM_GC_STATS_COLLECTION_TIME);
+StatisticDescriptor[] descriptors = {d1, d2};
+StatisticsType type = mock(StatisticsType.class);
+when(stats.getType()).thenReturn(type);
+when(type.getStatistics()).thenReturn(descriptors);
+
+when(stats.get(any(StatisticDescriptor.class))).thenReturn(8L, 300L);
Review comment:
This way of specifying return values unnecessarily couples the test to
the current implementation—it insists that the implementation must call
`stats.get(sd)` this many times, and in this specific order.
On this line and the two later ones, it would be better to associate each
value with the appropriate statistic descriptor, the way the other test does.
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
[GitHub] [geode-native] pdxcodemonkey merged pull request #655: GEODE-8524: Add support for KEY_SET and GET_ALL_70 messages
pdxcodemonkey merged pull request #655: URL: https://github.com/apache/geode-native/pull/655 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
