[GitHub] geode issue #733: Keeping old versions around even during a clean
Github user upthewaterspout commented on the issue: https://github.com/apache/geode/pull/733 Closing this PR, it seems like this isn't the right change. I'm a little short on time to mess with this, so if someone else wants to tackle handling the download in a nicer way please do. Regarding my attempt to use the ivy hack - it looks like what was getting in my way was using apache's redirect to the mirrors, but I'm not sure how to get around that. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode pull request #733: Keeping old versions around even during a clean
Github user upthewaterspout closed the pull request at: https://github.com/apache/geode/pull/733 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode pull request #740: GEODE-3513: Removing the use of native sessions ses...
Github user upthewaterspout commented on a diff in the pull request: https://github.com/apache/geode/pull/740#discussion_r135150238 --- Diff: extensions/geode-modules-session/src/main/java/org/apache/geode/modules/session/filter/SessionListener.java --- @@ -22,6 +22,11 @@ import javax.servlet.http.HttpSessionEvent; import javax.servlet.http.HttpSessionListener; +/** --- End diff -- modify_war should no longer add this session listener. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode pull request #740: GEODE-3513: Removing the use of native sessions ses...
Github user upthewaterspout commented on a diff in the pull request:
https://github.com/apache/geode/pull/740#discussion_r135150926
--- Diff:
extensions/session-testing-war/src/main/java/org/apache/geode/modules/session/SessionCountingListener.java
---
@@ -12,28 +12,35 @@
* or implied. See the License for the specific language governing
permissions and limitations under
* the License.
*/
+package org.apache.geode.modules.session;
-package org.apache.geode.modules.session.internal.filter;
-
-import org.apache.geode.modules.session.filter.SessionCachingFilter;
-
-import javax.servlet.http.HttpSession;
+import java.util.concurrent.atomic.AtomicInteger;
import javax.servlet.http.HttpSessionEvent;
import javax.servlet.http.HttpSessionListener;
-public class HttpSessionListenerImpl2 extends AbstractListener implements
HttpSessionListener {
+public class SessionCountingListener extends ListenerStoredInSessionContext
+implements HttpSessionListener {
+ private final AtomicInteger sessionCount = new AtomicInteger();
--- End diff --
Is this sessionCount field used?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode pull request #740: GEODE-3513: Removing the use of native sessions ses...
Github user upthewaterspout commented on a diff in the pull request:
https://github.com/apache/geode/pull/740#discussion_r135151782
--- Diff:
geode-assembly/src/test/java/org/apache/geode/session/tests/Jetty9CachingClientServerTest.java
---
@@ -0,0 +1,83 @@
+/*
+ * 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.session.tests;
+
+import static org.junit.Assert.assertEquals;
+
+import java.io.IOException;
+import java.net.URISyntaxException;
+import java.util.concurrent.TimeUnit;
+
+import javax.servlet.http.HttpSession;
+
+import org.awaitility.Awaitility;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import org.apache.geode.cache.Cache;
+import org.apache.geode.cache.Region;
+import org.apache.geode.modules.session.functions.GetSessionCount;
+import org.apache.geode.test.dunit.DUnitEnv;
+
+/**
+ * Jetty 9 Client Server tests
+ *
+ * Runs all the tests in {@link CargoTestBase} on the Jetty 9 install,
setup in the
+ * {@link #setupJettyInstall()} method before tests are run.
+ */
+public class Jetty9CachingClientServerTest extends
GenericAppServerClientServerTest {
+ private static ContainerInstall install;
+
+ @BeforeClass
+ public static void setupJettyInstall() throws Exception {
+install = new
GenericAppServerInstall(GenericAppServerInstall.GenericAppServerVersion.JETTY9,
+ContainerInstall.ConnectionType.CACHING_CLIENT_SERVER,
+ContainerInstall.DEFAULT_INSTALL_DIR + "Jetty9ClientServerTest");
--- End diff --
Change this install directory?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode pull request #740: GEODE-3513: Removing the use of native sessions ses...
Github user upthewaterspout commented on a diff in the pull request: https://github.com/apache/geode/pull/740#discussion_r135148933 --- Diff: extensions/geode-modules-session-internal/src/main/java/org/apache/geode/modules/session/internal/filter/attributes/AbstractSessionAttributes.java --- @@ -66,6 +66,8 @@ */ protected String jvmOwnerId; + protected long creationTime; --- End diff -- Does this field need to be serialized? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode issue #740: GEODE-3513: Removing the use of native sessions session ca...
Github user upthewaterspout commented on the issue: https://github.com/apache/geode/pull/740 @jhuynh1 @boglesby --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode pull request #740: GEODE-3513: Removing the use of native sessions ses...
GitHub user upthewaterspout opened a pull request: https://github.com/apache/geode/pull/740 GEODE-3513: Removing the use of native sessions session caching In the session module for generic app servers, we were asking the application server for a 'native' session and then wrapping it on our own GemfireHttpSession. However, we were not cleaning up that native session, which means that in PROXY mode we were leaving these sessions on the client with them being useful. The GemfireHttpSession now no longer wraps a native session. We are still temporarily creating a native session because it is the only way for us to get the session timeout value that was configured in web.xml, but the native session is immediately invalidated. Adding and extending cargo session tests to test how sessions are being cleaned up from the clients and the server. 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, you check travis-ci for build issues and submit an update to your PR as soon as possible. If you need help, please send an email to dev@geode.apache.org. You can merge this pull request into a Git repository by running: $ git pull https://github.com/upthewaterspout/incubator-geode feature/GEODE-3513 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode/pull/740.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #740 commit 93163bf8077cd6363dfcb9c1e1a37d1876e1669e Author: Dan Smith <upthewatersp...@apache.org> Date: 2017-08-11T16:46:40Z GEODE-3513: Removing the use of native sessions session caching In the session module for generic app servers, we were asking the application server for a 'native' session and then wrapping it on our own GemfireHttpSession. However, we were not cleaning up that native session, which means that in PROXY mode we were leaving these sessions on the client with them being useful. The GemfireHttpSession now no longer wraps a native session. We are still temporarily creating a native session because it is the only way for us to get the session timeout value that was configured in web.xml, but the native session is immediately invalidated. Adding and extending cargo session tests to test how sessions are being cleaned up from the clients and the server. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode issue #733: Keeping old versions around even during a clean
Github user upthewaterspout commented on the issue: https://github.com/apache/geode/pull/733 > Would it be possible to include old versions as dependencies? Yeah, that's what I would really like to do, actually. The problem is that gradle doesn't make it easy to cache an arbitrary url, only something that is published as part of a maven/ivy repo. I tried using this hack to pretend that a url was an ivy repo, but it doesn't seem to work for me - https://stackoverflow.com/a/34327202. I can try messing around with a bit more though. Or maybe we could publish geode 1.2.0 as a maven dependency? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode pull request #733: Keeping old versions around even during a clean
GitHub user upthewaterspout opened a pull request: https://github.com/apache/geode/pull/733 Keeping old versions around even during a clean Putting old versions of geode in a separate directory so they are not removed during a clean. 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, you check travis-ci for build issues and submit an update to your PR as soon as possible. If you need help, please send an email to dev@geode.apache.org. @jhuynh1 @kohlmu-pivotal You can merge this pull request into a Git repository by running: $ git pull https://github.com/upthewaterspout/incubator-geode feature/download-outside-build Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode/pull/733.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #733 commit 4f72450533018162636f2b003c7e9900219036f3 Author: Dan Smith <upthewatersp...@apache.org> Date: 2017-08-22T20:53:16Z Keeping old versions around even during a clean Putting old versions of geode in a separate directory so they are not removed during a clean. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode pull request #728: Don't download geode 1.2 every time a build runs
GitHub user upthewaterspout opened a pull request: https://github.com/apache/geode/pull/728 Don't download geode 1.2 every time a build runs Setting the overwrite flag on the download tasks to false so that we don't download the old version of geode for every build. 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, you check travis-ci for build issues and submit an update to your PR as soon as possible. If you need help, please send an email to dev@geode.apache.org. @boglesby @metatype You can merge this pull request into a Git repository by running: $ git pull https://github.com/upthewaterspout/incubator-geode feature/old_version_download Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode/pull/728.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #728 commit 4574428a1e0196a7477c6cf2b308131acaf91a48 Author: Dan Smith <upthewatersp...@apache.org> Date: 2017-08-21T20:30:22Z Don't download geode 1.2 every time a build runs Setting the overwrite flag on the download tasks to false so that we don't download the old version of geode for every build. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode pull request #712: GEODE-3434: Allow the modules to be interoperable w...
Github user upthewaterspout commented on a diff in the pull request:
https://github.com/apache/geode/pull/712#discussion_r133827499
--- Diff:
extensions/geode-modules/src/main/java/org/apache/geode/modules/session/catalina/DeltaSession.java
---
@@ -553,8 +555,30 @@ public void fromData(DataInput in) throws IOException,
ClassNotFoundException {
}
}
- protected Map readInAttributes(final DataInput in) throws IOException,
ClassNotFoundException {
-return DataSerializer.readObject(in);
+ private void readInAttributes(DataInput in) throws IOException,
ClassNotFoundException {
+Map map = DataSerializer.readObject(in);
+ConcurrentMap newMap = new ConcurrentHashMap();
+newMap.putAll(map);
+try {
+ Field field = getAttributesFieldObject();
+ field.setAccessible(true);
+ field.set(this, newMap);
+} catch (NoSuchFieldException e) {
+ logError(e);
--- End diff --
I think it would be better to throw an exception than log an error here, so
it's obvious if something is broken here.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode pull request #712: GEODE-3434: Allow the modules to be interoperable w...
Github user upthewaterspout commented on a diff in the pull request:
https://github.com/apache/geode/pull/712#discussion_r133827419
--- Diff:
extensions/geode-modules/src/main/java/org/apache/geode/modules/session/catalina/DeltaSession.java
---
@@ -586,6 +610,20 @@ public int getSizeInBytes() {
return serializedAttributes;
}
+ protected ConcurrentMap getAttributes() {
+try {
+ Field field = getAttributesFieldObject();
+ field.setAccessible(true);
+ Map oldMap = (Map) field.get(this);
+ ConcurrentMap newMap = new ConcurrentHashMap();
--- End diff --
Why do we need to copy the map here? I think the attributes field is always
set to a ConcurrentHashMap, even though in tomcat 6 the field was of type map.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode pull request #609: GEODE-2886 : sent IllegalStateException instead of ...
Github user upthewaterspout commented on a diff in the pull request:
https://github.com/apache/geode/pull/609#discussion_r131433628
--- Diff:
geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/distributed/WaitUntilFlushedFunction.java
---
@@ -85,8 +72,10 @@ public void execute(FunctionContext context) {
}
} else {
- throw new IllegalArgumentException(
+ IllegalStateException illegalStateException = new
IllegalStateException(
"The AEQ does not exist for the index " + indexName + " region "
+ region.getFullPath());
+ logger.error(illegalStateException.getMessage());
--- End diff --
I think it would be better here to just throw the IllegalStateException.
That makes it clear that we intend the exception to propagate back to the
caller.
Based on what I've seen from my testing, calling
resultSender.lastResult(new IllegalStateException()) will behave the same as
throwing the exception: on the remote side ResultCollector.getResult throws an
exception. However, I think that is kind of weird, whereas it makes sense that
if you throw an exception in the function it will propagate back as an
exception on the remote side.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode pull request #658: GEODE-3315: Replaced PreferBytes... with VMCachedDe...
Github user upthewaterspout closed the pull request at: https://github.com/apache/geode/pull/658 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode issue #658: GEODE-3315: Replaced PreferBytes... with VMCachedDeseriali...
Github user upthewaterspout commented on the issue: https://github.com/apache/geode/pull/658 Merged to develop in 56ea940d3c826e98b16d6b508fc834f7bd50220c. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode pull request #655: GEODE-3030: Set possibleDuplicate=true for all buck...
Github user upthewaterspout closed the pull request at: https://github.com/apache/geode/pull/655 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode pull request #658: GEODE-3315: Replaced PreferBytes... with VMCachedDe...
GitHub user upthewaterspout opened a pull request: https://github.com/apache/geode/pull/658 GEODE-3315: Replaced PreferBytes... with VMCachedDeserializable When getting a HAEventWrapper as part of a GII, make sure that we store the wrapper in a VMCachedDeserializable. This object needs to have a reference to the HAContainer. If PREFER_SERIALIZED is set to true, we we using a PreferBytesSerializable which would always create new copy of the HAEventWrapper. 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, you check travis-ci for build issues and submit an update to your PR as soon as possible. If you need help, please send an email to dev@geode.apache.org. You can merge this pull request into a Git repository by running: $ git pull https://github.com/upthewaterspout/incubator-geode feature/GEODE-3315 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode/pull/658.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #658 commit 68687f8f97bda1ba82ba1c192a6601b16ba7a67c Author: Barry Oglesby <bogle...@pivotal.io> Date: 2017-06-22T20:52:24Z GEODE-3315: Replaced PreferBytes... with VMCachedDeserializable When getting a HAEventWrapper as part of a GII, make sure that we store the wrapper in a VMCachedDeserializable. This object needs to have a reference to the HAContainer. If PREFER_SERIALIZED is set to true, we we using a PreferBytesSerializable which would always create new copy of the HAEventWrapper. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode pull request #655: GEODE-3030: Set possibleDuplicate=true for all buck...
GitHub user upthewaterspout opened a pull request: https://github.com/apache/geode/pull/655 GEODE-3030: Set possibleDuplicate=true for all bucket events after failover ### 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, you check travis-ci for build issues and submit an update to your PR as soon as possible. If you need help, please send an email to dev@geode.apache.org. You can merge this pull request into a Git repository by running: $ git pull https://github.com/upthewaterspout/incubator-geode feature/GEODE-3030 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode/pull/655.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #655 commit 2649d0faf3f64a1f25898f4b3f0ee512a7824f09 Author: Barry Oglesby <bogle...@pivotal.io> Date: 2017-06-12T22:04:59Z GEODE-3030: Set possibleDuplicate=true for all bucket events after failover --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode pull request #645: GEODE-3113: Modify HARegionQueue test to use Awaiti...
Github user upthewaterspout commented on a diff in the pull request:
https://github.com/apache/geode/pull/645#discussion_r128376504
--- Diff:
geode-core/src/test/java/org/apache/geode/internal/cache/ha/HARQAddOperationJUnitTest.java
---
@@ -287,12 +289,11 @@ public void testNoExpiryOnThreadIdentifier() {
eventsMap.get(threadId));
// wait for some more time to allow expiry on data
--- End diff --
Should probably remove this comment about waiting for some time?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode pull request #640: GEODE-3232: Get a snapshot of maps when serializing...
GitHub user upthewaterspout opened a pull request: https://github.com/apache/geode/pull/640 GEODE-3232: Get a snapshot of maps when serializing FilterProfile Avoiding a race when serializing a CopyOnWrite data structures be grabbing a copy first. 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, you check travis-ci for build issues and submit an update to your PR as soon as possible. If you need help, please send an email to dev@geode.apache.org. You can merge this pull request into a Git repository by running: $ git pull https://github.com/upthewaterspout/incubator-geode feature/GEODE-3232 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode/pull/640.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #640 commit aaba2aae27c5b4f57d3bfba6220f6dfff60222f0 Author: Dan Smith <upthewatersp...@apache.org> Date: 2017-07-17T23:46:19Z GEODE-3232: Get a snapshot of maps when serializing FilterProfile Avoiding a race when serializing a CopyOnWrite data structures be grabbing a copy first. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode issue #640: GEODE-3232: Get a snapshot of maps when serializing Filter...
Github user upthewaterspout commented on the issue: https://github.com/apache/geode/pull/640 @ladyVader @nabarunnag --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode-examples issue #9: GEODE-3199: Make signing with a gpg key optional
Github user upthewaterspout commented on the issue: https://github.com/apache/geode-examples/pull/9 @metatype - I updated the PR. Travis is now stuck on find a 1.2 geode distribution, but it got past the signing check. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode-examples issue #9: GEODE-3199: Make signing with a gpg key optional
Github user upthewaterspout commented on the issue: https://github.com/apache/geode-examples/pull/9 Sure, I'll change it. I was using askpass just because that mirrored what the main geode repo uses, but I think signArtifacts or something like that is clearer. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode-examples pull request #9: GEODE-3199: Make signing with a gpg key opti...
GitHub user upthewaterspout opened a pull request: https://github.com/apache/geode-examples/pull/9 GEODE-3199: Make signing with a gpg key optional Make it optional to sign the archives with a gpg key, to avoid annoying users trying to build the examples. You can merge this pull request into a Git repository by running: $ git pull https://github.com/upthewaterspout/geode-examples feature/GEODE-3199 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode-examples/pull/9.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #9 commit c6b110abece7b0801054e8f44b0f430155391e90 Author: Dan Smith <upthewatersp...@apache.org> Date: 2017-07-12T22:30:13Z GEODE-3199: Make signing with a gpg key optional Make it optional to sign the archives with a gpg key, to avoid annoying users trying to build the examples. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode-examples pull request #7: GEODE-3196: Fixing the format of the .md5 an...
GitHub user upthewaterspout opened a pull request: https://github.com/apache/geode-examples/pull/7 GEODE-3196: Fixing the format of the .md5 and .sha256 files You can merge this pull request into a Git repository by running: $ git pull https://github.com/upthewaterspout/geode-examples feature/GEODE-3196 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode-examples/pull/7.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #7 commit 2ee271b134e6c49e0b7920bb147c475c03722c03 Author: Dan Smith <upthewatersp...@apache.org> Date: 2017-07-12T20:05:03Z GEODE-3196: Fixing the format of the .md5 and .sha256 files --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode issue #626: GEODE-3179: Fixing bundled jars test finding slf4j-jdk jar
Github user upthewaterspout commented on the issue: https://github.com/apache/geode/pull/626 +1 I'll merge this. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode issue #620: GEODE-3172: Fix serialization errors with client queues fr...
Github user upthewaterspout commented on the issue: https://github.com/apache/geode/pull/620 After reviewing these changes with @jhuynh1 - it looks like the HAEventWrapper will never get serialized - we don't configure eviction on the regions that hold HAEventWrapper. So these changes should work. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode issue #620: GEODE-3172: Fix serialization errors with client queues fr...
Github user upthewaterspout commented on the issue: https://github.com/apache/geode/pull/620 Head's up - I'm not sure my changes to fix 1.2->1.0 copying of the queue are going to completely work. If the HAEventWrapper gets serialized for some reason (maybe evicted to disk?) Then I don't think the 1.2 member will translate it to the 1.0 format. I'm looking into this now. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode issue #620: GEODE-3172: Fix serialization errors with client queues fr...
Github user upthewaterspout commented on the issue: https://github.com/apache/geode/pull/620 @jhuynh1 @ladyVader @bschuchardt @hiteshk25 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode pull request #620: GEODE-3172: Fix serialization errors with client qu...
GitHub user upthewaterspout opened a pull request: https://github.com/apache/geode/pull/620 GEODE-3172: Fix serialization errors with client queues from between 1.0 and 1.2 Adding new tests and fixing errors when copying a client queue from a 1.0 member to a 1.2 member, or from a 1.2 member to a 1.0 member. We were not respecting the version of the 1.0 member when deserializing or serializing the HAEventWrapper class, which is stored as a value in a geode region. This will affect *all* GII's between different versions, because I had to change the GII code to serialize values using the version of the GII recipient. 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, you check travis-ci for build issues and submit an update to your PR as soon as possible. If you need help, please send an email to dev@geode.apache.org. You can merge this pull request into a Git repository by running: $ git pull https://github.com/upthewaterspout/incubator-geode feature/GEODE-3172 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode/pull/620.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #620 commit 607d5d283dca5cd030d03d6201d6b5b4677d9b04 Author: Dan Smith <upthewatersp...@apache.org> Date: 2017-07-06T22:08:04Z GEODE-3172: Test of GII'ing client queue from 9.0 to 9.1 Adding a test that demonstrates the serialization issues we are seeing with this issue. The test starts a 9.0 server, puts some data in the queue and starts a 9.1 server. The test fails because the 9.1 server never gets the queue contents. In the log the serialization error is present. commit f983511edc2d83560f436f350e1b445469d98dfc Author: Dan Smith <upthewatersp...@apache.org> Date: 2017-07-06T22:07:44Z GEODE-3172: Deserialize HAEventWrapper using peer version Rather than deserialize the event wrapper using the current version, use the version of the peer that sent the wrapper too this member. This is a partial fix for this issue, because it only fixes the case of 1.2 copying the queue from 1.0. If a 1.0 member copies the queue from 1.2, there is still an issue. commit e35333f940f66ad904be410ab80076506bc83ca4 Author: Dan Smith <upthewatersp...@apache.org> Date: 2017-07-07T01:07:53Z GEODE-3172: Writing region entries using the GII recipients version We were serializing entries to the remote member using Version.CURRENT, rather than the version of the remote member, when sending entries as part of GII. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode pull request #603: GEODE-3128: Changed the batch size to 1000 when cre...
Github user upthewaterspout commented on a diff in the pull request: https://github.com/apache/geode/pull/603#discussion_r125094490 --- Diff: geode-lucene/src/test/java/org/apache/geode/cache/lucene/LuceneQueriesIntegrationTest.java --- @@ -59,7 +64,7 @@ @Rule public ExpectedException thrown = ExpectedException.none(); private static final String INDEX_NAME = "index"; - protected static final String REGION_NAME = "index"; + public static final String REGION_NAME = "index"; --- End diff -- I don't think this needed to be made public - I think maybe the new test is importing the wrong static REGION_NAME? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode issue #608: GEODE-3140: Removed DiskDirRule and replaced use with Temp...
Github user upthewaterspout commented on the issue: https://github.com/apache/geode/pull/608 +1 - I'll merge this. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode issue #584: GEODE-2901: Adding integration tests of session replicatio...
Github user upthewaterspout commented on the issue: https://github.com/apache/geode/pull/584 +1 - Looks good to me! --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode pull request #570: GEODE-3055: waitUntilFlush should use data region's...
Github user upthewaterspout commented on a diff in the pull request:
https://github.com/apache/geode/pull/570#discussion_r122085382
--- Diff:
geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegionDataStore.java
---
@@ -1472,6 +1472,19 @@ public boolean removeBucket(int bucketId, boolean
forceRemovePrimary) {
}
BucketAdvisor bucketAdvisor = bucketRegion.getBucketAdvisor();
+ InternalDistributedMember primary = bucketAdvisor.getPrimary();
+ InternalDistributedMember myId =
+
this.partitionedRegion.getDistributionManager().getDistributionManagerId();
+ if (primary == null || myId.equals(primary)) {
--- End diff --
This seems similar to the logic a few lines down where we say "if
(!forceRemovePrimary && bucketAdvisor.isPrimary()) {..."
Unlike that line, your new logic doesn't honor the forceRemovePrimary flag.
Should it? I don't actually see any cases where that is passed in as true, so
maybe we should just remove that flag?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode issue #538: GEODE-2975: Change the location of the lucene xsd
Github user upthewaterspout commented on the issue: https://github.com/apache/geode/pull/538 @boglesby - Let me know what you think of the new changes. I added some validation to the code. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode issue #538: GEODE-2975: Change the location of the lucene xsd
Github user upthewaterspout commented on the issue: https://github.com/apache/geode/pull/538 @boglesby - I see the problem with lucene:indexxx. It looks like in cache-1.0.xsd, which specify this in order to allow other schemas (like lucene.xsd) to extend region with additional elements: ` ` Unfortunately, using 'lax' instead of 'strict' here means that we don't validate the new xsd as strictly - additional elements are ignored, etc. I'm worried about the impact of changing this validation. I think the thing to do maybe is just to give a little better error message in our java code and call it a day. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode-site pull request #6: Adding an assemble task for travis to invoke
GitHub user upthewaterspout opened a pull request: https://github.com/apache/geode-site/pull/6 Adding an assemble task for travis to invoke Travis always invokes assemble for a gradle project, apparently. You can merge this pull request into a Git repository by running: $ git pull https://github.com/upthewaterspout/geode-site feature/fix_travis Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode-site/pull/6.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #6 commit cd837fabec838f5cdd794bc02696288069a0969f Author: Dan Smith <upthewatersp...@apache.org> Date: 2017-05-30T18:22:59Z Adding an assemble task for travis to invoke Travis always invokes assemble for a gradle project, apparently. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode issue #538: GEODE-2975: Change the location of the lucene xsd
Github user upthewaterspout commented on the issue: https://github.com/apache/geode/pull/538 @metatype - Yes, we'll need to update the website as well, the location of the xsd is also wrong on the website, but for different reasons. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode issue #538: GEODE-2975: Change the location of the lucene xsd
Github user upthewaterspout commented on the issue: https://github.com/apache/geode/pull/538 @nabarunnag @boglesby --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode pull request #538: GEODE-2975: Change the location of the lucene xsd
GitHub user upthewaterspout opened a pull request: https://github.com/apache/geode/pull/538 GEODE-2975: Change the location of the lucene xsd The lucene xsd was not in the right location on disk, which resulted in a failure to validate lucene elements in the xml file. 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? - [X] 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, you check travis-ci for build issues and submit an update to your PR as soon as possible. If you need help, please send an email to dev@geode.apache.org. You can merge this pull request into a Git repository by running: $ git pull https://github.com/apache/geode feature/GEODE-2975 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode/pull/538.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #538 commit 313febd9eb1cc2b89ab790d00183010b9d5d27b2 Author: Dan Smith <upthewatersp...@apache.org> Date: 2017-05-25T20:05:23Z GEODE-2975: Change the location of the lucene xsd The lucene xsd was not in the right location on disk, which resulted in a failure to validate lucene elements in the xml file. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode pull request #506: GEODE-2900: DUnit test of moving primary during AEQ...
GitHub user upthewaterspout opened a pull request: https://github.com/apache/geode/pull/506 GEODE-2900: DUnit test of moving primary during AEQ dispatching Adding a dunit test that moves the primary and moves it again during AEQ dispatching. @jhuynh1 - I based this test on top of your PR for GEODE-2900. Feel free to merge it in if the test looks good to you. This test fails without your changes and passes with those changes. You can merge this pull request into a Git repository by running: $ git pull https://github.com/upthewaterspout/incubator-geode feature/GEODE-2900-test Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode/pull/506.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #506 commit 7cf0be13ed2abe9a707a2b4fe0ded66f0870da77 Author: Dan Smith <upthewatersp...@apache.org> Date: 2017-05-10T23:32:05Z GEODE-2900: DUnit test of moving primary during AEQ dispatching Adding a dunit test that moves the primary and moves it again during AEQ dispatching. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode pull request #503: GEODE-2907: Removed @Experimental tag from the Luce...
Github user upthewaterspout commented on a diff in the pull request:
https://github.com/apache/geode/pull/503#discussion_r115624028
--- Diff:
geode-lucene/src/main/java/org/apache/geode/cache/lucene/LuceneQuery.java ---
@@ -60,7 +82,16 @@
* the limit specified by {@link #getLimit()} A {@link
LuceneResultStruct} contains the region
* key, value, and a score for that entry.
*
+ * @return a List of LuceneResultStruct that match the Lucene query
* @throws LuceneQueryException if the query could not be parsed or
executed.
+ * @throws CacheClosedException if the cache was closed while the Lucene
query was being executed.
+ * @throws PrimaryBucketException if the primary bucket was moved from
the node on which the
--- End diff --
Another PrimaryBucketException
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode pull request #503: GEODE-2907: Removed @Experimental tag from the Luce...
Github user upthewaterspout commented on a diff in the pull request:
https://github.com/apache/geode/pull/503#discussion_r115623920
--- Diff:
geode-lucene/src/main/java/org/apache/geode/cache/lucene/LuceneQuery.java ---
@@ -37,21 +42,38 @@
* Results are returned in order of their score with respect to this query.
*
*/
-@Experimental
public interface LuceneQuery<K, V> {
/**
* Execute the query and return the region keys that match this query,
up to the limit specified
* by {@link #getLimit()}.
- *
+ *
+ * @return Collection of Apache Geode region keys that satisfy the
Lucene query.
* @throws LuceneQueryException if the query could not be parsed or
executed.
+ * @throws CacheClosedException if the cache was closed while the Lucene
query was being executed.
+ * @throws PrimaryBucketException if the primary bucket was moved from
the node on which the
--- End diff --
PrimaryBucketException is an internal exception, which should not be part
of the API. Do we actually throw this exception from this method? It seems like
we should be retrying in any case.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode issue #494: GEODE-2889: Update the last access time of native sessions
Github user upthewaterspout commented on the issue: https://github.com/apache/geode/pull/494 @jhuynh1 @jdeppe-pivotal --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode pull request #494: GEODE-2889: Update the last access time of native s...
GitHub user upthewaterspout opened a pull request: https://github.com/apache/geode/pull/494 GEODE-2889: Update the last access time of native sessions Our getSession method had some races where we were holding onto a reference to the native session, but not updating the last accessed time of the native session by calling into getSession of the container. This could allow the container to expire the session in the middle of our method. You can merge this pull request into a Git repository by running: $ git pull https://github.com/upthewaterspout/incubator-geode feature/GEODE-2889 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode/pull/494.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #494 commit a9a79d54c782e13763f0c4a1b3bef4293804fb7e Author: Dan Smith <upthewatersp...@apache.org> Date: 2017-05-03T22:49:57Z GEODE-2889: Update the last access time of native sessions Our getSession method had some races where we were holding onto a reference to the native session, but not updating the last accessed time of the native session by calling into getSession of the container. This could allow the container to expire the session in the middle of our method. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode issue #489: GEODE-1728: Http session filter should only apply to REQUE...
Github user upthewaterspout commented on the issue: https://github.com/apache/geode/pull/489 @joeymcallister @davebarnes97 - can you review? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode pull request #489: GEODE-1728: Http session filter should only apply t...
GitHub user upthewaterspout opened a pull request: https://github.com/apache/geode/pull/489 GEODE-1728: Http session filter should only apply to REQUEST The docs should not tell users to add apply the session filter to FORWARD, INCLUDE, and ERROR dispatchers. It should just be the default, REQUEST. You can merge this pull request into a Git repository by running: $ git pull https://github.com/upthewaterspout/incubator-geode feature/GEODE-1728 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode/pull/489.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #489 commit ce634f4ab194bf378a8890ed1f3069396e8ce72a Author: Dan Smith <upthewatersp...@apache.org> Date: 2017-05-03T17:16:41Z GEODE-1728: Http session filter should not be applied to FORWARDS The docs should not tell users to add apply the session filter to FORWARD, INCLUDE, and ERROR dispatchers. It should just be the default, REQUEST. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode pull request #481: GEODE-2828: AEQ being created before the user regio...
Github user upthewaterspout commented on a diff in the pull request:
https://github.com/apache/geode/pull/481#discussion_r114158872
--- Diff:
geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/LuceneIndexForPartitionedRegion.java
---
@@ -78,17 +87,9 @@ protected RepositoryManager createRepositoryManager() {
// create PR fileAndChunkRegion, but not to create its buckets for now
final String fileRegionName = createFileRegionName();
PartitionAttributes partitionAttributes =
dataRegion.getPartitionAttributes();
-
-
-// create PR chunkRegion, but not to create its buckets for now
-
-// we will create RegionDirectories on the fly when data comes in
-HeterogeneousLuceneSerializer mapper = new
HeterogeneousLuceneSerializer(getFieldNames());
-PartitionedRepositoryManager partitionedRepositoryManager =
-new PartitionedRepositoryManager(this, mapper);
DM dm =
this.cache.getInternalDistributedSystem().getDistributionManager();
LuceneBucketListener lucenePrimaryBucketListener =
-new LuceneBucketListener(partitionedRepositoryManager, dm);
+new LuceneBucketListener((PartitionedRepositoryManager)
partitionedRepositoryManager, dm);
--- End diff --
Having to cast is a bit of a bad smell here. It seems like either
LuceneBucketListener should take an AbstractPartitionedRepositoryManager, or
createLuceneListenersAndFileChunkRegions should have been passed in a
PartitionRespositoryManager.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode pull request #481: GEODE-2828: AEQ being created before the user regio...
Github user upthewaterspout commented on a diff in the pull request:
https://github.com/apache/geode/pull/481#discussion_r114158965
--- Diff:
geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/LuceneIndexImpl.java
---
@@ -131,30 +131,41 @@ protected void initialize() {
if (!hasInitialized) {
/* create index region */
dataRegion = getDataRegion();
- // assert dataRegion != null;
-
- repositoryManager = createRepositoryManager();
-
- // create AEQ, AEQ listener and specify the listener to
repositoryManager
- createAEQ(dataRegion);
-
+ createLuceneListenersAndFileChunkRegions(
+ (AbstractPartitionedRepositoryManager) repositoryManager);
--- End diff --
More casting going on here.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode pull request #479: GEODE-2828: AEQ created before the user region
Github user upthewaterspout commented on a diff in the pull request: https://github.com/apache/geode/pull/479#discussion_r113564312 --- Diff: geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/LuceneEventListener.java --- @@ -53,6 +54,8 @@ Logger logger = LogService.getLogger(); private final RepositoryManager repositoryManager; + private CountDownLatch isFileAndChunkRegionReady = new CountDownLatch(1); + private boolean ready = false; --- End diff -- Does this need to be a separate flag? Seems like the CoutDownLatch already has the flag. If not this should be volatile. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode issue #467: GEODE-258: Remove deprecated Cache.getLoggerI18n and getSe...
Github user upthewaterspout commented on the issue: https://github.com/apache/geode/pull/467 I'm not quite sure about this one. LogWriter and LoggerI18n are also deprecated. I believe they were deprecated because geode is now using log4j2. @dschneider-pivotal - It looks like this PR contains the changes suggested in the ticket, but is just removing these two methods really what we want to do? My other query is that log4j2 was introduced fairly recently, so is it too soon to remove LogWriter and LogWriterI18n? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode issue #465: GEODE-253: Removed depreated and not used EntryNotFoundInR...
Github user upthewaterspout commented on the issue: https://github.com/apache/geode/pull/465 I think it's good to merge! --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode issue #466: GEODE-255: Removed deprecated DataSerializer.register(Clas...
Github user upthewaterspout commented on the issue: https://github.com/apache/geode/pull/466 I think it's good to merge! --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode issue #468: GEODE-260: Remove deprecated RemoteTransactionException
Github user upthewaterspout commented on the issue: https://github.com/apache/geode/pull/468 I think it's good to merge. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode issue #469: GEODE-266: Remove the deprecated ObjectSizerImpl class
Github user upthewaterspout commented on the issue: https://github.com/apache/geode/pull/469 I think it's good to merge. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode-examples issue #4: Updating and simplifying examples
Github user upthewaterspout commented on the issue: https://github.com/apache/geode-examples/pull/4 Is lombok supposed to work in my IDE without installing a plugin? For me this code has compile errors in intellij. I like the fact that you got rid of the shell scripts in favor of gfsh scripts, seems like it makes the example a lot simpler looking! I think this might be going a little too far in terms of removing tests. There are definitely things in the existing code that runAll is not testing since all you do is System.out.println. Beyond that, I think showing how to write code using geode that has good unit tests might be part of what makes a good example of geode? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode issue #472: GEODE-2808 - Fixing lock ordering issues in DeltaSession
Github user upthewaterspout commented on the issue: https://github.com/apache/geode/pull/472 @jhuynh1 @boglesby --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode pull request #472: GEODE-2808 - Fixing lock ordering issues in DeltaSe...
GitHub user upthewaterspout opened a pull request: https://github.com/apache/geode/pull/472 GEODE-2808 - Fixing lock ordering issues in DeltaSession Region expiration of sessions and explicit expiration of sessions had lock ordering issues. Fixing the code so that expiration goes through the region entry lock first, before getting the lock on StandardSession. Adding a workaround for the fact that liferay calls removeAttribute from within session expiration by ignoreing remoteAttribute calls during expiration. You can merge this pull request into a Git repository by running: $ git pull https://github.com/upthewaterspout/incubator-geode feature/GEODE-2808 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode/pull/472.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #472 commit 8667c805d4a7b77cca2c883e34200169beab585a Author: Dan Smith <upthewatersp...@apache.org> Date: 2017-04-21T18:36:24Z GEODE-2808 - Fixing lock ordering issues in DeltaSession Region expiration of sessions and explicit expiration of sessions had lock ordering issues. Fixing the code so that expiration goes through the region entry lock first, before getting the lock on StandardSession. Adding a workaround for the fact that liferay calls removeAttribute from within session expiration by ignoreing remoteAttribute calls during expiration. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode issue #450: GEODE-2632: create ClientCachePutBench
Github user upthewaterspout commented on the issue: https://github.com/apache/geode/pull/450 Regarding putting JMH benchmarks in the core - seems fine. I think I originally made geode-benchmarks a separate project so it would be easy to share code and compare benchmarks between modules - eg comparing lucene to OQL queries. But putting the benchmarks in each module maybe makes more sense. This does seem to be somewhat stretching what JMH is designed for. JMH is targeted towards *microbenchmarks* so launching a separate server process seems a bit of a stretch. In particular, it's not clear to me here whether your server is getting restarted between benchmark iterations. JMH specifically tries to restart the JVM multiple times to deal with inconsistencies, but maybe only your client is getting restarted? In general I think we should probably be focusing on single VM, smaller unit benchmarks with JMH - benchmarking distributed systems might be better done with a different framework and multiple hosts. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode issue #321: [GEODE-1577] Unhelpful generic types on Execution.execute
Github user upthewaterspout commented on the issue: https://github.com/apache/geode/pull/321 +1 these latest changes look good to me! --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode issue #430: GEODE-2690: Created a new thread pool for flush operations
Github user upthewaterspout commented on the issue: https://github.com/apache/geode/pull/430 I Think it would be better to figure out why we a creating a ton of threads, rather than create a thread pool specifically just for this one message. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode issue #427: GEODE-2674: Changing the lucene listener to get from the r...
Github user upthewaterspout commented on the issue: https://github.com/apache/geode/pull/427 Changes are merged --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode pull request #427: GEODE-2674: Changing the lucene listener to get fro...
Github user upthewaterspout closed the pull request at: https://github.com/apache/geode/pull/427 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode issue #426: GEODE-2660 Add @Experimental to the Redis adapter.
Github user upthewaterspout commented on the issue: https://github.com/apache/geode/pull/426 @galen-pivotal - it looks like the public API is just GeodeRedisServer. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode pull request #427: GEODE-2674: Changing the lucene listener to get fro...
Github user upthewaterspout commented on a diff in the pull request: https://github.com/apache/geode/pull/427#discussion_r106504069 --- Diff: geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/LuceneEventSubstitutionFilter.java --- @@ -0,0 +1,37 @@ +/* + * 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.cache.lucene.internal; + +import org.apache.geode.cache.EntryEvent; +import org.apache.geode.cache.wan.GatewayEventSubstitutionFilter; +import org.apache.geode.internal.cache.Token; + +/** + * A substitution filter which throws away the value of the entry and replaces it with an INVALID --- End diff -- Hmm, I was thinking null would just mean remove the event or don't substitute the value. But maybe the gateway actually handles that appropriately. Let me test that. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode pull request #427: GEODE-2674: Changing the lucene listener to get fro...
Github user upthewaterspout commented on a diff in the pull request: https://github.com/apache/geode/pull/427#discussion_r106484913 --- Diff: geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/LuceneEventSubstitutionFilter.java --- @@ -0,0 +1,37 @@ +/* + * 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.cache.lucene.internal; + +import org.apache.geode.cache.EntryEvent; +import org.apache.geode.cache.wan.GatewayEventSubstitutionFilter; +import org.apache.geode.internal.cache.Token; + +/** + * A substitution filter which throws away the value of the entry and replaces it with an INVALID --- End diff -- You are right. I tried Token.NOT_AVAILABLE and then Token.INVALID, but that made the queue unhappy. I'll change the comment. I thought I was using a static string? Is there something that would be better to use here? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode pull request #427: GEODE-2674: Changing the lucene listener to get fro...
GitHub user upthewaterspout opened a pull request: https://github.com/apache/geode/pull/427 GEODE-2674: Changing the lucene listener to get from the region Rather than use the value that is in the queue, use the latest value from the region to update the lucene index. This ensures that even if the queue contains spurious events due to retries or other issues, we put the correct value in the index. This also potentially saves memory and disk space for the queue, because the queue does not need to hold the value for the entry. @boglesby @gesterzhou @jhuynh1 @ladyVader @nabarunnag You can merge this pull request into a Git repository by running: $ git pull https://github.com/upthewaterspout/incubator-geode feature/GEODE-2674 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode/pull/427.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #427 commit 9f9439ab0ec6c2a0221e78aa5825e9a0759bac80 Author: Dan Smith <upthewatersp...@apache.org> Date: 2017-03-15T20:23:20Z GEODE-2674: Changing the lucene listener to fetch the value from the region Rather than use the value that is in the queue, use the latest value from the region to update the lucene index. This ensures that even if the queue contains spurious events due to retries or other issues, we put the correct value in the index. This also potentially saves memory and disk space for the queue, because the queue does not need to hold the value for the entry. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode issue #421: GEODE-2595: Change LuceneService.createIndex to use a fact...
Github user upthewaterspout commented on the issue: https://github.com/apache/geode/pull/421 Good catch, Gester! I'll fix that. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode issue #421: GEODE-2595: Change LuceneService.createIndex to use a fact...
Github user upthewaterspout commented on the issue: https://github.com/apache/geode/pull/421 Good catch Anthony. I've pushed a fix. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode pull request #421: GEODE-2595: Change LuceneService.createIndex to use...
GitHub user upthewaterspout opened a pull request: https://github.com/apache/geode/pull/421 GEODE-2595: Change LuceneService.createIndex to use a factory Changing LuceneService.createIndex to createIndexFactory and using a factory pattern to create the index. This allows us to introduce new options to the index create without breaking backwards compatibility in the future. @ladyVader @nabarunnag @boglesby You can merge this pull request into a Git repository by running: $ git pull https://github.com/upthewaterspout/incubator-geode feature/GEODE-2595 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode/pull/421.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #421 commit f912be793ff7a73855df903148d4b74dfdc415b9 Author: Dan Smith <upthewatersp...@apache.org> Date: 2017-03-09T01:15:44Z GEODE-2595: Change LuceneService.createIndex to use a factory Changing LuceneService.createIndex to createIndexFactory and using a factory pattern to create the index. This allows us to introduce new options to the index create without breaking backwards compatibility in the future. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode pull request #420: GEODE-2635: Creating DUnit tests to test eviction i...
Github user upthewaterspout commented on a diff in the pull request:
https://github.com/apache/geode/pull/420#discussion_r105061375
--- Diff:
geode-lucene/src/test/java/org/apache/geode/cache/lucene/EvictionDUnitTest.java
---
@@ -0,0 +1,145 @@
+/*
+ * 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.cache.lucene;
+
+import static
org.apache.geode.cache.lucene.test.LuceneTestUtilities.INDEX_NAME;
+import static
org.apache.geode.cache.lucene.test.LuceneTestUtilities.REGION_NAME;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
+
+import org.apache.geode.cache.Cache;
+import org.apache.geode.cache.Region;
+import org.apache.geode.distributed.internal.DistributionConfig;
+import org.apache.geode.internal.cache.GemFireCacheImpl;
+import org.apache.geode.internal.cache.PartitionedRegion;
+import org.apache.geode.internal.cache.control.HeapMemoryMonitor;
+import org.apache.geode.test.dunit.SerializableRunnableIF;
+import org.apache.geode.test.junit.categories.DistributedTest;
+import org.awaitility.Awaitility;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.junit.runner.RunWith;
+
+import java.util.List;
+import java.util.concurrent.TimeUnit;
+import java.util.stream.IntStream;
+import junitparams.JUnitParamsRunner;
+import junitparams.Parameters;
+
+
+@Category(DistributedTest.class)
+@RunWith(JUnitParamsRunner.class)
+public class EvictionDUnitTest extends LuceneQueriesAccessorBase {
+
+ protected final static float INITIAL_EVICTION_HEAP_PERCENTAGE = 50.9f;
+ protected final static float EVICTION_HEAP_PERCENTAGE_FAKE_NOTIFICATION
= 85.0f;
+ protected final static int TEST_MAX_MEMORY = 100;
+ protected final static int MEMORY_USED_FAKE_NOTIFICATION = 90;
+
+ protected RegionTestableType[]
getPartitionRedundantOverflowEvictionRegionType() {
+return new RegionTestableType[]
{RegionTestableType.PARTITION_REDUNDANT_EVICTION_OVERFLOW,
+
RegionTestableType.PARTITION_PERSISTENT_REDUNDANT_EVICTION_OVERFLOW};
+ }
+
+ protected RegionTestableType[]
getPartitionRedundantLocalDestroyEvictionRegionType() {
+return new RegionTestableType[]
{RegionTestableType.PARTITION_REDUNDANT_EVICTION_LOCAL_DESTROY};
+ }
+
+ @Test
+ @Parameters(method =
"getPartitionRedundantLocalDestroyEvictionRegionType")
+ public void
regionWithEvictionWithLocalDestroyMustNotbeAbleToCreateLuceneIndexes(
+ RegionTestableType regionTestType) {
+SerializableRunnableIF createIndex = () -> {
+ LuceneService luceneService = LuceneServiceProvider.get(getCache());
+ luceneService.createIndex(INDEX_NAME, REGION_NAME, "text");
+};
+
+dataStore1.invoke(() -> {
+ try {
+initDataStore(createIndex, regionTestType);
+ } catch (UnsupportedOperationException e) {
+assertEquals(
+"Lucene indexes on regions with eviction and action local
destroy are not supported",
+e.getMessage());
+assertNull(getCache().getRegion(REGION_NAME));
+ }
+});
+
+ }
+
+ @Test
+ @Parameters(method = "getPartitionRedundantOverflowEvictionRegionType")
+ public void
regionsWithEvictionWithOverflowMustBeAbleToCreateLuceneIndexes(
+ RegionTestableType regionTestType) {
+SerializableRunnableIF createIndex = () -> {
+ LuceneService luceneService = LuceneServiceProvider.get(getCache());
+ luceneService.createIndex(INDEX_NAME, REGION_NAME, "text");
+};
+
+dataStore1.invoke(() -> initDataStore(createIndex, regionTestType));
+
+accessor.invoke(() -> initDataStore(createIndex, regionTestType));
+
+accessor.invoke(() -> {
+ Cache cache = getCache();
+
[GitHub] geode-examples pull request #3: GEODE-2231 A partitioned region example
Github user upthewaterspout commented on a diff in the pull request: https://github.com/apache/geode-examples/pull/3#discussion_r103763443 --- Diff: partitioned/src/main/java/org/apache/geode/examples/partitioned/BaseClient.java --- @@ -0,0 +1,79 @@ +/* --- End diff -- Nitpick - I feel like this BaseClient actually makes the code less readable. It might be better to just create the caches and regions inline in the producer and consumer, and just have a static class that has the common properties. Also, getRegion1 and getRegion2 aren't very descriptive method names. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode-examples pull request #3: GEODE-2231 A partitioned region example
Github user upthewaterspout commented on a diff in the pull request: https://github.com/apache/geode-examples/pull/3#discussion_r103761595 --- Diff: partitioned/README.md --- @@ -0,0 +1,276 @@ + + +# Geode partitioned region example + +This example demonstrates the basic property of partitioning, as well --- End diff -- It might be good to reflow the text here, if someone is looking at it with a regular text editor it looks a little choppy. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode-examples pull request #3: GEODE-2231 A partitioned region example
Github user upthewaterspout commented on a diff in the pull request:
https://github.com/apache/geode-examples/pull/3#discussion_r103763778
--- Diff:
partitioned/src/main/java/org/apache/geode/examples/partitioned/Consumer.java
---
@@ -0,0 +1,81 @@
+/*
+ * 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.examples.partitioned;
+
+import java.util.*;
+import org.apache.geode.cache.client.ClientCache;
+import org.apache.geode.cache.Region;
+
+public class Consumer extends BaseClient {
+
+ public static void main(String[] args) {
+Consumer c = new Consumer();
+c.checkAndPrint(args);
+ }
+
+ public void checkAndPrint(String[] args) {
+if (0 == args.length) {
+ throw new RuntimeException("Expected argument specifying region
name.");
+} else {
--- End diff --
Nitpick - no need for an else. If (0 == args.length), your method was over
when it threw an exception. This could help from having a bunch of nested
levels of if/else here.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode-examples pull request #3: GEODE-2231 A partitioned region example
Github user upthewaterspout commented on a diff in the pull request:
https://github.com/apache/geode-examples/pull/3#discussion_r103763868
--- Diff:
partitioned/src/main/java/org/apache/geode/examples/partitioned/Consumer.java
---
@@ -0,0 +1,81 @@
+/*
+ * 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.examples.partitioned;
+
+import java.util.*;
+import org.apache.geode.cache.client.ClientCache;
+import org.apache.geode.cache.Region;
+
+public class Consumer extends BaseClient {
+
+ public static void main(String[] args) {
+Consumer c = new Consumer();
+c.checkAndPrint(args);
+ }
+
+ public void checkAndPrint(String[] args) {
+if (0 == args.length) {
+ throw new RuntimeException("Expected argument specifying region
name.");
+} else {
+ if (args.length > 1) {
+throw new RuntimeException("Expected only 1 argument, and received
more than 1.");
+ } else {
+if (args[0].equals("EmployeeRegion")) {
+ this.printRegionContents();
+} else {
+ if (args[0].equals("BadEmployeeRegion")) {
+this.printBadRegionContents();
+ } else {
+throw new RuntimeException("Unrecognized region name in
argument specification.");
+ }
+}
+ }
+}
+ }
+
+ public Consumer() {}
+
+ public Consumer(ClientCache clientCache) {
+this.clientCache = clientCache;
+ }
+
+
+ public void printRegionContents() {
+/* Print EmployeeRegion size and contents */
+Region r1 = this.getRegion1();
+Set setOfKeys1 = r1.keySetOnServer();
+logger.info(setOfKeys1.size() + " entries in EmployeeRegion on the
server(s).");
+if (setOfKeys1.size() > 0) {
--- End diff --
Nitpick - no need for this if check I think.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode issue #410: Overhauling the javadocs for the LuceneService
Github user upthewaterspout commented on the issue: https://github.com/apache/geode/pull/410 @jhuynh1 @karensmolermiller @gesterzhou --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode issue #353: GEODE-2269 update to allow region entries non null empty k...
Github user upthewaterspout commented on the issue: https://github.com/apache/geode/pull/353 +1 I'll merge the non .gitignore changes. I don't think we should be ignoring directories named bin, we actually have some code checked in those directories. I suspect maybe you are using eclipse? The default eclipse behavior is to create an output directory called bin. If so, you should use ./gradlew eclipse and then just import the generated eclipse project into your workspace. Not only will that fix the output directory but it will setup your classpath for you! --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode issue #321: [GEODE-1577] Unhelpful generic types on Execution.execute
Github user upthewaterspout commented on the issue:
https://github.com/apache/geode/pull/321
Hi all,
I'm really sorry I missed this. I was out of town for a while and somehow
the discussion slipped through the cracks when I was catching up.
These changes are what I had in mind, but I didn't realize that this would
break existing code that was doing a cast. I know adding a generics should be
safe, but in this case we're fixing generics that are broken which is why I
think we are getting into trouble.
I did a little messing around. It looks like we might be able to get away
with adding generics to Execution.
The only caveat is that once we add generics to Execution we are stuck with
them. There is an existing bug to also add an argument type to the
FunctionContext - see GEODE-2217. So I think maybe the thing to do is to add
three types parameters to execution. Something like this:
```
/**
*
* @param The type of the argument passed into the function, if any
* @param The type of the aggregrated result returned by the
ResultCollector
*/
public interface Execution<IN, OUT,AGG> {
Execution<IN, OUT, AGG> withArgs(IN args);
ResultCollector<OUT, AGG> execute(...)
}
```
What do you think? The other option I suppose is to deprecate the Execution
class and introduce a new one, or deprecate the execute methods and introduce
new ones, but that seems painful.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode issue #344: Fix the Native Client Build on Ubuntu
Github user upthewaterspout commented on the issue: https://github.com/apache/geode/pull/344 This PR fixed the issue for me on ubuntu. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode issue #341: GEODE-2306: Update native client BUILDING.md to reflect ch...
Github user upthewaterspout commented on the issue: https://github.com/apache/geode/pull/341 @pivotal-jbarrett - Here's a jira - https://issues.apache.org/jira/browse/GEODE-2317. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode-examples issue #2: Build updates for examples
Github user upthewaterspout commented on the issue: https://github.com/apache/geode-examples/pull/2 Wowsers, good stuff! A couple of comments/questions: 1. Should we really remove gradlew.bat? That means windows users won't be able to run the examples. 2. Since you are using the mirrors, it's probably a good idea to verify the downloaded geode binary before running it, to protect against a malicious mirror. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode issue #342: GEODE-2309: Replace Pivotal Copyright and add Apache Licen...
Github user upthewaterspout commented on the issue: https://github.com/apache/geode/pull/342 Looks good so far, but there are still a lot of files missing apache headers. I don't know how easy it will be to integrate [apache rat](https://creadur.apache.org/rat/) into the cmake build, but for now you can run it from the command line to get a list of files that are missing licenses: java -jar /path/to/apache-rat-0.12.jar . I see a few java files, shell scripts, text files, and a lot of xml files without apache headers. [missing_licences.txt](https://github.com/apache/geode/files/712483/missing_licences.txt) --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode-examples issue #1: GEODE-2260: Revise geode-examples to be in their ow...
Github user upthewaterspout commented on the issue: https://github.com/apache/geode-examples/pull/1 +1 - Ship it! --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode issue #322: [GEODE-165] Fix for Add build support for generating antlr...
Github user upthewaterspout commented on the issue: https://github.com/apache/geode/pull/322 @agingade - Yeah, they'll get generated every time you do a clean followed a by a build. Presumably if you don't do a clean the antlr plugin will detect if the files are up to date. Prior to this fix I don't think there was even a README on how to generate these files. This fix is really the ideal solution - generated files should never be checked in to the source. This way everyone knows they need to modify oql.g, and everyone knows how to generate the resulting files because it happens automatically as part of the build. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode issue #296: GEODE-2109 : Calling submit on ExecutionService can cause ...
Github user upthewaterspout commented on the issue: https://github.com/apache/geode/pull/296 @deepakddixit - I think using LoggingThreadGroup is definitely a better fix. The changes look good to me now, I'll merge them. Thanks! --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode issue #322: [GEODE-165] Fix for Add build support for generating antlr...
Github user upthewaterspout commented on the issue: https://github.com/apache/geode/pull/322 +1 Awesome!!! --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode issue #296: GEODE-2109 : Calling submit on ExecutionService can cause ...
Github user upthewaterspout commented on the issue: https://github.com/apache/geode/pull/296 Hi @deepakddixit Sorry for the slow response. I'm a little concerned that with your changes to the test in ef4d38d, the test is no longer actually testing what happens if the runnable passed to the executor throws an exception. I think those changes *do* show that the problem is not with SystemErrRule, since with your change the test passes consistently. What that means is that there might be a product issue. I did a little bit of investigation, and I found that if I set an uncaught exception handler in the product, your original test passes consistently. I'm not quite sure why the default uncaught exception handler was not consistently working, but I think it's better to send the error to the log anyway. I stuck my changes up as a pull request to your branch if want to look at them. Let me know what you think, if this looks good to you I can merge this stuff: https://github.com/deepakddixit/incubator-geode/pull/1 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode pull request #318: Handle exceptions and don't deserialize PDX objects...
Github user upthewaterspout commented on a diff in the pull request:
https://github.com/apache/geode/pull/318#discussion_r92688937
--- Diff:
geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegion.java
---
@@ -8751,18 +8751,26 @@ public Index createIndex(boolean
remotelyOriginated, IndexType indexType, String
// Second step is iterating over REs and populating all the created
indexes
if (unpopulatedIndexes != null && unpopulatedIndexes.size() > 0) {
- throwException = populateEmptyIndexes(unpopulatedIndexes,
exceptionsMap);
+ throwException |= populateEmptyIndexes(unpopulatedIndexes,
exceptionsMap);
}
// Third step is to send the message to remote nodes
// Locally originated create index request.
// Send create request to other PR nodes.
-throwException =
+throwException |=
sendCreateIndexesMessage(remotelyOriginated, indexDefinitions,
indexes, exceptionsMap);
// If exception is throw in any of the above steps
if (throwException) {
- throw new MultiIndexCreationException(exceptionsMap);
+ try {
+for (String indexName : exceptionsMap.keySet()) {
+ Index index = indexManager.getIndex(indexName);
+ indexManager.removeIndex(index);
+ removeIndex(index, remotelyOriginated);
--- End diff --
That's right. In my test, once I fixed the fact that we were losing the
throwException flag, it didn't actually use the index, but the index was still
there if you actually called getIndex.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode pull request #318: Handle exceptions and don't deserialize PDX objects...
Github user upthewaterspout commented on a diff in the pull request:
https://github.com/apache/geode/pull/318#discussion_r92688373
--- Diff:
geode-core/src/test/java/org/apache/geode/internal/cache/PartitionedRegionQueryDUnitTest.java
---
@@ -149,6 +154,92 @@ public void run() {
});
}
+ @Test
+ public void testIndexDoesNotDeserializePdxObjects() {
+Host host = Host.getHost(0);
+VM vm0 = host.getVM(0);
+VM vm1 = host.getVM(1);
+
+SerializableRunnableIF createPR = () -> {
+ Cache cache = getCache();
+ PartitionAttributesFactory paf = new PartitionAttributesFactory();
+ paf.setTotalNumBuckets(10);
+
cache.createRegionFactory(RegionShortcut.PARTITION).setPartitionAttributes(paf.create())
+ .create("region");
+};
+vm0.invoke(createPR);
+vm1.invoke(createPR);
+
+// Do Puts. These objects can't be deserialized because they throw
+// and exception from the constructor
+vm0.invoke(() -> {
+ Cache cache = getCache();
+ Region region = cache.getRegion("region");
+ region.put(0, new PdxNotDeserializableAsset(0, "B"));
+ region.put(10, new PdxNotDeserializableAsset(1, "B"));
+ region.put(1, new PdxNotDeserializableAsset(1, "B"));
+ IntStream.range(11, 100)
+ .forEach(i -> region.put(i, new PdxNotDeserializableAsset(i,
Integer.toString(i;
+});
+
+// If this tries to deserialize the assets, it will fail
+vm0.invoke(() -> {
+ Cache cache = getCache();
+ cache.getQueryService().createHashIndex("ContractDocumentIndex",
"document", "/region");
+});
+
+vm0.invoke(() -> {
+ QueryService qs = getCache().getQueryService();
+ SelectResults results = (SelectResults) qs
+ .newQuery(" select assetId,document from /region where
document='B' limit 1000")
+ .execute();
+
+ assertEquals(3, results.size());
+ final Index index = qs.getIndex(getCache().getRegion("region"),
"ContractDocumentIndex");
+ assertEquals(1, index.getStatistics().getTotalUses());
+});
+ }
+
+ @Test
+ public void testFailureToCreateIndexOnRemoteNodeThrowsException() {
--- End diff --
Sure, I can add one.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode pull request #318: Handle exceptions and don't deserialize PDX objects...
GitHub user upthewaterspout opened a pull request: https://github.com/apache/geode/pull/318 Handle exceptions and don't deserialize PDX objects when creating indexes These are two related changes to our index creation code. We should not deserialize PDX objects during index creation. We should also fail the index creation and clean up the index if there is a failure. I'm a committer, I can merge these changes. Creating the PR for review. You can merge this pull request into a Git repository by running: $ git pull https://github.com/upthewaterspout/incubator-geode feature/GEODE-1272 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode/pull/318.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #318 commit 6782e0f100dacb5034e41f3854be27060f8af2e8 Author: Dan Smith <upthewatersp...@apache.org> Date: 2016-12-15T00:59:51Z GEODE-2216: Throwing an exception if index creation fails. Making sure index creation always throws an exception and cleans up the index if the index creation fails. Adding a test that causes index creation failure by failing to deserialize entries. commit 4a240dc27d77ae98dde10b4097e9eee6d515e1ce Author: Dan Smith <upthewatersp...@apache.org> Date: 2016-12-15T01:08:36Z GEODE-1272 Don't deserialize PDX objects when creating an index Setting the flag to prevent deserialization of PDX objects while populating an index that is defined on a partitioned region. We were setting this flag in the member that initially created the index, but not in other members that receive the IndexCreationMessage. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode issue #296: GEODE-2109 : Calling submit on ExecutionService can cause ...
Github user upthewaterspout commented on the issue: https://github.com/apache/geode/pull/296 @deepakddixit I was running ./gradlew test when it failed. What's weird to me is that we're not seeing the stack trace from the test for what actually failed the test, but I think there's definitely something doing wrong here. Maybe it is a bug in SystemErrRule. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode pull request #303: GEODE-1984: Fix Issue Make GatewaySender destroy a ...
Github user upthewaterspout commented on a diff in the pull request:
https://github.com/apache/geode/pull/303#discussion_r91393140
--- Diff:
geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/GatewaySenderDestroyFunction.java
---
@@ -0,0 +1,90 @@
+/*
+ * 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.cli.functions;
+
+import org.apache.geode.cache.Cache;
+import org.apache.geode.cache.CacheFactory;
+import org.apache.geode.cache.execute.FunctionAdapter;
+import org.apache.geode.cache.execute.FunctionContext;
+import org.apache.geode.cache.execute.ResultSender;
+import org.apache.geode.cache.wan.GatewaySender;
+import org.apache.geode.internal.InternalEntity;
+import org.apache.geode.internal.cache.wan.GatewaySenderException;
+import org.apache.geode.internal.logging.LogService;
+import org.apache.geode.management.internal.cli.CliUtil;
+import org.apache.geode.management.internal.cli.i18n.CliStrings;
+import org.apache.logging.log4j.Logger;
+
+public class GatewaySenderDestroyFunction extends FunctionAdapter
implements InternalEntity {
+ private static final long serialVersionUID = 1459761440357690134L;
--- End diff --
Minor nitpick - you can start with the serialVersionUID of 1 for new
classes, you don't have to generate one based on the class. It's fine if you
want to leave it as this though.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode pull request #303: GEODE-1984: Fix Issue Make GatewaySender destroy a ...
Github user upthewaterspout commented on a diff in the pull request: https://github.com/apache/geode/pull/303#discussion_r91392448 --- Diff: geode-core/src/main/java/org/apache/geode/cache/wan/GatewaySender.java --- @@ -400,4 +400,19 @@ public int getMaxParallelismForReplicatedRegion(); + + /** + * Destroys the GatewaySender. Before destroying the sender, caller needs to to ensure that the + * sender is stopped so that all the resources (threads, connection pool etc.) will be released + * properly. Stopping the sender is not handled in the destroy. Destroy is carried out in + * following steps: 1. Take the lifeCycleLock. 2. If the sender is attached to any application + * region, throw an exception. 3. Close the GatewaySenderAdvisor. 4. Remove the sender from the + * cache. 5. Destroy the region underlying the GatewaySender. + * + * In case of ParallelGatewaySender, the destroy operation does distributed destroy of the QPR. In + * case of SerialGatewaySender, the queue region is destroyed locally. --- End diff -- Is this still accurate? I see in the code you now call stop before calling destroy, so does the sender still need to be stopped? This looks like it's the internal documentation about what happens. Maybe right something more oriented towards the user for the public method? Also add an @since Geode 1.1 tag. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode issue #296: GEODE-2109 : Calling submit on ExecutionService can cause ...
Github user upthewaterspout commented on the issue: https://github.com/apache/geode/pull/296 +1 looks good to me. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-geode issue #299: [ GEODE-2141 ] #comment Fix Issue #2141
Github user upthewaterspout commented on the issue: https://github.com/apache/incubator-geode/pull/299 +1 - looks good to me, I think it's ready to merge. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-geode pull request #299: [ GEODE-2141 ] #comment Fix Issue #2141
Github user upthewaterspout commented on a diff in the pull request:
https://github.com/apache/incubator-geode/pull/299#discussion_r90077733
--- Diff:
geode-core/src/main/java/org/apache/geode/internal/statistics/StatMonitorHandler.java
---
@@ -50,36 +50,26 @@ public StatMonitorHandler() {
/** Adds a monitor which will be notified of samples */
public boolean addMonitor(StatisticsMonitor monitor) {
-synchronized (this) {
- boolean added = false;
- List oldMonitors = this.monitors;
- if (!oldMonitors.contains(monitor)) {
-List newMonitors = new
ArrayList(oldMonitors);
-added = newMonitors.add(monitor);
-this.monitors = Collections.unmodifiableList(newMonitors);
- }
- if (!this.monitors.isEmpty()) {
-startNotifier_IfEnabledAndNotRunning();
- }
- return added;
+boolean added = false;
--- End diff --
I don't think it's safe to remove the synchronization here. You're calling
contains followed by add followed by isEmpty. Also, startNotifier_... is
dependent on the state of this.monitors. this.monitors could be getting changed
by other threads at any point in here.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-geode pull request #293: GEODE-1653: Executing a fire-and-forget f...
Github user upthewaterspout commented on a diff in the pull request:
https://github.com/apache/incubator-geode/pull/293#discussion_r90073132
--- Diff:
geode-core/src/test/java/org/apache/geode/internal/cache/FireAndForgetFunctionOnAllServersDUnitTest.java
---
@@ -0,0 +1,164 @@
+/*
+ * 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 org.apache.geode.cache.Region;
+import org.apache.geode.cache.client.*;
+import org.apache.geode.cache.client.internal.LocatorTestBase;
+import org.apache.geode.cache.execute.Execution;
+import org.apache.geode.cache.execute.Function;
+import org.apache.geode.cache.execute.FunctionService;
+import org.apache.geode.internal.AvailablePort;
+import org.apache.geode.internal.cache.functions.TestFunction;
+import org.apache.geode.test.dunit.Assert;
+import org.apache.geode.test.dunit.Host;
+import org.apache.geode.test.dunit.NetworkUtils;
+import org.apache.geode.test.dunit.VM;
+import org.apache.geode.test.junit.categories.DistributedTest;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import static com.jayway.awaitility.Awaitility.*;
+import static java.util.concurrent.TimeUnit.*;
+
+/**
+ * Created by amey on 22/11/16.
+ */
+@Category(DistributedTest.class)
+public class FireAndForgetFunctionOnAllServersDUnitTest extends
LocatorTestBase {
+
+ public FireAndForgetFunctionOnAllServersDUnitTest() {
+super();
+ }
+
+ @Override
+ public final void postSetUp() throws Exception {
+disconnectAllFromDS();
+ }
+
+ @Override
+ protected final void postTearDownLocatorTestBase() throws Exception {
+disconnectAllFromDS();
+ }
+
+ @Test
+ public void testFireAndForgetFunctionOnAllServers() {
+
+// Test case for Executing a fire-and-forget function on all servers
as opposed to only
+// executing on the ones the
+// client is currently connected to.
+
+Host host = Host.getHost(0);
+VM locator = host.getVM(0);
+VM server1 = host.getVM(1);
+VM server2 = host.getVM(2);
+VM client = host.getVM(3);
+
+final int locatorPort =
AvailablePort.getRandomAvailablePort(AvailablePort.SOCKET);
+final String locatorHost = NetworkUtils.getServerHostName(host);
+
+// Step 1. Start a locator and one cache server.
+locator.invoke("Start Locator", () -> startLocator(locatorHost,
locatorPort, ""));
+
+String locString = getLocatorString(host, locatorPort);
+
+// Step 2. Start a server and create a replicated region "R1".
+server1.invoke("Start BridgeServer",
+() -> startBridgeServer(new String[] {"R1"}, locString, new
String[] {"R1"}));
+
+// Step 3. Create a client cache with pool mentioning locator.
+client.invoke("create client cache and pool mentioning locator", () ->
{
+ ClientCacheFactory ccf = new ClientCacheFactory();
+ ccf.addPoolLocator(locatorHost, locatorPort);
+ ClientCache cache = ccf.create();
+ Pool pool1 = PoolManager.createFactory().addLocator(locatorHost,
locatorPort)
+ .setServerGroup("R1").create("R1");
+
+ Region region1 =
cache.createClientRegionFactory(ClientRegionShortcut.PROXY).setPoolName("R1")
+ .create("R1");
+
+ // Step 4. Execute the function to put DistributedMemberID into
above created replicated
+ // region.
+ Function function =
+ new TestFunction(false,
TestFunction.TEST_FUNCTION_FIREANDFORGET_ONALL_SERVERS);
+ FunctionService.registerFunction(function);
+
+ String regionName = "R1";
+ Execution dataSet = FunctionService.onServers(pool1);
+ dataSet.withArgs(regionName).execute(function);
+
+
