[GitHub] [geode] mivanac merged pull request #5426: GEODE-8292: Added check if key is destroyed in CQResults
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
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
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…
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
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
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
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
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
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 …
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 …
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 …
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…
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
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…
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
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
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
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
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
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
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
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
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…
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.
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
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