[GitHub] [geode] dschneider-pivotal opened a new pull request #5419: GEODE-6564: fix entryExpiryTasks leak
dschneider-pivotal opened a new pull request #5419: URL: https://github.com/apache/geode/pull/5419 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] gesterzhou commented on a change in pull request #5391: GEODE-7846: Adding Stats for Partitioned Region Clear
gesterzhou commented on a change in pull request #5391:
URL: https://github.com/apache/geode/pull/5391#discussion_r464551192
##
File path:
geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegionMap.java
##
@@ -344,7 +344,12 @@ void incClearCount(LocalRegion lr) {
if (lr != null && !(lr instanceof HARegion)) {
CachePerfStats stats = lr.getCachePerfStats();
if (stats != null) {
-stats.incClearCount();
+if (lr instanceof BucketRegion) {
Review comment:
Usually we use: if (owner.isUsedForPartitionedRegionBucket()) {
instead.
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] mkevo commented on a change in pull request #5397: GEODE-8293: fix decrementing activeCQCount
mkevo commented on a change in pull request #5397:
URL: https://github.com/apache/geode/pull/5397#discussion_r464297701
##
File path:
geode-cq/src/main/java/org/apache/geode/cache/query/cq/internal/ServerCQImpl.java
##
@@ -367,10 +367,12 @@ public void close(boolean sendRequestToServer) throws
CqClosedException, CqExcep
this.removeFromCqMap();
// Stat update.
- if (stateBeforeClosing == CqStateImpl.RUNNING) {
-cqService.stats().decCqsActive();
- } else if (stateBeforeClosing == CqStateImpl.STOPPED) {
-cqService.stats().decCqsStopped();
+ if (!cqName.equals(serverCqName)) {
Review comment:
I need somehow to makes difference between servers, one on which cq is
register and others on which it will process registerCq. The server on which cq
is registered doing also incrementing active activeCqCount, but while closing
or stopping it decrements on all, but with this check it will decrement just on
the one server where it is incremented. I'm not sure if this is a good check
but it works, and I asked on dev list for a help to differentiate on which
decrement is needed, but It is concluded that this is a bug but without any
idea how to differentiate 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] lgtm-com[bot] commented on pull request #5390: ClassLoader isolation
lgtm-com[bot] commented on pull request #5390: URL: https://github.com/apache/geode/pull/5390#issuecomment-668133588 This pull request **introduces 2 alerts** and **fixes 2** when merging 1e2455c497ed45f195c5b2c4f7b65e08f362d3ca into 54421c4cd5ddde9e459f544fef9f27b44fa1ab6c - [view on LGTM.com](https://lgtm.com/projects/g/apache/geode/rev/pr-4554256831c8306281714a9c0b6253c122fac9a2) **new alerts:** * 2 for Potential input resource leak **fixed alerts:** * 2 for Unused variable, import, function or class 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] jdeppe-pivotal opened a new pull request #5420: GEODE-8333: fix pubsub hang
jdeppe-pivotal opened a new pull request #5420: URL: https://github.com/apache/geode/pull/5420 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] sabbeyPivotal commented on pull request #5416: GEODE-8382: Run Redis tests against Redis API for Geode
sabbeyPivotal commented on pull request #5416: URL: https://github.com/apache/geode/pull/5416#issuecomment-668168119 @smgoller @rhoughton-pivot @onichols-pivotal @dickcav 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] gesterzhou commented on a change in pull request #5391: GEODE-7846: Adding Stats for Partitioned Region Clear
gesterzhou commented on a change in pull request #5391: URL: https://github.com/apache/geode/pull/5391#discussion_r464581367 ## File path: geode-core/src/main/java/org/apache/geode/internal/cache/CachePerfStats.java ## @@ -274,7 +278,14 @@ "Current number of regions configured for reliablity that are missing required roles with Limited access"; final String reliableRegionsMissingNoAccessDesc = "Current number of regions configured for reliablity that are missing required roles with No access"; -final String clearsDesc = "The total number of times a clear has been done on this cache."; +final String regionClearsDesc = +"The total number of times a clear has been done on this cache."; Review comment: on this cache? or on this region? 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] gesterzhou commented on a change in pull request #5391: GEODE-7846: Adding Stats for Partitioned Region Clear
gesterzhou commented on a change in pull request #5391: URL: https://github.com/apache/geode/pull/5391#discussion_r464583502 ## File path: geode-core/src/main/java/org/apache/geode/internal/cache/CachePerfStats.java ## @@ -274,7 +278,14 @@ "Current number of regions configured for reliablity that are missing required roles with Limited access"; final String reliableRegionsMissingNoAccessDesc = "Current number of regions configured for reliablity that are missing required roles with No access"; -final String clearsDesc = "The total number of times a clear has been done on this cache."; +final String regionClearsDesc = +"The total number of times a clear has been done on this cache."; +final String bucketClearsDesc = +"The total number of times a clear has been done on this region and it's bucket regions"; +final String partitionedRegionClearLocalDurationDesc = +"The time in nanoseconds partitioned region clear has been running for the region on this member"; Review comment: we have to follow other duration stats's timeunit. If other duration stats used nanoseconds, we use it too. Otherwise, we should not. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] onichols-pivotal commented on a change in pull request #5416: GEODE-8382: Run Redis tests against Redis API for Geode
onichols-pivotal commented on a change in pull request #5416:
URL: https://github.com/apache/geode/pull/5416#discussion_r464644167
##
File path: ci/scripts/execute_redis_tests.sh
##
@@ -0,0 +1,51 @@
+#!/usr/bin/env bash
+
+#
+# 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.
+
+cd ..
+git clone --config transfer.fsckObjects=false
https://github.com/prettyClouds/redis.git
+cd redis
+git checkout tests-geode-redis
+
+JAVA_HOME="/usr/lib/jvm/java-11-openjdk-amd64" \
Review comment:
you should not need to hardcode the JDK path. The JDK you should be
using is passed to you as `${JAVA_TEST_PATH}` (see ci/scripts/execute_tests.sh)
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] onichols-pivotal commented on a change in pull request #5416: GEODE-8382: Run Redis tests against Redis API for Geode
onichols-pivotal commented on a change in pull request #5416: URL: https://github.com/apache/geode/pull/5416#discussion_r464646511 ## File path: ci/scripts/execute_redis_tests.sh ## @@ -0,0 +1,51 @@ +#!/usr/bin/env bash + +# +# 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. + +cd .. +git clone --config transfer.fsckObjects=false https://github.com/prettyClouds/redis.git +cd redis +git checkout tests-geode-redis + +JAVA_HOME="/usr/lib/jvm/java-11-openjdk-amd64" \ +../geode-assembly/build/install/apache-geode/bin/gfsh start server \ + --J=-Denable-redis-unsupported-commands=true \ + --name=server1 \ + --redis-port=6380 \ + --redis-bind-address=127.0.0.1 \ + --redis-password=foobar + +failCount=0 + +./runtest --host 127.0.0.1 --port 6380 --single unit/auth + +((failCount+=$?)) + +JAVA_HOME="/usr/lib/jvm/java-11-openjdk-amd64" \ Review comment: redundant as you have already set JAVA_HOME above 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] BenjaminPerryRoss commented on a change in pull request #5391: GEODE-7846: Adding Stats for Partitioned Region Clear
BenjaminPerryRoss commented on a change in pull request #5391: URL: https://github.com/apache/geode/pull/5391#discussion_r464609432 ## File path: geode-core/src/main/java/org/apache/geode/internal/cache/CachePerfStats.java ## @@ -274,7 +278,14 @@ "Current number of regions configured for reliablity that are missing required roles with Limited access"; final String reliableRegionsMissingNoAccessDesc = "Current number of regions configured for reliablity that are missing required roles with No access"; -final String clearsDesc = "The total number of times a clear has been done on this cache."; +final String regionClearsDesc = +"The total number of times a clear has been done on this cache."; +final String bucketClearsDesc = +"The total number of times a clear has been done on this region and it's bucket regions"; +final String partitionedRegionClearLocalDurationDesc = +"The time in nanoseconds partitioned region clear has been running for the region on this member"; Review comment: I actually switched this from the original metric I used to nanoseconds because other stats in the file seem to be using them and I wanted to be consistent (such as EventQueueThrottleTime or any of the tx lifetime stats in that class). 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 #5419: GEODE-6564: fix entryExpiryTasks leak
dschneider-pivotal commented on pull request #5419: URL: https://github.com/apache/geode/pull/5419#issuecomment-668226584 The dunit failure is the known issue: GEODE-7710 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] bschuchardt merged pull request #5413: GEODE-8389: reconnect attempt fails due to distribution configuration…
bschuchardt merged pull request #5413: URL: https://github.com/apache/geode/pull/5413 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 #5416: GEODE-8382: Run Redis tests against Redis API for Geode
sabbeyPivotal commented on a change in pull request #5416: URL: https://github.com/apache/geode/pull/5416#discussion_r464665333 ## File path: ci/pipelines/shared/jinja.variables.yml ## @@ -140,6 +140,19 @@ tests: PLATFORM: linux RAM: '210' name: Upgrade +- ARTIFACT_SLUG: redistests + CALL_STACK_TIMEOUT: '1200' + CPUS: '4' + DUNIT_PARALLEL_FORKS: '0' + EXECUTE_TEST_TIMEOUT: 30m + GRADLE_TASK: ':geode-redis:redisAPITest' + MAX_IN_FLIGHT: 1 + ONLY_JDK: 11 + PARALLEL_DUNIT: 'false' + PARALLEL_GRADLE: 'true' + PLATFORM: linux + RAM: '16' Review comment: just changed 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] rhoughton-pivot commented on a change in pull request #5416: GEODE-8382: Run Redis tests against Redis API for Geode
rhoughton-pivot commented on a change in pull request #5416: URL: https://github.com/apache/geode/pull/5416#discussion_r464641006 ## File path: ci/scripts/execute_redis_tests.sh ## @@ -0,0 +1,51 @@ +#!/usr/bin/env bash + +# +# 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. + +cd .. +git clone --config transfer.fsckObjects=false https://github.com/prettyClouds/redis.git +cd redis +git checkout tests-geode-redis + +JAVA_HOME="/usr/lib/jvm/java-11-openjdk-amd64" \ +../geode-assembly/build/install/apache-geode/bin/gfsh start server \ + --J=-Denable-redis-unsupported-commands=true \ + --name=server1 \ + --redis-port=6380 \ + --redis-bind-address=127.0.0.1 \ + --redis-password=foobar + +failCount=0 + +./runtest --host 127.0.0.1 --port 6380 --single unit/auth + +((failCount+=$?)) + +JAVA_HOME="/usr/lib/jvm/java-11-openjdk-amd64" \ +../geode-assembly/build/install/apache-geode/bin/gfsh start server \ + --J=-Denable-redis-unsupported-commands=true \ + --name=server2 \ + --server-port=0 \ + --redis-port=6379 \ + --redis-bind-address=127.0.0.1 + +./runtest --host 127.0.0.1 --port 6379 --single unit/type/set --single unit/expire + +((failCount+=$?)) + +exit $failCount Review comment: I saw that this does fail-red when the count != 0. Looks good. ## File path: ci/pipelines/shared/jinja.variables.yml ## @@ -140,6 +140,19 @@ tests: PLATFORM: linux RAM: '210' name: Upgrade +- ARTIFACT_SLUG: redistests + CALL_STACK_TIMEOUT: '1200' + CPUS: '4' + DUNIT_PARALLEL_FORKS: '0' + EXECUTE_TEST_TIMEOUT: 30m + GRADLE_TASK: ':geode-redis:redisAPITest' + MAX_IN_FLIGHT: 1 + ONLY_JDK: 11 + PARALLEL_DUNIT: 'false' + PARALLEL_GRADLE: 'true' + PLATFORM: linux + RAM: '16' Review comment: I looked at the resource usage for this heavy lifter, and we can safely take this down to 8GB ram, easy. 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] onichols-pivotal commented on a change in pull request #5416: GEODE-8382: Run Redis tests against Redis API for Geode
onichols-pivotal commented on a change in pull request #5416: URL: https://github.com/apache/geode/pull/5416#discussion_r464647336 ## File path: ci/scripts/execute_redis_tests.sh ## @@ -0,0 +1,51 @@ +#!/usr/bin/env bash + +# +# 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. + +cd .. +git clone --config transfer.fsckObjects=false https://github.com/prettyClouds/redis.git Review comment: would a comment be appropriate here explaining why a personal fork is being used and the plan/timeline for eliminating dependency on a personal fork? 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 #5416: GEODE-8382: Run Redis tests against Redis API for Geode
sabbeyPivotal commented on a change in pull request #5416: URL: https://github.com/apache/geode/pull/5416#discussion_r464665517 ## File path: ci/scripts/execute_redis_tests.sh ## @@ -0,0 +1,51 @@ +#!/usr/bin/env bash + +# +# 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. + +cd .. +git clone --config transfer.fsckObjects=false https://github.com/prettyClouds/redis.git +cd redis +git checkout tests-geode-redis + +JAVA_HOME="/usr/lib/jvm/java-11-openjdk-amd64" \ +../geode-assembly/build/install/apache-geode/bin/gfsh start server \ + --J=-Denable-redis-unsupported-commands=true \ + --name=server1 \ + --redis-port=6380 \ + --redis-bind-address=127.0.0.1 \ + --redis-password=foobar + +failCount=0 + +./runtest --host 127.0.0.1 --port 6380 --single unit/auth + +((failCount+=$?)) + +JAVA_HOME="/usr/lib/jvm/java-11-openjdk-amd64" \ +../geode-assembly/build/install/apache-geode/bin/gfsh start server \ + --J=-Denable-redis-unsupported-commands=true \ + --name=server2 \ + --server-port=0 \ + --redis-port=6379 \ + --redis-bind-address=127.0.0.1 + +./runtest --host 127.0.0.1 --port 6379 --single unit/type/set --single unit/expire + +((failCount+=$?)) + +exit $failCount Review comment: sweet, thanks for double checking! 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 #5416: GEODE-8382: Run Redis tests against Redis API for Geode
sabbeyPivotal commented on a change in pull request #5416: URL: https://github.com/apache/geode/pull/5416#discussion_r464665951 ## File path: ci/scripts/execute_redis_tests.sh ## @@ -0,0 +1,51 @@ +#!/usr/bin/env bash + +# +# 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. + +cd .. +git clone --config transfer.fsckObjects=false https://github.com/prettyClouds/redis.git +cd redis +git checkout tests-geode-redis + +JAVA_HOME="/usr/lib/jvm/java-11-openjdk-amd64" \ Review comment: Good call. I just updated it and exported the JAVA_HOME variable, definitely check it over and make sure it looks correct. 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] lgtm-com[bot] commented on pull request #5390: ClassLoader isolation
lgtm-com[bot] commented on pull request #5390: URL: https://github.com/apache/geode/pull/5390#issuecomment-668245249 This pull request **introduces 2 alerts** and **fixes 2** when merging 373fdda56c7adb155d9255e2f96eb96993110467 into 54421c4cd5ddde9e459f544fef9f27b44fa1ab6c - [view on LGTM.com](https://lgtm.com/projects/g/apache/geode/rev/pr-5ce97f63a28f0b1a92c8b4f65cf428a820b36375) **new alerts:** * 2 for Potential input resource leak **fixed alerts:** * 2 for Unused variable, import, function or class 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 #5419: GEODE-6564: fix entryExpiryTasks leak
dschneider-pivotal merged pull request #5419: URL: https://github.com/apache/geode/pull/5419 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 #5421: Pr2
sabbeyPivotal closed pull request #5421: URL: https://github.com/apache/geode/pull/5421 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 opened a new pull request #5421: Pr2
sabbeyPivotal opened a new pull request #5421: URL: https://github.com/apache/geode/pull/5421 test456 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] BenjaminPerryRoss commented on a change in pull request #5391: GEODE-7846: Adding Stats for Partitioned Region Clear
BenjaminPerryRoss commented on a change in pull request #5391: URL: https://github.com/apache/geode/pull/5391#discussion_r464609432 ## File path: geode-core/src/main/java/org/apache/geode/internal/cache/CachePerfStats.java ## @@ -274,7 +278,14 @@ "Current number of regions configured for reliablity that are missing required roles with Limited access"; final String reliableRegionsMissingNoAccessDesc = "Current number of regions configured for reliablity that are missing required roles with No access"; -final String clearsDesc = "The total number of times a clear has been done on this cache."; +final String regionClearsDesc = +"The total number of times a clear has been done on this cache."; +final String bucketClearsDesc = +"The total number of times a clear has been done on this region and it's bucket regions"; +final String partitionedRegionClearLocalDurationDesc = +"The time in nanoseconds partitioned region clear has been running for the region on this member"; Review comment: I actually switch this from the original metric I used to nanoseconds because other stats in the file seem to be using them and I wanted to be consistent (such as EventQueueThrottleTime or any of the tx lifetime stats in that class). 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] onichols-pivotal commented on a change in pull request #5416: GEODE-8382: Run Redis tests against Redis API for Geode
onichols-pivotal commented on a change in pull request #5416: URL: https://github.com/apache/geode/pull/5416#discussion_r464644167 ## File path: ci/scripts/execute_redis_tests.sh ## @@ -0,0 +1,51 @@ +#!/usr/bin/env bash + +# +# 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. + +cd .. +git clone --config transfer.fsckObjects=false https://github.com/prettyClouds/redis.git +cd redis +git checkout tests-geode-redis + +JAVA_HOME="/usr/lib/jvm/java-11-openjdk-amd64" \ Review comment: rather than hardcoding the JDK path, please use JAVA_HOME at this point, as you've already configured `ONLY_JDK: 11` and ci/scripts/execute_tests.sh sets up JAVA_HOME for the heavy lifter already 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 #5416: GEODE-8382: Run Redis tests against Redis API for Geode
sabbeyPivotal commented on a change in pull request #5416: URL: https://github.com/apache/geode/pull/5416#discussion_r464666362 ## File path: ci/scripts/execute_redis_tests.sh ## @@ -0,0 +1,51 @@ +#!/usr/bin/env bash + +# +# 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. + +cd .. +git clone --config transfer.fsckObjects=false https://github.com/prettyClouds/redis.git Review comment: Just added a comment. Let me know if you think it explains the situation sufficiently. 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 #5416: GEODE-8382: Run Redis tests against Redis API for Geode
sabbeyPivotal commented on a change in pull request #5416: URL: https://github.com/apache/geode/pull/5416#discussion_r464666188 ## File path: ci/scripts/execute_redis_tests.sh ## @@ -0,0 +1,51 @@ +#!/usr/bin/env bash + +# +# 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. + +cd .. +git clone --config transfer.fsckObjects=false https://github.com/prettyClouds/redis.git +cd redis +git checkout tests-geode-redis + +JAVA_HOME="/usr/lib/jvm/java-11-openjdk-amd64" \ +../geode-assembly/build/install/apache-geode/bin/gfsh start server \ + --J=-Denable-redis-unsupported-commands=true \ + --name=server1 \ + --redis-port=6380 \ + --redis-bind-address=127.0.0.1 \ + --redis-password=foobar + +failCount=0 + +./runtest --host 127.0.0.1 --port 6380 --single unit/auth + +((failCount+=$?)) + +JAVA_HOME="/usr/lib/jvm/java-11-openjdk-amd64" \ Review comment: Just exported the variable and was able to remove this. Thanks for the 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
