[GitHub] [geode] mivanac merged pull request #5426: GEODE-8292: Added check if key is destroyed in CQResults

2020-08-07 Thread GitBox


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


   



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] jujoramos commented on pull request #5227: GEODE-8191: flaky test correction

2020-08-07 Thread GitBox


jujoramos commented on pull request #5227:
URL: https://github.com/apache/geode/pull/5227#issuecomment-670504376


   Hello @mivanac , 
   Just for the record, I've been actively reviewing this `PR` since it was 
opened but I don't think the fix is sufficient to fix the flakiness, that's why 
I requested changes on `15/06/2020`; there wasn't any response from your side 
afterwards.



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 commented on pull request #5227: GEODE-8191: flaky test correction

2020-08-07 Thread GitBox


mivanac commented on pull request #5227:
URL: https://github.com/apache/geode/pull/5227#issuecomment-670507310


   Just for record, If you look at last commit, you can see that your requst 
(to force the bucket creation) is fullfilled.



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 #5432: GEODE-8191_1: test updated, added bucket initialization after creatio…

2020-08-07 Thread GitBox


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


   …n of each child region
   
   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] masaki-yamakawa commented on pull request #5418: GEODE-8396: Fixing NullPointerException in create jdbc-mapping command

2020-08-07 Thread GitBox


masaki-yamakawa commented on pull request #5418:
URL: https://github.com/apache/geode/pull/5418#issuecomment-670544653


   @DonalEvans 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] jujoramos commented on pull request #5227: GEODE-8191: flaky test correction

2020-08-07 Thread GitBox


jujoramos commented on pull request #5227:
URL: https://github.com/apache/geode/pull/5227#issuecomment-670514196


   Dang!, somehow I totally missed that notification and there wasn't any new 
comments on the `PR` (other than the one from Owen) so I didn't get any 
specific email addressed to myself (hard to keep track of old `PRs` when there 
are so many emails coming and going), sorry about that!.
   I can have a look at the `PR` if you re-open 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] mivanac commented on pull request #5227: GEODE-8191: flaky test correction

2020-08-07 Thread GitBox


mivanac commented on pull request #5227:
URL: https://github.com/apache/geode/pull/5227#issuecomment-670492076


   Since there is no reply on this PR, I will close 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] mivanac closed pull request #5227: GEODE-8191: flaky test correction

2020-08-07 Thread GitBox


mivanac closed pull request #5227:
URL: https://github.com/apache/geode/pull/5227


   



This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [geode] rhoughton-pivot merged pull request #5427: GEODE-8406: Enable early-return in CI scripts for CI-only changes

2020-08-07 Thread GitBox


rhoughton-pivot merged pull request #5427:
URL: https://github.com/apache/geode/pull/5427


   



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 #5348: DRAFT: Add flag in gateway sender to not store dropped events while stopped …

2020-08-07 Thread GitBox


albertogpz commented on a change in pull request #5348:
URL: https://github.com/apache/geode/pull/5348#discussion_r467114776



##
File path: 
geode-core/src/main/java/org/apache/geode/management/GatewaySenderMXBean.java
##
@@ -273,6 +273,8 @@
*/
   int getEventsExceedingAlertThreshold();
 
