[GitHub] incubator-geode pull request #299: [ GEODE-2141 ] #comment Fix Issue #2141
GitHub user davinash opened a pull request: https://github.com/apache/incubator-geode/pull/299 [ GEODE-2141 ] #comment Fix Issue #2141 Replaced List with ConcurrentHashSet You can merge this pull request into a Git repository by running: $ git pull https://github.com/davinash/incubator-geode feature/GEODE-2141 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/incubator-geode/pull/299.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 #299 commit e4da4c43dc691e59d34640e95c5d63a5a1b0ea1c Author: adongre Date: 2016-11-27T14:11:48Z GEODE-2141: Replace use of List with ConcurrentHashSet for storing various stats and listener. Moved methods stopNotifier_IfEnabledAndRunning and startNotifier_IfEnabledAndNotRunning in synchronized block. --- 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 #283: GEODE-2098: Moved gfsh history file locat...
Github user davinash closed the pull request at: https://github.com/apache/incubator-geode/pull/283 --- 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 #283: GEODE-2098: Moved gfsh history file location fro...
Github user davinash commented on the issue: https://github.com/apache/incubator-geode/pull/283 closing the pull 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] incubator-geode pull request #300: [GEODE-224] Geode Spark connector parser ...
GitHub user vectorijk opened a pull request: https://github.com/apache/incubator-geode/pull/300 [GEODE-224] Geode Spark connector parser is not processing type casting properly This issue is related to wrong regex of query string. You can merge this pull request into a Git repository by running: $ git pull https://github.com/vectorijk/incubator-geode GEODE-224 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/incubator-geode/pull/300.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 #300 commit 86e1235716267094740c06a7fe3f13e85f303cbc Author: Kai Jiang Date: 2016-11-29T11:21:00Z GEODE-224 Spark Connector parser is not processing type casting properly This issue is related to wrong regex of query string. --- 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 #300: [GEODE-224] Geode Spark connector parser is not ...
Github user vectorijk commented on the issue: https://github.com/apache/incubator-geode/pull/300 cc @markito --- 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 metatype commented on a diff in the pull request: https://github.com/apache/incubator-geode/pull/299#discussion_r90025760 --- Diff: geode-core/src/main/java/org/apache/geode/internal/statistics/StatMonitorHandler.java --- @@ -110,7 +100,7 @@ public void sampled(long nanosTimeStamp, List resourceInstance } private void monitor(final long sampleTimeMillis, final List resourceInstance) { -List currentMonitors = StatMonitorHandler.this.monitors; +ConcurrentHashSet currentMonitors = StatMonitorHandler.this.monitors; --- End diff -- I don't think this copy is needed since `monitors` is never updated after initialization. --- 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 metatype commented on a diff in the pull request: https://github.com/apache/incubator-geode/pull/299#discussion_r90025338 --- Diff: geode-core/src/main/java/org/apache/geode/internal/statistics/StatisticsMonitor.java --- @@ -29,23 +29,20 @@ private final Object mutex = new Object(); - private volatile List listeners = Collections.emptyList(); + private volatile ConcurrentHashSet listeners = --- End diff -- Instead of `volatile` I think this should be final. I don't see a write to this var after initialization. --- 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 metatype commented on a diff in the pull request: https://github.com/apache/incubator-geode/pull/299#discussion_r90026482 --- Diff: geode-core/src/main/java/org/apache/geode/internal/statistics/StatisticsMonitor.java --- @@ -114,14 +100,14 @@ protected void monitor(long millisTimeStamp, List resourceInst private final void monitorStatisticIds(long millisTimeStamp, List resourceInstances) { -List statisticIdsToMonitor = statisticIds; +ConcurrentHashSet statisticIdsToMonitor = statisticIds; --- End diff -- I don't think this copy is needed since `statisticIdsToMonitor` is never updated after initialization. --- 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 metatype commented on a diff in the pull request: https://github.com/apache/incubator-geode/pull/299#discussion_r90025277 --- Diff: geode-core/src/main/java/org/apache/geode/internal/statistics/StatMonitorHandler.java --- @@ -38,7 +37,8 @@ private final boolean enableMonitorThread; /** The registered monitors */ - private volatile List monitors = Collections.emptyList(); + private volatile ConcurrentHashSet monitors = --- End diff -- Instead of `volatile` I think this should be final. I don't see a write to this var after initialization. --- 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 metatype commented on a diff in the pull request: https://github.com/apache/incubator-geode/pull/299#discussion_r90025623 --- Diff: geode-core/src/main/java/org/apache/geode/internal/statistics/StatisticsMonitor.java --- @@ -114,14 +100,14 @@ protected void monitor(long millisTimeStamp, List resourceInst private final void monitorStatisticIds(long millisTimeStamp, List resourceInstances) { -List statisticIdsToMonitor = statisticIds; +ConcurrentHashSet statisticIdsToMonitor = statisticIds; if (!statisticIdsToMonitor.isEmpty()) { // TODO: } } protected final void notifyListeners(StatisticsNotification notification) { -List listenersToNotify = this.listeners; +ConcurrentHashSet listenersToNotify = this.listeners; --- End diff -- I don't think this copy is needed since `listeners` is never updated after initialization. --- 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 metatype commented on a diff in the pull request: https://github.com/apache/incubator-geode/pull/299#discussion_r90025856 --- Diff: geode-core/src/main/java/org/apache/geode/internal/statistics/StatMonitorHandler.java --- @@ -228,7 +222,7 @@ private void work() { } } if (working && latestTask != null) { -List currentMonitors = StatMonitorHandler.this.monitors; +ConcurrentHashSet currentMonitors = StatMonitorHandler.this.monitors; --- End diff -- I don't think this copy is needed since `monitors` is never updated after initialization. --- 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 metatype commented on a diff in the pull request: https://github.com/apache/incubator-geode/pull/299#discussion_r90025325 --- Diff: geode-core/src/main/java/org/apache/geode/internal/statistics/StatisticsMonitor.java --- @@ -29,23 +29,20 @@ private final Object mutex = new Object(); - private volatile List listeners = Collections.emptyList(); + private volatile ConcurrentHashSet listeners = + new ConcurrentHashSet(); - private volatile List statisticIds = Collections.emptyList(); + private volatile ConcurrentHashSet statisticIds = --- End diff -- Instead of `volatile` I think this should be final. I don't see a write to this var after initialization. --- 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 #290: GEODE-2092: Security examples should not be in t...
Github user metatype commented on the issue: https://github.com/apache/incubator-geode/pull/290 @kjduling Is this ready to be 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] incubator-geode issue #290: GEODE-2092: Security examples should not be in t...
Github user kjduling commented on the issue: https://github.com/apache/incubator-geode/pull/290 @metatype Yes, precheckin ran 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] incubator-geode issue #290: GEODE-2092: Security examples should not be in t...
Github user jinmeiliao commented on the issue: https://github.com/apache/incubator-geode/pull/290 I'll pull this in. --- 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 #290: GEODE-2092: Security examples should not ...
Github user asfgit closed the pull request at: https://github.com/apache/incubator-geode/pull/290 --- 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 #292: [GEODE-1509] Fixed : Reduce the memory usage of ...
Github user upthewaterspout commented on the issue: https://github.com/apache/incubator-geode/pull/292 +1 I think this should save 16 bytes per entry. The changes look good to me! I'll merge them. --- 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 #292: [GEODE-1509] Fixed : Reduce the memory us...
Github user asfgit closed the pull request at: https://github.com/apache/incubator-geode/pull/292 --- 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 #300: [GEODE-224] Geode Spark connector parser ...
Github user asfgit closed the pull request at: https://github.com/apache/incubator-geode/pull/300 --- 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 #300: [GEODE-224] Geode Spark connector parser is not ...
Github user jhuynh1 commented on the issue: https://github.com/apache/incubator-geode/pull/300 The changes look good, I've merged them in. Thanks for the contribution! --- 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); + + // Using Awatility, if the condition is not met during the timeout, a + // ConditionTimeoutException will be thrown. This makes analyzing the
[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_r90070586 --- Diff: geode-core/src/main/java/org/apache/geode/cache/client/internal/ExecuteFunctionNoAckOp.java --- @@ -236,4 +236,16 @@ protected Message createResponseMessage() { } } + private static List getAllServers(PoolImpl pool) { +List servers; +if (pool.getLocators() == null || pool.getLocators().isEmpty()) { + servers = ((ExplicitConnectionSourceImpl) pool.getConnectionSource()).getAllServers(); --- End diff -- It seems like getAllServers ought to be a method on ConnectionSource, so this code doesn't have to check for the different types of connection sources and cast. --- 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_r90072798 --- 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); + + // Using Awatility, if the condition is not met during the timeout, a + // ConditionTimeoutException will be thrown. This makes analyzing the
[GitHub] incubator-geode issue #300: [GEODE-224] Geode Spark connector parser is not ...
Github user markito commented on the issue: https://github.com/apache/incubator-geode/pull/300 @vectorijk good catch! 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] 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_r90077848 --- 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; +if (!this.monitors.contains(monitor)) { + added = this.monitors.add(monitor); +} +if (!this.monitors.isEmpty()) { + startNotifier_IfEnabledAndNotRunning(); } +return added; } /** Removes a monitor that will no longer be used */ public boolean removeMonitor(StatisticsMonitor monitor) { -synchronized (this) { - boolean removed = false; - List oldMonitors = this.monitors; - if (oldMonitors.contains(monitor)) { -List newMonitors = new ArrayList(oldMonitors); -removed = newMonitors.remove(monitor); -this.monitors = Collections.unmodifiableList(newMonitors); - } - if (this.monitors.isEmpty()) { -stopNotifier_IfEnabledAndRunning(); - } - return removed; +boolean removed = false; --- End diff -- Same issue here - it's not safe to remove the synchronization. --- 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_r90078053 --- Diff: geode-core/src/main/java/org/apache/geode/internal/statistics/StatMonitorHandler.java --- @@ -138,8 +127,8 @@ public void allocatedResourceInstance(ResourceInstance resourceInstance) {} public void destroyedResourceInstance(ResourceInstance resourceInstance) {} /** For testing only */ - List getMonitorsSnapshot() { -return Collections.unmodifiableList(this.monitors); + ConcurrentHashSet getMonitorsSnapshot() { --- End diff -- Maybe this ought to just return a Collection? --- 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 issue #300: [GEODE-224] Geode Spark connector parser is not ...
Github user vectorijk commented on the issue: https://github.com/apache/incubator-geode/pull/300 @jhuynh1 @markito Thanks for the 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. ---
Re: Review Request 54003: GEODE-2137 client membership IDs contain 17 bytes of useless information that should be removed
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54003/ --- (Updated Nov. 29, 2016, 9:12 p.m.) Review request for geode, Galen O'Sullivan, Hitesh Khamesra, and Udo Kohlmeyer. Changes --- This updated diff removes some fields in InternalDistributedMember that were duplicated in GMSMember, reducing the size of membership IDs. This also rolls the product version forward so that we can retain backward compatibility with the 1.0 release of Geode. Bugs: geode-2137 https://issues.apache.org/jira/browse/geode-2137 Repository: geode Description --- This removes the UUID and member-weight bytes from the serialized form of event IDs, version tags and client IDs while retaining them for server IDs in general. Diffs (updated) - geode-core/src/main/java/org/apache/geode/distributed/internal/membership/InternalDistributedMember.java 0d4fd9e9a8d22266635a26c77d8dcb61a343a628 geode-core/src/main/java/org/apache/geode/distributed/internal/membership/MemberAttributes.java 75cdd498b5879e9ec91418447819b65737713449 geode-core/src/main/java/org/apache/geode/distributed/internal/membership/NetMember.java 4db207fb118a58c778b26dc6b104c8b9568d7112 geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSMember.java 670d62b54f12182420c604086813e3be34fa geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSMemberFactory.java c84690a441abec4eaab2d9808d1925b74728bfdd geode-core/src/main/java/org/apache/geode/internal/Version.java fabb3a74391e3c9aae415eb6f434ad942ee014fc geode-core/src/main/java/org/apache/geode/internal/cache/persistence/DiskStoreID.java e94b081d7559e6bea3dde2b9cbf6ebe568842193 geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/CommandInitializer.java 0fe44cf589f290d9c2504868264982264d038ac0 geode-core/src/main/java/org/apache/geode/internal/cache/versions/VersionSource.java bfbda2f9b030bef40940d165c2711f913c5caf2b geode-core/src/main/java/org/apache/geode/internal/cache/versions/VersionTag.java fa2c15669e97cddbaedd4ac326d47a01dd288434 geode-core/src/test/java/org/apache/geode/distributed/LocatorDUnitTest.java 128f970ff3d93dbd0010dce88e9fd123239b0a80 geode-core/src/test/java/org/apache/geode/distributed/LocatorJUnitTest.java d1e56c46b78a28d26278c074fa52c49a4992e954 geode-core/src/test/java/org/apache/geode/distributed/RoleDUnitTest.java d8047038c8977dbaa7a99bed9fe8cce9a1f241b4 geode-core/src/test/java/org/apache/geode/distributed/internal/DistributionManagerDUnitTest.java b38a769b6804d2da8f230d33ae15b0a36c53b422 geode-core/src/test/java/org/apache/geode/distributed/internal/InternalDistributedSystemJUnitTest.java 49e8fa6be557b0f7aa0e7ca91733d2a8fd5ce00d geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/fd/GMSHealthMonitorJUnitTest.java ebf87be77e84fe3f97fdf5f51305942986f00152 geode-core/src/test/java/org/apache/geode/test/dunit/standalone/ProcessManager.java c3c33d04014b173d45bf4bbf7c74a10fc0cc1335 Diff: https://reviews.apache.org/r/54003/diff/ Testing --- precheckin, membership and client/server integration testing Thanks, Bruce Schuchardt
[GitHub] incubator-geode pull request #293: GEODE-1653: Executing a fire-and-forget f...
Github user jhuynh1 commented on a diff in the pull request: https://github.com/apache/incubator-geode/pull/293#discussion_r90116715 --- 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. --- End diff -- Minor nitpick, would you be able to remove the author tags? --- 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 #296: GEODE-2109 : Calling submit on ExecutionS...
Github user upthewaterspout commented on a diff in the pull request: https://github.com/apache/incubator-geode/pull/296#discussion_r90118721 --- Diff: geode-core/src/main/java/org/apache/geode/management/internal/FederatingManager.java --- @@ -386,7 +398,7 @@ public void startManagingActivity() throws Exception { * */ - private class GIITask implements Callable { + private class GIITask implements Callable { --- End diff -- Why did you remove from this class declaration? --- 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 #294: Properly close WebDriver for UITests
Github user jaredjstewart commented on a diff in the pull request: https://github.com/apache/incubator-geode/pull/294#discussion_r90124555 --- Diff: geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/rules/ScreenshotOnFailureRule.java --- @@ -45,7 +45,9 @@ private void takeScreenshot(String screenshotName) { if (driver instanceof TakesScreenshot) { File tempFile = ((TakesScreenshot) driver).getScreenshotAs(OutputType.FILE); try { -FileUtils.copyFile(tempFile, new File("build/screenshots/" + screenshotName + ".png")); +File screenshot = new File("build/screenshots/" + screenshotName + ".png"); +FileUtils.copyFile(tempFile, screenshot); +System.err.println("Screenshot saved to: " + screenshot.getCanonicalPath()); --- End diff -- This is intentional, it adds some output to help find where the screenshots end up when one gets taken. --- 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. ---
Re: Review Request 54003: GEODE-2137 client membership IDs contain 17 bytes of useless information that should be removed
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54003/#review157338 --- Ship it! Ship It! - Udo Kohlmeyer On Nov. 29, 2016, 9:12 p.m., Bruce Schuchardt wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/54003/ > --- > > (Updated Nov. 29, 2016, 9:12 p.m.) > > > Review request for geode, Galen O'Sullivan, Hitesh Khamesra, and Udo > Kohlmeyer. > > > Bugs: geode-2137 > https://issues.apache.org/jira/browse/geode-2137 > > > Repository: geode > > > Description > --- > > This removes the UUID and member-weight bytes from the serialized form of > event IDs, version tags and client IDs while retaining them for server IDs in > general. > > > Diffs > - > > > geode-core/src/main/java/org/apache/geode/distributed/internal/membership/InternalDistributedMember.java > 0d4fd9e9a8d22266635a26c77d8dcb61a343a628 > > geode-core/src/main/java/org/apache/geode/distributed/internal/membership/MemberAttributes.java > 75cdd498b5879e9ec91418447819b65737713449 > > geode-core/src/main/java/org/apache/geode/distributed/internal/membership/NetMember.java > 4db207fb118a58c778b26dc6b104c8b9568d7112 > > geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSMember.java > 670d62b54f12182420c604086813e3be34fa > > geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSMemberFactory.java > c84690a441abec4eaab2d9808d1925b74728bfdd > geode-core/src/main/java/org/apache/geode/internal/Version.java > fabb3a74391e3c9aae415eb6f434ad942ee014fc > > geode-core/src/main/java/org/apache/geode/internal/cache/persistence/DiskStoreID.java > e94b081d7559e6bea3dde2b9cbf6ebe568842193 > > geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/CommandInitializer.java > 0fe44cf589f290d9c2504868264982264d038ac0 > > geode-core/src/main/java/org/apache/geode/internal/cache/versions/VersionSource.java > bfbda2f9b030bef40940d165c2711f913c5caf2b > > geode-core/src/main/java/org/apache/geode/internal/cache/versions/VersionTag.java > fa2c15669e97cddbaedd4ac326d47a01dd288434 > geode-core/src/test/java/org/apache/geode/distributed/LocatorDUnitTest.java > 128f970ff3d93dbd0010dce88e9fd123239b0a80 > geode-core/src/test/java/org/apache/geode/distributed/LocatorJUnitTest.java > d1e56c46b78a28d26278c074fa52c49a4992e954 > geode-core/src/test/java/org/apache/geode/distributed/RoleDUnitTest.java > d8047038c8977dbaa7a99bed9fe8cce9a1f241b4 > > geode-core/src/test/java/org/apache/geode/distributed/internal/DistributionManagerDUnitTest.java > b38a769b6804d2da8f230d33ae15b0a36c53b422 > > geode-core/src/test/java/org/apache/geode/distributed/internal/InternalDistributedSystemJUnitTest.java > 49e8fa6be557b0f7aa0e7ca91733d2a8fd5ce00d > > geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/fd/GMSHealthMonitorJUnitTest.java > ebf87be77e84fe3f97fdf5f51305942986f00152 > > geode-core/src/test/java/org/apache/geode/test/dunit/standalone/ProcessManager.java > c3c33d04014b173d45bf4bbf7c74a10fc0cc1335 > > Diff: https://reviews.apache.org/r/54003/diff/ > > > Testing > --- > > precheckin, membership and client/server integration testing > > > Thanks, > > Bruce Schuchardt > >
Review Request 54184: Adding a test to show secondary queue is being drained for GEODE-2123
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54184/ --- Review request for geode, Barry Oglesby and Dan Smith. Repository: geode Description --- Also fixed a possibly flaky test by removing the sleeps and having it check the actual size of the queue (including secondary buckets) Removed some unnecessary sleep calls Diffs - geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegionDataStore.java 2e9fdbc geode-core/src/main/java/org/apache/geode/internal/cache/wan/parallel/ConcurrentParallelGatewaySenderQueue.java 603fd6c geode-core/src/main/java/org/apache/geode/internal/cache/wan/parallel/ParallelGatewaySenderQueue.java 553847d geode-wan/src/test/java/org/apache/geode/internal/cache/wan/WANTestBase.java f9c18ec geode-wan/src/test/java/org/apache/geode/internal/cache/wan/concurrent/ConcurrentParallelGatewaySenderOperation_2_DUnitTest.java 931a49c geode-wan/src/test/java/org/apache/geode/internal/cache/wan/parallel/ParallelWANPropagationConcurrentOpsDUnitTest.java 0ce6782 Diff: https://reviews.apache.org/r/54184/diff/ Testing --- Thanks, Jason Huynh
Re: Review Request 54184: Adding a test to show secondary queue is being drained for GEODE-2123
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54184/#review157355 --- Ship it! Ship It! - Dan Smith On Nov. 29, 2016, 10:43 p.m., Jason Huynh wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/54184/ > --- > > (Updated Nov. 29, 2016, 10:43 p.m.) > > > Review request for geode, Barry Oglesby and Dan Smith. > > > Repository: geode > > > Description > --- > > Also fixed a possibly flaky test by removing the sleeps and having it check > the actual size of the queue (including secondary buckets) > Removed some unnecessary sleep calls > > > Diffs > - > > > geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegionDataStore.java > 2e9fdbc > > geode-core/src/main/java/org/apache/geode/internal/cache/wan/parallel/ConcurrentParallelGatewaySenderQueue.java > 603fd6c > > geode-core/src/main/java/org/apache/geode/internal/cache/wan/parallel/ParallelGatewaySenderQueue.java > 553847d > > geode-wan/src/test/java/org/apache/geode/internal/cache/wan/WANTestBase.java > f9c18ec > > geode-wan/src/test/java/org/apache/geode/internal/cache/wan/concurrent/ConcurrentParallelGatewaySenderOperation_2_DUnitTest.java > 931a49c > > geode-wan/src/test/java/org/apache/geode/internal/cache/wan/parallel/ParallelWANPropagationConcurrentOpsDUnitTest.java > 0ce6782 > > Diff: https://reviews.apache.org/r/54184/diff/ > > > Testing > --- > > > Thanks, > > Jason Huynh > >
[GitHub] incubator-geode pull request #293: GEODE-1653: Executing a fire-and-forget f...
Github user gesterzhou commented on a diff in the pull request: https://github.com/apache/incubator-geode/pull/293#discussion_r90173264 --- 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); + + // Using Awatility, if the condition is not met during the timeout, a + // ConditionTimeoutException will be thrown. This makes analyzing the failu
[GitHub] incubator-geode pull request #293: GEODE-1653: Executing a fire-and-forget f...
Github user gesterzhou commented on a diff in the pull request: https://github.com/apache/incubator-geode/pull/293#discussion_r90173276 --- Diff: geode-core/src/main/java/org/apache/geode/cache/client/internal/ExecuteFunctionNoAckOp.java --- @@ -236,4 +236,16 @@ protected Message createResponseMessage() { } } + private static List getAllServers(PoolImpl pool) { +List servers; +if (pool.getLocators() == null || pool.getLocators().isEmpty()) { + servers = ((ExplicitConnectionSourceImpl) pool.getConnectionSource()).getAllServers(); --- End diff -- This can be done by introducing a getAllServers() into DataSource interface and modify the AutoConnectionSourceImpl.findAllServers() into getAllServers(). Also there's no need to create a static method in ExecuteFunctionNoAckOp.java. Also need to modify ExecuteFunctionOp.java to use ArrayList servers = pool.getConnectionSource().getAllServers(); --- 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. ---