[ 
https://issues.apache.org/jira/browse/GEODE-1653?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15706119#comment-15706119
 ] 

ASF GitHub Bot commented on GEODE-1653:
---------------------------------------

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 failure much simpler
    +      await().atMost(60, SECONDS).until(() -> {
    +        return (region1.keySetOnServer().size() == 1);
    --- End diff --
    
    I think it's slightly better to use an assertion with Awaitility, like this:
    await().atMost(60, SECONDS).until(() -> assertEquals(1, 
region1.keySetOnServer().size())
    
    This way, if the test fails you will see what the actual value of 
region1.keySetOnServer().size() is, rather than just a failure.


> Executing a fire-and-forget function on all servers doesn't actually execute 
> on all servers
> -------------------------------------------------------------------------------------------
>
>                 Key: GEODE-1653
>                 URL: https://issues.apache.org/jira/browse/GEODE-1653
>             Project: Geode
>          Issue Type: Bug
>          Components: functions
>            Reporter: Barry Oglesby
>            Assignee: Amey Barve
>
> Executing a fire-and-forget function on all servers only executes on the ones 
> the client is currently connected to.
> The two {{ExecuteFunctionNoAckOp execute}} methods get the servers to execute 
> against using code like:
> {noformat}
> pool.getCurrentServers();
> {noformat}
> This method just gets all the {{EndpointManager's endpointMap's 
> ServerLocations}}.
> What should be done is more like what the {{ExecuteFunctionOp execute}} 
> methods do:
> {noformat}
> private static List<ServerLocation> getAllServers(PoolImpl pool) {
>   List<ServerLocation> servers = null;
>   if (pool.getLocators() == null || pool.getLocators().isEmpty()) {
>     servers = ((ExplicitConnectionSourceImpl) 
> pool.getConnectionSource()).getAllServers();
>   } else {
>     servers = ((AutoConnectionSourceImpl) 
> pool.getConnectionSource()).findAllServers(); // n/w call on locator
>   }
>   return servers;
> }
> {noformat}



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to