-
-
+  /**

Review comment:
   true

##
File path: 
geode-core/src/main/java/org/apache/geode/management/internal/beans/GatewaySenderMBean.java
##
@@ -241,4 +241,9 @@ public boolean isConnected() {
   public int getEventsExceedingAlertThreshold() {
 return bridge.getEventsExceedingAlertThreshold();
   }
+

Review comment:
   true





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 #5348: DRAFT: Add flag in gateway sender to not store dropped events while stopped …

2020-08-07 Thread GitBox


albertogpz commented on a change in pull request #5348:
URL: https://github.com/apache/geode/pull/5348#discussion_r467114892



##
File path: 
geode-core/src/main/java/org/apache/geode/management/internal/beans/GatewaySenderMBeanBridge.java
##
@@ -334,4 +334,8 @@ public boolean isConnected() {
   public int getEventsExceedingAlertThreshold() {
 return 
getStatistic(StatsKey.GATEWAYSENDER_EVENTS_EXCEEDING_ALERT_THRESHOLD).intValue();
   }
+

Review comment:
   true





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 #5348: DRAFT: Add flag in gateway sender to not store dropped events while stopped …

2020-08-07 Thread GitBox


albertogpz commented on a change in pull request #5348:
URL: https://github.com/apache/geode/pull/5348#discussion_r467116996



##
File path: 
geode-core/src/main/java/org/apache/geode/management/internal/configuration/functions/GatewaySenderManageFunction.java
##
@@ -0,0 +1,148 @@
+/*
+ * 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.management.internal.configuration.functions;
+
+import org.apache.logging.log4j.Logger;
+
+import org.apache.geode.annotations.Immutable;
+import org.apache.geode.cache.Cache;
+import org.apache.geode.cache.execute.FunctionContext;
+import org.apache.geode.cache.wan.GatewaySender;
+import org.apache.geode.internal.cache.execute.InternalFunction;
+import org.apache.geode.internal.cache.xmlcache.CacheXml;
+import org.apache.geode.logging.internal.log4j.api.LogService;
+import org.apache.geode.management.internal.configuration.domain.XmlEntity;
+import org.apache.geode.management.internal.functions.CliFunctionResult;
+
+public class GatewaySenderManageFunction implements InternalFunction {
+  private static final Logger logger = LogService.getLogger();
+  private static final long serialVersionUID = -6022504778575202026L;
+
+  @Immutable
+  public static final GatewaySenderManageFunction INSTANCE = new 
GatewaySenderManageFunction();
+
+  private static final String ID = GatewaySenderManageFunction.class.getName();
+
+  @Override
+  public boolean hasResult() {
+return true;
+  }
+
+
+  /**
+   * This function is used to report all the gateway sender instances for a 
given sender
+   * if all intances are stopped. In order to do so, it invokes the 
setMustQueueDroppedEvents()
+   * method
+   * on every gateway sender instance for the gateway sender passed as a 
parameter.
+   * The function requires an Object[] as context.getArguments() in which
+   * - the first parameter must be a Boolean stating of all gateway sender 
instances are stopped
+   * - the second parameter must be a String containing the name of the sender.
+   */
+  @Override
+  public void execute(FunctionContext context) {
+String memberName = context.getMemberName();
+try {
+  Object[] arguments;
+
+  Boolean stop = getStopFromArguments(context.getArguments());
+  if (stop == null) {
+context.getResultSender().lastResult(new CliFunctionResult("", false,
+"Wrong arguments passed"));
+return;
+  }
+  String senderId = getSenderIdFromArguments(context.getArguments());
+  if (senderId == null) {
+context.getResultSender().lastResult(new CliFunctionResult("", false,
+"Wrong arguments passed"));
+return;
+  }
+
+  Cache cache = context.getCache();
+  GatewaySender sender = cache.getGatewaySender(senderId);
+
+  if (sender == null) {
+context.getResultSender().lastResult(new CliFunctionResult(memberName, 
false,
+String.format("Sender '%s' does not exist", senderId)));
+return;
+  }
+
+  sender.setMustQueueDroppedEvents(!stop);
+  XmlEntity xmlEntity = new XmlEntity(CacheXml.GATEWAY_SENDER, "name", 
senderId);

Review comment:
   I did not want to extend CliFunction because I intended this function to 
be used by users of the Java API as @gesterzhou was after.
   
   In a new commit:
   
   - I have simplified the function a bit because the first level try/catch was 
already taking care of any exception.
   
   - I have also changed the name of the setter/flag from 
setMustQueueDroppedEvents to setAllInstancesStopped. I think it is a more 
understandable name without knowing the low level details about the dropped 
events.
   
   @gesterzhou Would this solution be acceptable for you having fixed the 
problem in gfsh and at the same time having a function that could be called by 
Java API users?





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 merged pull request #5432: GEODE-8191_1: test updated, added bucket initialization after creatio…

2020-08-07 Thread GitBox


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


   



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] agingade merged pull request #5424: GEODE-8394: Rewind the message Part on command failure

2020-08-07 Thread GitBox


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


   



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 #5433: Revert "GEODE-8407: MergeLogFiles fails to include files with the sam…

2020-08-07 Thread GitBox


bschuchardt opened a new pull request #5433:
URL: https://github.com/apache/geode/pull/5433


   …e name b… (#5428)"
   
   This reverts commit d6c3b1f20a55c5d867f162a338910d5df7d47de5.
   The new test fails consistently on Windows.
   
   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] DonalEvans merged pull request #5418: GEODE-8396: Fixing NullPointerException in create jdbc-mapping command

2020-08-07 Thread GitBox


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


   



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 #625: GEODE-8340: Enforce Switch compiler warnings as errors

2020-08-07 Thread GitBox


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



##
File path: cppcache/src/ExceptionTypes.cpp
##
@@ -297,7 +297,25 @@ const std::string& getThreadLocalExceptionMessage();
   PutAllPartialResultException ex(message);
   throw ex;
 }
