[GitHub] [geode-native] gaussianrecurrence opened a new pull request #659: GEODE-8548 Avoid hostname resolution in TcpConn

2020-09-29 Thread GitBox


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


- Calling get_host_name() in TcpConn::connect involves a system call that
  might take several seconds to complete.
- So endpoint is stored instead upon TcpConn construction and used later
  instead of making the system call.



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] mivanac opened a new pull request #5567: GEODE-8547: Added impacts to show missing disk-stores

2020-09-29 Thread GitBox


mivanac opened a new pull request #5567:
URL: https://github.com/apache/geode/pull/5567


   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] mkevo merged pull request #5509: GEODE-8491: Do not store dropped events in stopped primary gateway se…

2020-09-29 Thread GitBox


mkevo merged pull request #5509:
URL: https://github.com/apache/geode/pull/5509


   



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] moleske merged pull request #645: GEODE-8469: Enable no-missing-variable-declarations

2020-09-28 Thread GitBox


moleske merged pull request #645:
URL: https://github.com/apache/geode-native/pull/645


   



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] jhuynh1 commented on pull request #5524: Usability improvements for 1.13

2020-09-28 Thread GitBox


jhuynh1 commented on pull request #5524:
URL: https://github.com/apache/geode/pull/5524#issuecomment-700267261


   @upthewaterspout unfortunately, those are how they were merged into develop. 
 I think someone clicked merge instead of squash and merge.  Those are all part 
of a set of commits for one feature/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] onichols-pivotal merged pull request #5565: GEODE-8496: un-upgrade archunit to avoid OOM on JDK8

2020-09-28 Thread GitBox


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


   



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 #5539: Bump assertj from 3.15.0 to 3.17.2

2020-09-28 Thread GitBox


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


   



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 commented on a change in pull request #5562: GEODE-8542: java.lang.IllegalStateException: tcp message exceeded max…

2020-09-28 Thread GitBox


echobravopapa commented on a change in pull request #5562:
URL: https://github.com/apache/geode/pull/5562#discussion_r496195999



##
File path: 
geode-core/src/test/java/org/apache/geode/internal/tcp/MsgStreamerTest.java
##
@@ -92,9 +117,9 @@ protected BaseMsgStreamer createMsgStreamer(boolean 
mixedDestinationVersions) {
 when(connection2.getRemoteAddress()).thenReturn(member2);
 
when(connection2.getSendBufferSize()).thenReturn(Connection.SMALL_BUFFER_SIZE);
 if (mixedDestinationVersions) {
-  
when(connection1.getRemoteVersion()).thenReturn(KnownVersion.GEODE_1_12_0);
+  
when(connection2.getRemoteVersion()).thenReturn(KnownVersion.GEODE_1_12_0);
 } else {
-  when(connection1.getRemoteVersion()).thenReturn(KnownVersion.CURRENT);
+  when(connection2.getRemoteVersion()).thenReturn(KnownVersion.CURRENT);

Review comment:
   `connection2` does not seem to be used in the test cases





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] moleske commented on a change in pull request #645: GEODE-8469: Enable no-missing-variable-declarations

2020-09-28 Thread GitBox


moleske commented on a change in pull request #645:
URL: https://github.com/apache/geode-native/pull/645#discussion_r496186063



##
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:
   Got it!  I'll have some tests run again just to make sure it is still 
green





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 opened a new pull request #5566: GEODE-8522: Switching exception log back to debug (merge to 1.13)

2020-09-28 Thread GitBox


upthewaterspout opened a new pull request #5566:
URL: https://github.com/apache/geode/pull/5566


   This log message happens during the course of normal startup of multiple
   locators. We should not be logging a full stack trace during normal startup.
   
   (cherry picked from commit 3df057ce1ad9913ee7991377d02e86faa371fef0)
   
   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] upthewaterspout commented on pull request #5524: Usability improvements for 1.13

