[GitHub] [geode] onichols-pivotal merged pull request #5537: Bump archunit from 0.12.0 to 0.14.1
onichols-pivotal merged pull request #5537: URL: https://github.com/apache/geode/pull/5537 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 merged pull request #5554: Revert "Bump junit from 4.12 to 4.13"
onichols-pivotal merged pull request #5554: URL: https://github.com/apache/geode/pull/5554 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 opened a new pull request #5564: DRAFT: GEODE-8252: Rename DistributedCounters
kirklund opened a new pull request #5564: URL: https://github.com/apache/geode/pull/5564 Rename SharedCountersRule as DistributedCounters. 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 opened a new pull request #5563: DRAFT: GEODE-8252: Rename DistributedErrorCollector
kirklund opened a new pull request #5563: URL: https://github.com/apache/geode/pull/5563 Rename SharedErrorCollector as DistributedErrorCollector. 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 opened a new pull request #5562: GEODE-8542: java.lang.IllegalStateException: tcp message exceeded max…
bschuchardt opened a new pull request #5562: URL: https://github.com/apache/geode/pull/5562 … size of 16777215 Limit the size of message chunks to the maximum message size allowed by org.apache.geode.internal.tcp.Connection. 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-native] pivotal-jbarrett commented on a change in pull request #645: GEODE-8469: Enable no-missing-variable-declarations
pivotal-jbarrett commented on a change in pull request #645:
URL: https://github.com/apache/geode-native/pull/645#discussion_r495290623
##
File path: cppcache/src/ExceptionTypes.cpp
##
@@ -30,8 +30,8 @@ namespace client {
void setThreadLocalExceptionMessage(std::string exMsg);
const std::string& getThreadLocalExceptionMessage();
-std::map>
+static std::map>
Review comment:
Can we make this conform to the naming convention.
Will be nice if some day this could be `constexpr`.
https://www.youtube.com/watch?v=INn3xa4pMfg
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 #5561: try a diff exception
lgtm-com[bot] commented on pull request #5561: URL: https://github.com/apache/geode/pull/5561#issuecomment-699168122 This pull request **introduces 1 alert** and **fixes 1** when merging c51ed007bcd7d6435f2829e14f65520fdd39cf05 into c05118a5cf31cdd20e595ea9093809eb9f6872fc - [view on LGTM.com](https://lgtm.com/projects/g/apache/geode/rev/pr-1e9f8cc094752f9b5dd7470495f1cf47fb72d617) **new alerts:** * 1 for Implicit conversion from array to string **fixed alerts:** * 1 for Implicit conversion from array to string 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] echobravopapa opened a new pull request #5561: try a diff exception
echobravopapa opened a new pull request #5561: URL: https://github.com/apache/geode/pull/5561 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] mhansonp commented on pull request #5560: GEODE-7845: adding PRClear protection code
mhansonp commented on pull request #5560: URL: https://github.com/apache/geode/pull/5560#issuecomment-699151510 I am not a fan of this solution at all. I am hoping people can provide some useful feedback to make this way better. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] mhansonp opened a new pull request #5560: GEODE-7845: adding PRClear protection code
mhansonp opened a new pull request #5560: URL: https://github.com/apache/geode/pull/5560 - prevent PRClear messages from going to older servers. 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 opened a new pull request #5559: GEODE-8544: Making VM class start versioned VM
mhansonp opened a new pull request #5559: URL: https://github.com/apache/geode/pull/5559 Fixing an issue where the VM class will not properly start a versioned server. 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 #5556: Added logging to checkCancelled to get stacktrace
lgtm-com[bot] commented on pull request #5556: URL: https://github.com/apache/geode/pull/5556#issuecomment-699150325 This pull request **introduces 1 alert** when merging c05118a5cf31cdd20e595ea9093809eb9f6872fc into 22f2c5247411f646521f47f6c0f900feaa3aa65c - [view on LGTM.com](https://lgtm.com/projects/g/apache/geode/rev/pr-d7fdda521b751e022920d0f44a09fef6575bc1e3) **new alerts:** * 1 for Implicit conversion from array to string 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 opened a new pull request #5558: DRAFT: GEODE-8539: Update FixedPartitioningDUnitTest with Rules
kirklund opened a new pull request #5558: URL: https://github.com/apache/geode/pull/5558 Depends on #5557 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 opened a new pull request #5557: GEODE-8540: Create new DistributedBlackboard Rule
kirklund opened a new pull request #5557: URL: https://github.com/apache/geode/pull/5557 Package up DUnitBlackboard as a JUnit Rule named DistributedBlackboard. 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 #5530: GEODE-8517: GatewaySenderEventImpl's 2 new attributes were introduced…
gesterzhou commented on a change in pull request #5530:
URL: https://github.com/apache/geode/pull/5530#discussion_r495220452
##
File path:
geode-core/src/main/java/org/apache/geode/internal/cache/wan/GatewaySenderEventImpl.java
##
@@ -710,7 +710,7 @@ public int getDSFID() {
@Override
public void toData(DataOutput out,
SerializationContext context) throws IOException {
-toDataPre_GEODE_1_13_0_0(out, context);
+toDataPre_GEODE_1_14_0_0(out, context);
boolean hasTransaction = this.transactionId != null;
Review comment:
The version in toData() is 0. So it will not help. I added "if (version
== 0 || version >= KnownVersion.GEODE_1_14_0.ordinal()) {" too. The result is
the same. It did not improve on reproducing 49196.
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-699140620 @Bill - are my new changes better? I put in an info level message. I left the catch for ClassNotFoundException alone. I'm not really clear on why that exception is appropriate for a retry in this case, but I'm hesitant to change behavior in this 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
[GitHub] [geode] echobravopapa opened a new pull request #5556: Added logging to checkCancelled to get stacktrace
echobravopapa opened a new pull request #5556: URL: https://github.com/apache/geode/pull/5556 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] dschneider-pivotal opened a new pull request #5555: GEODE-8541: move test to integrationTest folder
dschneider-pivotal opened a new pull request #: URL: https://github.com/apache/geode/pull/ 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 #5554: Revert "Bump junit from 4.12 to 4.13"
kirklund commented on pull request #5554: URL: https://github.com/apache/geode/pull/5554#issuecomment-699086756 I think this failure caused several downstream failures in UnitTest: ``` org.apache.geode.internal.cache.ReplicateWithExpirationClearIntegrationTest > clearDoesNotLeaveEntryExpiryTaskInRegion FAILED org.apache.geode.management.ManagementException: MBean Not Registered In GemFire Domain at org.apache.geode.management.internal.SystemManagementService.federate(SystemManagementService.java:233) at org.apache.geode.management.internal.beans.ManagementAdapter.handleCacheCreation(ManagementAdapter.java:185) at org.apache.geode.management.internal.beans.ManagementListener.handleEvent(ManagementListener.java:127) at org.apache.geode.distributed.internal.InternalDistributedSystem.notifyResourceEventListeners(InternalDistributedSystem.java:2086) at org.apache.geode.distributed.internal.InternalDistributedSystem.handleResourceEvent(InternalDistributedSystem.java:643) at org.apache.geode.internal.cache.GemFireCacheImpl.initialize(GemFireCacheImpl.java:1436) at org.apache.geode.internal.cache.InternalCacheBuilder.create(InternalCacheBuilder.java:191) at org.apache.geode.internal.cache.InternalCacheBuilder.create(InternalCacheBuilder.java:158) at org.apache.geode.cache.CacheFactory.create(CacheFactory.java:142) at org.apache.geode.internal.cache.ReplicateWithExpirationClearIntegrationTest.setUp(ReplicateWithExpirationClearIntegrationTest.java:48) ``` ReplicateWithExpirationClearIntegrationTest is also an integration test and should be moved from src/test to src/integrationTest. 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 #656: GEODE-8530: Fix for coredump during tx commit
pdxcodemonkey merged pull request #656: URL: https://github.com/apache/geode-native/pull/656 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 opened a new pull request #5554: Revert "Bump junit from 4.12 to 4.13"
onichols-pivotal opened a new pull request #5554: URL: https://github.com/apache/geode/pull/5554 Reverts apache/geode#5538 as it caused org.apache.geode.internal.cache.SimpleDiskRegionJUnitTest.testBasicClose to fail on Windows If it's easier to just fix the issue than to revert+fix, go for it, but it it's not immediately obvious to me what the problem is. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] albertogpz commented on a change in pull request #5509: GEODE-8491: Do not store dropped events in stopped primary gateway se…
albertogpz commented on a change in pull request #5509:
URL: https://github.com/apache/geode/pull/5509#discussion_r495138023
##
File path:
geode-core/src/main/java/org/apache/geode/internal/cache/wan/AbstractGatewaySender.java
##
@@ -1118,6 +1115,17 @@ public void distribute(EnumListenerEvent operation,
EntryEventImpl event,
}
}
+ private void recordDroppedEvent(EntryEventImpl event) {
+if (this.eventProcessor != null) {
+ this.eventProcessor.registerEventDroppedInPrimaryQueue(event);
Review comment:
@boglesby and @gesterzhou, friendly reminder :-)
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 opened a new pull request #657: GOEDO-8534: fix xcode 12 build
pdxcodemonkey opened a new pull request #657: URL: https://github.com/apache/geode-native/pull/657 We acquired a couple new compiler warnings in the upgrade, so this placates the Mac build for XCode 12. @mreddington @dihardman @davebarnes97 @karensmolermiller 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] DonalEvans opened a new pull request #5553: GEODE-8536: Allow limited retries when creating Lucene IndexWriter
DonalEvans opened a new pull request #5553: URL: https://github.com/apache/geode/pull/5553 Authored-by: Donal Evans Thank you for submitting a contribution to Apache Geode. In order to streamline the review of the contribution we ask you to ensure the following steps have been taken: ### For all changes: - [x] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message? - [x] Has your PR been rebased against the latest commit within the target branch (typically `develop`)? - [x] Is your initial contribution a single, squashed commit? - [x] Does `gradlew build` run cleanly? - [x] Have you written or updated unit tests to verify your changes? - [N/A] 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] jhutchison commented on a change in pull request #5552: GEODE-8538: Create test to validate ordering of redis pipeline commands
jhutchison commented on a change in pull request #5552:
URL: https://github.com/apache/geode/pull/5552#discussion_r495085108
##
File path:
geode-redis/src/integrationTest/java/org/apache/geode/redis/CommandPipeliningIntegrationTest.java
##
@@ -93,6 +98,42 @@ public void whenPipelining_commandResponsesAreNotCorrupted()
{
assertThat(mockSubscriber.getReceivedMessages()).isEqualTo(expectedMessages);
}
+
+ @Test
+ public void should_returnResultsOfPipelinedCommands_inCorrectOrder() {
+Logger logger = LogManager.getLogger("org.apache.geode.redis.internal");
+Configurator.setAllLevels(logger.getName(), Level.getLevel("DEBUG"));
+FastLogger.setDelegating(true);
+
+Jedis jedis = new Jedis("localhost", server.getPort(), 1000);
+int NUMBER_OF_COMMANDS_IN_PIPELINE = 100;
+int numberOfPipeLineRequests = 1000;
+
+do {
+ Pipeline p = jedis.pipelined();
+ for (int i = 0; i < NUMBER_OF_COMMANDS_IN_PIPELINE; i++) {
+p.echo(String.valueOf(i));
+ }
+
+ List results = p.syncAndReturnAll();
+
+ verifyResultOrder(NUMBER_OF_COMMANDS_IN_PIPELINE, results);
+ numberOfPipeLineRequests--;
+} while (numberOfPipeLineRequests > 0);
+
+jedis.flushAll();
+jedis.close();
+ }
+
+ private void verifyResultOrder(int NUMBER_OF_COMMAND_IN_PIPELINE,
List results) {
Review comment:
made final. 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] ringles commented on a change in pull request #5552: GEODE-8538: Create test to validate ordering of redis pipeline commands
ringles commented on a change in pull request #5552:
URL: https://github.com/apache/geode/pull/5552#discussion_r495082332
##
File path:
geode-redis/src/integrationTest/java/org/apache/geode/redis/CommandPipeliningIntegrationTest.java
##
@@ -93,6 +98,42 @@ public void whenPipelining_commandResponsesAreNotCorrupted()
{
assertThat(mockSubscriber.getReceivedMessages()).isEqualTo(expectedMessages);
}
+
+ @Test
+ public void should_returnResultsOfPipelinedCommands_inCorrectOrder() {
+Logger logger = LogManager.getLogger("org.apache.geode.redis.internal");
+Configurator.setAllLevels(logger.getName(), Level.getLevel("DEBUG"));
+FastLogger.setDelegating(true);
+
+Jedis jedis = new Jedis("localhost", server.getPort(), 1000);
+int NUMBER_OF_COMMANDS_IN_PIPELINE = 100;
+int numberOfPipeLineRequests = 1000;
+
+do {
+ Pipeline p = jedis.pipelined();
+ for (int i = 0; i < NUMBER_OF_COMMANDS_IN_PIPELINE; i++) {
+p.echo(String.valueOf(i));
+ }
+
+ List results = p.syncAndReturnAll();
+
+ verifyResultOrder(NUMBER_OF_COMMANDS_IN_PIPELINE, results);
+ numberOfPipeLineRequests--;
+} while (numberOfPipeLineRequests > 0);
+
+jedis.flushAll();
+jedis.close();
+ }
+
+ private void verifyResultOrder(int NUMBER_OF_COMMAND_IN_PIPELINE,
List results) {
Review comment:
NUMBER_OF_COMMAND_IN_PIPELINE looks like a constant, but isn't. It
should be something like "numberOfCommands" or "numberOfCommandsInPipeline".
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 #5552: GEODE-8538: Create test to validate ordering of redis pipeline commands
jhutchison opened a new pull request #5552: URL: https://github.com/apache/geode/pull/5552 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] jdeppe-pivotal merged pull request #5550: GEODE-8498: make AbstractSubscription write to channel synchronously
jdeppe-pivotal merged pull request #5550: URL: 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-native] alb3rtobr commented on pull request #651: Remove bucketServerLocation if timeout error
alb3rtobr commented on pull request #651: URL: https://github.com/apache/geode-native/pull/651#issuecomment-698811497 @pdxcodemonkey @echobravopapa Have you had the opportunity to check how could I add a test to verify this? What if I create a separate ticket for the test case and this change is merge in the meanwhile? As after a timeout there is always a IO error, this change seems an improvement in the current situation when that happens, as it is in our 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
[GitHub] [geode] jvarenina commented on a change in pull request #5360: GEODE-8329: Fix for durable CQ reqistration recovery
jvarenina commented on a change in pull request #5360:
URL: https://github.com/apache/geode/pull/5360#discussion_r494922796
##
File path:
geode-core/src/main/java/org/apache/geode/cache/client/internal/QueueManagerImpl.java
##
@@ -1112,7 +1112,8 @@ private void recoverCqs(Connection recoveredConnection,
boolean isDurable) {
.set(((DefaultQueryService)
this.pool.getQueryService()).getUserAttributes(name));
}
try {
-if (((CqStateImpl) cqi.getState()).getState() != CqStateImpl.INIT) {
+if (((CqStateImpl) cqi.getState()).getState() != CqStateImpl.INIT
Review comment:
Hi @agingade ,
Sorry for bothering, but this request change hangs here for a long period of
time. If you don't have time or my comments are not clear please don't hesitate
to contact me, but I would be really grateful if we could decide how to proceed
with this 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
[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] 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] 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 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] 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] 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] 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] 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-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] 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] 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] 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] 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-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
[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] 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 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.
##
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] 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] 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 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] jvarenina commented on a change in pull request #5360: GEODE-8329: Fix for durable CQ reqistration recovery
jvarenina commented on a change in pull request #5360:
URL: https://github.com/apache/geode/pull/5360#discussion_r494922796
##
File path:
geode-core/src/main/java/org/apache/geode/cache/client/internal/QueueManagerImpl.java
##
@@ -1112,7 +1112,8 @@ private void recoverCqs(Connection recoveredConnection,
boolean isDurable) {
.set(((DefaultQueryService)
this.pool.getQueryService()).getUserAttributes(name));
}
try {
-if (((CqStateImpl) cqi.getState()).getState() != CqStateImpl.INIT) {
+if (((CqStateImpl) cqi.getState()).getState() != CqStateImpl.INIT
Review comment:
Hi @agingade ,
Sorry for bothering, but this request change hangs here for a long period of
time. If you don't have time or my comments are not clear please don't hesitate
to contact me, but I would be really grateful if we could decide how to proceed
with this 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
[GitHub] [geode-native] alb3rtobr commented on pull request #651: Remove bucketServerLocation if timeout error
alb3rtobr commented on pull request #651: URL: https://github.com/apache/geode-native/pull/651#issuecomment-698811497 @pdxcodemonkey @echobravopapa Have you had the opportunity to check how could I add a test to verify this? What if I create a separate ticket for the test case and this change is merge in the meanwhile? As after a timeout there is always a IO error, this change seems an improvement in the current situation when that happens, as it is in our 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
[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