-default: {
+case GF_NOERR:

Review comment:
   
![pdxcodemonkey?](http://media2.giphy.com/gifsu/1qbj0e6kDAjS7elwu0/giphy-caption.gif?cid=6104955e5f2b035851577a4b4d4cc90a)
   
   (I mostly just wanted to post that gif, no particular rush )





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 opened a new pull request #5434: DRAFT: Solution to not queue forever dropped events with 2 timeouts

2020-08-07 Thread GitBox


albertogpz opened a new pull request #5434:
URL: https://github.com/apache/geode/pull/5434


   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] jujoramos commented on pull request #5175: GEODE-8119: Threads hangs when offline disk store command is invoked

2020-08-07 Thread GitBox


jujoramos commented on pull request #5175:
URL: https://github.com/apache/geode/pull/5175#issuecomment-670655764


   @mkevo 
   Thanks for adding the extra tests. I'm not fully convinced about the 
implemented fix, however, did you consider my comments 
[here](https://github.com/apache/geode/pull/5175#issuecomment-638793177)?. 
Can't you just simply invoke the `DiskStoreImpl.close()` method instead of 
modifying the `DiskStoreCommandsUtils` class, as it involves way less 
modifications and we're sure it won't affect other areas of the `disk` 
management stuff?.
   



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 #628: GEODE-8344: Add GatewaySenderEventCallbackArgument class

2020-08-07 Thread GitBox


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



##
File path: cppcache/integration/test/Order.cpp
##
@@ -0,0 +1,60 @@
+/*
+ * 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.
+ */
+
+#include "Order.hpp"
+
+#include 
+#include 
+
+namespace testGeode8344 {

Review comment:
   I would consider picking a namespace name that is more descriptive.  I 
think a few months from now (or for me, a few weeks) it will not be obvious 
what Geode 8344 means without having to go look up it up in jira





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 #5435: GEODE-8413: CI failure: MergeLogFilesIntegrationTest.testDircountZero fails on Windows

2020-08-07 Thread GitBox


bschuchardt opened a new pull request #5435:
URL: https://github.com/apache/geode/pull/5435


   Fixing directory path formation problem for Windows.  I've tested these 
changes on a Mac and on a Windows machine.
   
   
   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] bschuchardt commented on pull request #5435: GEODE-8413: CI failure: MergeLogFilesIntegrationTest.testDircountZero fails on Windows

2020-08-07 Thread GitBox


bschuchardt commented on pull request #5435:
URL: https://github.com/apache/geode/pull/5435#issuecomment-670643196


   Since this is a test-only change and the checks that use that test have 
passed I'm merging this PR before all of the non-required checks have finished.



This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [geode] bschuchardt merged pull request #5435: GEODE-8413: CI failure: MergeLogFilesIntegrationTest.testDircountZero fails on Windows

2020-08-07 Thread GitBox


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


   



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 closed pull request #5433: Revert "GEODE-8407: MergeLogFiles fails to include files with the sam…

2020-08-07 Thread GitBox


bschuchardt closed pull request #5433:
URL: https://github.com/apache/geode/pull/5433


   



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 opened a new pull request #5436: GEODE-7672: add dunit test to verify OQL index after PR clear.

2020-08-07 Thread GitBox


jinmeiliao opened a new pull request #5436:
URL: https://github.com/apache/geode/pull/5436


   * allow ClusterStartupRule to launch dunit default locator



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] jivthesh commented on pull request #5430: added code of conduct to project

2020-08-07 Thread GitBox


jivthesh commented on pull request #5430:
URL: https://github.com/apache/geode/pull/5430#issuecomment-670828286


   @metatype okay
   



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