2020-09-28 Thread GitBox


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


   One question - I see a bunch of commits with titles like "Fix spotless 
issues". Are these how this commits all went on develop? Is the below really 
just all one change?
   
   * GEODE-8283: Provide REST interface for disk-store creation 3bbaca9
   * Fix spotless issues 49b361b
   * Add missing documentation files to assembly 9dddcc0
   * Updating assembly validation txt (#4) 0dbc3e8
   * Remove Id field from DiskStore 7d0e6fb
   * Fix jq issues 7f744d2
   * Disk store rest api (#5) 985368f
   * Add JQ testing for disk-store controller dc228d0
   * Change DiskDir size to be Integer 470783c
   * Fix serializables b5b3479
   * Remove Id field from DiskStore cf3406c 
   



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 merged pull request #5545: GEODE-8522: Switching a exception log back to debug

2020-09-28 Thread GitBox


upthewaterspout merged pull request #5545:
URL: https://github.com/apache/geode/pull/5545


   



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 #5509: GEODE-8491: Do not store dropped events in stopped primary gateway se…

2020-09-28 Thread GitBox


gesterzhou commented on a change in pull request #5509:
URL: https://github.com/apache/geode/pull/5509#discussion_r496146173



##
File path: 
geode-core/src/main/java/org/apache/geode/internal/cache/wan/AbstractGatewaySender.java
##
@@ -1118,6 +1115,24 @@ public void distribute(EnumListenerEvent operation, 
EntryEventImpl event,
 }
   }
 
+  private void recordDroppedEvent(EntryEventImpl event) {
+final boolean isDebugEnabled = logger.isDebugEnabled();
+if (this.eventProcessor != null) {
+  this.eventProcessor.registerEventDroppedInPrimaryQueue(event);
+} else {
+  // Add empty event so that in case the event stays for long in
+  // tmpDroppedEvents it takes as little space as possible.
+  // No need to have all the contents of the event for a dropped one.
+  EntryEventImpl emptyEvent = new EntryEventImpl(event.getKey(), false);

Review comment:
   For this part, it's unnecessary to enhance 
registerEventDroppedInPrimaryQueue() since it's used for a very small window 
(sender is restarting). 
   
   I feel your previous solution is good enough. 





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] mreddington opened a new pull request #658: GEODE-8129: A test that breaks and reestablishes the proxy connection.

2020-09-28 Thread GitBox


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


   



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 a change in pull request #5562: GEODE-8542: java.lang.IllegalStateException: tcp message exceeded max…

2020-09-28 Thread GitBox


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



##
File path: 
geode-core/src/test/java/org/apache/geode/internal/tcp/MsgStreamerTest.java
##
@@ -92,9 +117,9 @@ protected BaseMsgStreamer createMsgStreamer(boolean 
mixedDestinationVersions) {
 when(connection2.getRemoteAddress()).thenReturn(member2);
 
when(connection2.getSendBufferSize()).thenReturn(Connection.SMALL_BUFFER_SIZE);
 if (mixedDestinationVersions) {
-  
when(connection1.getRemoteVersion()).thenReturn(KnownVersion.GEODE_1_12_0);
+  
when(connection2.getRemoteVersion()).thenReturn(KnownVersion.GEODE_1_12_0);
 } else {
-  when(connection1.getRemoteVersion()).thenReturn(KnownVersion.CURRENT);
+  when(connection2.getRemoteVersion()).thenReturn(KnownVersion.CURRENT);

Review comment:
   I don't know why this code was changed to make the second connection 
(instead of the first), the old version connection. But the resulting code 
seems just as correct as the old code  ✓

##
File path: 
geode-core/src/main/java/org/apache/geode/internal/tcp/MsgStreamer.java
##
@@ -129,7 +130,8 @@ public ConnectExceptions getConnectExceptions() {
 this.stats = stats;
 this.msg = msg;
 this.cons = cons;
-this.buffer = bufferPool.acquireDirectSenderBuffer(sendBufferSize);
+int bufferSize = Math.min(sendBufferSize, Connection.MAX_MSG_SIZE);
+this.buffer = bufferPool.acquireDirectSenderBuffer(bufferSize);

Review comment:
   since this class is in the same package as `Connection` this seems like 
appropriate coupling ✓

##
File path: 
geode-core/src/test/java/org/apache/geode/internal/tcp/MsgStreamerTest.java
##
@@ -77,6 +77,31 @@ public void streamerListReleaseWithException() throws 
IOException {
 verify(pool, times(2)).releaseSenderBuffer(isA(ByteBuffer.class));
   }
 
+  @Test
+  public void streamerRespectsMaxMessageSize() {
+InternalDistributedMember member1;
+member1 = new InternalDistributedMember("localhost", 1234);
+
+DistributionMessage message = new SerialAckedMessage();
+message.setRecipients(Arrays.asList(member1));
+
+when(connection1.getRemoteAddress()).thenReturn(member1);
+when(connection1.getRemoteVersion()).thenReturn(KnownVersion.CURRENT);
+// create a streamer for a Connection that has a buffer size that's larger 
than the
+// biggest message we can actually send. This is picked up by the 
MsgStreamer to allocate
+// a buffer
+when(connection1.getSendBufferSize()).thenReturn(Connection.MAX_MSG_SIZE * 
2);

Review comment:
   This test could be stronger if the requested message size was 
`MAX_MSG_SIZE + 1`. As it stands this test allows a lot of wiggle-room for the 
implementation.





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

2020-09-28 Thread GitBox


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


   Yeah I don't think `ClassNotFound` is appropriate for retry but, as is often 
the case, I bet the original developer used the IDE to write that catch block 
and since both of those exceptions are declared in the `throws` clause 
together, well.
   
   I think your latest change solves the problems.



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 #5565: GEODE-8496: un-upgrade archunit to avoid OOM on JDK8

2020-09-28 Thread GitBox


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


   @onichols-pivotal please include a reference to the ArchUnit 0.13.0 bug you 
found
   
   In general, we found with older versions of the tool, that sometimes we had 
to break up processing into smaller sets to avoid out of memory errors.



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 closed pull request #5381: GEODE-8367: Stopped parallel gateway sender should not add events int…

2020-09-28 Thread GitBox


gesterzhou closed pull request #5381:
URL: https://github.com/apache/geode/pull/5381


   



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 commented on pull request #656: GEODE-8530: Fix for coredump during tx commit

2020-09-28 Thread GitBox


jvarenina commented on pull request #656:
URL: https://github.com/apache/geode-native/pull/656#issuecomment-699804258


   Thanks for the review!



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 #5565: GEODE-8496: un-upgrade archunit to avoid OOM on JDK8

2020-09-26 Thread GitBox


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


   #5537 passed in PR checked, which run only JDK11, but under JDK8 a bug in 
archunit is encountered which results in OOM:
   
   
https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-develop-main/jobs/IntegrationTestOpenJDK8/builds/462
   
   > java.lang.OutOfMemoryError: Java heap space
   >at java.util.Formatter.parse(Formatter.java:2560)
   >at java.util.Formatter.format(Formatter.java:2501)
   >at java.util.Formatter.format(Formatter.java:2455)
   >at java.lang.String.format(String.java:2940)
   >at 
com.tngtech.archunit.core.domain.SourceCodeLocation.formatLocation(SourceCodeLocation.java:60)
   >at 
com.tngtech.archunit.core.domain.SourceCodeLocation.(SourceCodeLocation.java:78)
   >at 
com.tngtech.archunit.core.domain.SourceCodeLocation.of(SourceCodeLocation.java:52)
   >...
   
   This archunit bug seems to have been introduced in 0.13.0, so downgrade to 
the version prior



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 #5537: Bump archunit from 0.12.0 to 0.14.1

2020-09-25 Thread GitBox


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"

2020-09-25 Thread GitBox


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

2020-09-25 Thread GitBox


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

2020-09-25 Thread GitBox


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…

2020-09-25 Thread GitBox


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

2020-09-25 Thread GitBox


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

2020-09-25 Thread GitBox


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] kirklund merged pull request #5555: GEODE-8541: move test to integrationTest folder

2020-09-25 Thread GitBox


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


   



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

2020-09-25 Thread GitBox


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

2020-09-25 Thread GitBox


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

2020-09-25 Thread GitBox


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

2020-09-25 Thread GitBox


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

2020-09-25 Thread GitBox


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

2020-09-25 Thread GitBox


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

2020-09-25 Thread GitBox


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…

2020-09-25 Thread GitBox


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

2020-09-25 Thread GitBox


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

2020-09-25 Thread GitBox


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-native] pdxcodemonkey merged pull request #657: GOEDO-8534: fix xcode 12 build

2020-09-25 Thread GitBox


pdxcodemonkey merged pull request #657:
URL: https://github.com/apache/geode-native/pull/657


   



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

2020-09-25 Thread GitBox


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"

2020-09-25 Thread GitBox


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

2020-09-25 Thread GitBox


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"

2020-09-25 Thread GitBox


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…

2020-09-25 Thread GitBox


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

2020-09-25 Thread GitBox


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

2020-09-25 Thread GitBox


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

2020-09-25 Thread GitBox


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

2020-09-25 Thread GitBox


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

2020-09-25 Thread GitBox


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

2020-09-25 Thread GitBox


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

2020-09-25 Thread GitBox


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

2020-09-25 Thread GitBox


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

2020-09-25 Thread GitBox


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

2020-09-25 Thread GitBox


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

2020-09-25 Thread GitBox


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

2020-09-25 Thread GitBox


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

2020-09-25 Thread GitBox


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

2020-09-25 Thread GitBox


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 …

2020-09-25 Thread GitBox


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

2020-09-25 Thread GitBox


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

2020-09-25 Thread GitBox


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

2020-09-25 Thread GitBox


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

2020-09-25 Thread GitBox


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 …

2020-09-25 Thread GitBox


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

2020-09-25 Thread GitBox


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

2020-09-25 Thread GitBox


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

2020-09-25 Thread GitBox


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

2020-09-25 Thread GitBox


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

2020-09-25 Thread GitBox


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 …

2020-09-25 Thread GitBox


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

2020-09-25 Thread GitBox


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

2020-09-25 Thread GitBox


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

2020-09-25 Thread GitBox


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

2020-09-25 Thread GitBox


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

2020-09-25 Thread GitBox


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

2020-09-24 Thread GitBox


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

2020-09-24 Thread GitBox


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

2020-09-24 Thread GitBox


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

2020-09-24 Thread GitBox


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

2020-09-24 Thread GitBox


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

2020-09-24 Thread GitBox


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

2020-09-24 Thread GitBox


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

2020-09-24 Thread GitBox


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

2020-09-24 Thread GitBox


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

2020-09-24 Thread GitBox


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

2020-09-24 Thread GitBox


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

2020-09-24 Thread GitBox


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

2020-09-24 Thread GitBox


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

2020-09-24 Thread GitBox


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 …

2020-09-24 Thread GitBox


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

2020-09-24 Thread GitBox


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

2020-09-24 Thread GitBox


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

2020-09-24 Thread GitBox


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

2020-09-24 Thread GitBox


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

2020-09-24 Thread GitBox


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

2020-09-24 Thread GitBox


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

2020-09-24 Thread GitBox


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

2020-09-24 Thread GitBox


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

2020-09-24 Thread GitBox


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

2020-09-24 Thread GitBox


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




  1   2   3   4   5   6   7   8   9   10   >