[GitHub] geode pull request #673: GEODE-3284: New flow: getAvailableServers
Github user asfgit closed the pull request at: https://github.com/apache/geode/pull/673 --- 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 #673: GEODE-3284: New flow: getAvailableServers
Github user galen-pivotal commented on a diff in the pull request: https://github.com/apache/geode/pull/673#discussion_r130782011 --- Diff: geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/operations/GetAvailableServersOperationHandler.java --- @@ -0,0 +1,110 @@ +/* + * 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.protocol.protobuf.operations; + +import org.apache.commons.lang.StringUtils; +import org.apache.geode.cache.Cache; +import org.apache.geode.cache.client.internal.locator.GetAllServersRequest; +import org.apache.geode.cache.client.internal.locator.GetAllServersResponse; +import org.apache.geode.distributed.ConfigurationProperties; +import org.apache.geode.distributed.internal.InternalDistributedSystem; +import org.apache.geode.distributed.internal.ServerLocation; +import org.apache.geode.distributed.internal.tcpserver.TcpClient; +import org.apache.geode.internal.admin.remote.DistributionLocatorId; +import org.apache.geode.protocol.operations.OperationHandler; +import org.apache.geode.protocol.protobuf.BasicTypes; +import org.apache.geode.protocol.protobuf.Failure; +import org.apache.geode.protocol.protobuf.Result; +import org.apache.geode.protocol.protobuf.ServerAPI; +import org.apache.geode.protocol.protobuf.Success; +import org.apache.geode.serialization.SerializationService; + +import java.io.IOException; +import java.net.InetSocketAddress; +import java.util.Collection; +import java.util.HashSet; +import java.util.Optional; +import java.util.Properties; +import java.util.StringTokenizer; +import java.util.stream.Collectors; + +public class GetAvailableServersOperationHandler implements --- End diff -- @Experimental or a doc comment explaining why this won't be around forever would be 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 pull request #673: GEODE-3284: New flow: getAvailableServers
Github user galen-pivotal commented on a diff in the pull request: https://github.com/apache/geode/pull/673#discussion_r130781962 --- Diff: geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/operations/GetAvailableServersOperationHandler.java --- @@ -0,0 +1,110 @@ +/* + * 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.protocol.protobuf.operations; + +import org.apache.commons.lang.StringUtils; +import org.apache.geode.cache.Cache; +import org.apache.geode.cache.client.internal.locator.GetAllServersRequest; +import org.apache.geode.cache.client.internal.locator.GetAllServersResponse; +import org.apache.geode.distributed.ConfigurationProperties; +import org.apache.geode.distributed.internal.InternalDistributedSystem; +import org.apache.geode.distributed.internal.ServerLocation; +import org.apache.geode.distributed.internal.tcpserver.TcpClient; +import org.apache.geode.internal.admin.remote.DistributionLocatorId; +import org.apache.geode.protocol.operations.OperationHandler; +import org.apache.geode.protocol.protobuf.BasicTypes; +import org.apache.geode.protocol.protobuf.Failure; +import org.apache.geode.protocol.protobuf.Result; +import org.apache.geode.protocol.protobuf.ServerAPI; +import org.apache.geode.protocol.protobuf.Success; +import org.apache.geode.serialization.SerializationService; + +import java.io.IOException; +import java.net.InetSocketAddress; +import java.util.Collection; +import java.util.HashSet; +import java.util.Optional; +import java.util.Properties; +import java.util.StringTokenizer; +import java.util.stream.Collectors; + +public class GetAvailableServersOperationHandler implements +OperationHandler { + + @Override + public Result process( + SerializationService serializationService, ServerAPI.GetAvailableServersRequest request, + Cache cache) { +HashSet locators = getLocatorIdsFromDistributedProperties(cache); + +TcpClient tcpClient = getTcpClient(); +Optional> result = locators.stream() +.map((locatorId -> getGetAvailableServersFromLocator(tcpClient, locatorId))) +.filter((obj) -> obj != null).findFirst(); + +if (result.isPresent()) { + return result.get(); +} else { + return Failure + .of(BasicTypes.ErrorResponse.newBuilder().setMessage("Unable to find a locator").build()); +} + } + + private HashSet getLocatorIdsFromDistributedProperties(Cache cache) { --- End diff -- Will this work if locators are configured via cluster config? I'm guessing it won't work if the original locators are no longer in the cluster. Since this is a prototype and will be redone better, I think it's fine for 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 pull request #673: GEODE-3284: New flow: getAvailableServers
Github user pivotal-amurmann commented on a diff in the pull request: https://github.com/apache/geode/pull/673#discussion_r130647800 --- Diff: geode-protobuf/src/test/java/org/apache/geode/protocol/protobuf/operations/GetAvailableServersOperationHandlerJUnitTest.java --- @@ -0,0 +1,131 @@ +/* + * 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.protocol.protobuf.operations; + +import org.apache.geode.cache.client.internal.locator.GetAllServersResponse; +import org.apache.geode.distributed.ConfigurationProperties; +import org.apache.geode.distributed.internal.InternalDistributedSystem; +import org.apache.geode.distributed.internal.ServerLocation; +import org.apache.geode.distributed.internal.tcpserver.TcpClient; +import org.apache.geode.internal.cache.GemFireCacheImpl; +import org.apache.geode.protocol.protobuf.BasicTypes; +import org.apache.geode.protocol.protobuf.Failure; +import org.apache.geode.protocol.protobuf.Result; +import org.apache.geode.protocol.protobuf.ServerAPI; +import org.apache.geode.protocol.protobuf.ServerAPI.GetAvailableServersResponse; +import org.apache.geode.protocol.protobuf.Success; +import org.apache.geode.protocol.protobuf.utilities.ProtobufRequestUtilities; +import org.apache.geode.test.junit.categories.UnitTest; +import org.junit.Before; +import org.junit.Test; +import org.junit.experimental.categories.Category; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.Properties; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyBoolean; +import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +@Category(UnitTest.class) +public class GetAvailableServersOperationHandlerJUnitTest extends OperationHandlerJUnitTest { + + private TcpClient mockTCPClient; + + @Before + public void setUp() throws Exception { +super.setUp(); + +operationHandler = mock(GetAvailableServersOperationHandler.class); +cacheStub = mock(GemFireCacheImpl.class); +when(operationHandler.process(any(), any(), any())).thenCallRealMethod(); +InternalDistributedSystem mockDistributedSystem = mock(InternalDistributedSystem.class); --- End diff -- Can we have some blank lines as separators between segments that pertain to a single mock? Right now this is a fairly impenetrable wall of text. --- 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 #673: GEODE-3284: New flow: getAvailableServers
Github user pivotal-amurmann commented on a diff in the pull request: https://github.com/apache/geode/pull/673#discussion_r130646935 --- Diff: geode-protobuf/src/test/java/org/apache/geode/protocol/GetAvailableServersDUnitTest.java --- @@ -0,0 +1,108 @@ +/* + * 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.protocol; + +import org.apache.geode.cache.server.CacheServer; +import org.apache.geode.internal.cache.InternalCache; +import org.apache.geode.protocol.exception.InvalidProtocolMessageException; +import org.apache.geode.protocol.protobuf.ClientProtocol; +import org.apache.geode.protocol.protobuf.ServerAPI; +import org.apache.geode.protocol.protobuf.serializer.ProtobufProtocolSerializer; +import org.apache.geode.protocol.protobuf.utilities.ProtobufRequestUtilities; +import org.apache.geode.protocol.protobuf.utilities.ProtobufUtilities; +import org.apache.geode.test.dunit.DistributedTestUtils; +import org.apache.geode.test.dunit.Host; +import org.apache.geode.test.dunit.VM; +import org.apache.geode.test.dunit.cache.internal.JUnit4CacheTestCase; +import org.apache.geode.test.dunit.rules.DistributedRestoreSystemProperties; +import org.apache.geode.test.junit.categories.DistributedTest; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.experimental.categories.Category; + +import java.io.IOException; +import java.net.Socket; + +import static org.junit.Assert.assertEquals; + +@Category(DistributedTest.class) +public class GetAvailableServersDUnitTest extends JUnit4CacheTestCase { + + @Rule + public DistributedRestoreSystemProperties distributedRestoreSystemProperties = + new DistributedRestoreSystemProperties(); + + @Before + public void setup() { + + } + + @Test + public void testGetAllAvailableServersRequest() + throws IOException, InvalidProtocolMessageException { +Host host = Host.getHost(0); +VM vm0 = host.getVM(0); +VM vm1 = host.getVM(1); +VM vm2 = host.getVM(2); + +int locatorPort = DistributedTestUtils.getDUnitLocatorPort(); + +// int cacheServer1Port = vm0.invoke("Start Cache1", () -> startCacheWithCacheServer()); +int cacheServer1Port = startCacheWithCacheServer(); +int cacheServer2Port = vm1.invoke("Start Cache2", () -> startCacheWithCacheServer()); +int cacheServer3Port = vm2.invoke("Start Cache3", () -> startCacheWithCacheServer()); + +vm0.invoke(() -> { + Socket socket = new Socket(host.getHostName(), cacheServer1Port); + socket.getOutputStream().write(110); + + ClientProtocol.Request.Builder protobufRequestBuilder = + ProtobufUtilities.createProtobufRequestBuilder(); + ClientProtocol.Message getAvailableServersRequestMessage = + ProtobufUtilities.createProtobufMessage(ProtobufUtilities.createMessageHeader(1233445), --- End diff -- nit pick: Could we move the first param to the line below, so that is lines up with the second one? That would make this much more readable. Not sure if spotless hates that 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 #673: GEODE-3284: New flow: getAvailableServers
Github user pivotal-amurmann commented on a diff in the pull request: https://github.com/apache/geode/pull/673#discussion_r130646564 --- Diff: geode-protobuf/src/test/java/org/apache/geode/protocol/GetAvailableServersDUnitTest.java --- @@ -0,0 +1,108 @@ +/* + * 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.protocol; + +import org.apache.geode.cache.server.CacheServer; +import org.apache.geode.internal.cache.InternalCache; +import org.apache.geode.protocol.exception.InvalidProtocolMessageException; +import org.apache.geode.protocol.protobuf.ClientProtocol; +import org.apache.geode.protocol.protobuf.ServerAPI; +import org.apache.geode.protocol.protobuf.serializer.ProtobufProtocolSerializer; +import org.apache.geode.protocol.protobuf.utilities.ProtobufRequestUtilities; +import org.apache.geode.protocol.protobuf.utilities.ProtobufUtilities; +import org.apache.geode.test.dunit.DistributedTestUtils; +import org.apache.geode.test.dunit.Host; +import org.apache.geode.test.dunit.VM; +import org.apache.geode.test.dunit.cache.internal.JUnit4CacheTestCase; +import org.apache.geode.test.dunit.rules.DistributedRestoreSystemProperties; +import org.apache.geode.test.junit.categories.DistributedTest; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.experimental.categories.Category; + +import java.io.IOException; +import java.net.Socket; + +import static org.junit.Assert.assertEquals; + +@Category(DistributedTest.class) +public class GetAvailableServersDUnitTest extends JUnit4CacheTestCase { + + @Rule + public DistributedRestoreSystemProperties distributedRestoreSystemProperties = + new DistributedRestoreSystemProperties(); + + @Before + public void setup() { + + } + + @Test + public void testGetAllAvailableServersRequest() + throws IOException, InvalidProtocolMessageException { +Host host = Host.getHost(0); +VM vm0 = host.getVM(0); +VM vm1 = host.getVM(1); +VM vm2 = host.getVM(2); + +int locatorPort = DistributedTestUtils.getDUnitLocatorPort(); + +// int cacheServer1Port = vm0.invoke("Start Cache1", () -> startCacheWithCacheServer()); --- End diff -- let's get rid off 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 pull request #673: GEODE-3284: New flow: getAvailableServers
Github user pivotal-amurmann commented on a diff in the pull request: https://github.com/apache/geode/pull/673#discussion_r130646340 --- Diff: geode-protobuf/src/main/proto/clientProtocol.proto --- @@ -43,13 +43,12 @@ message Request { GetAllRequest getAllRequest = 5; RemoveRequest removeRequest = 6; RemoveAllRequest removeAllRequest = 7; -ListKeysRequest listKeysRequest = 8; CreateRegionRequest createRegionRequest = 21; DestroyRegionRequest destroyRegionRequest = 22; -PingRequest pingRequest = 41; -GetServersRequest getServersRequest = 42; +//PingRequest pingRequest = 41; --- End diff -- what are you saving it for? --- 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 #673: GEODE-3284: New flow: getAvailableServers
Github user pivotal-amurmann commented on a diff in the pull request: https://github.com/apache/geode/pull/673#discussion_r130646425 --- Diff: geode-protobuf/src/main/proto/clientProtocol.proto --- @@ -64,15 +63,14 @@ message Response { GetAllResponse getAllResponse = 5; RemoveResponse removeResponse = 6; RemoveAllResponse removeAllResponse = 7; -ListKeysResponse listKeysResponse = 8; ErrorResponse errorResponse = 13; CreateRegionResponse createRegionResponse = 20; DestroyRegionResponse destroyRegionResponse = 21; -PingResponse pingResponse = 41; -GetServersResponse getServersResponse = 42; +//PingResponse pingResponse = 41; --- End diff -- same as above --- 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 #673: GEODE-3284: New flow: getAvailableServers
Github user pivotal-amurmann commented on a diff in the pull request: https://github.com/apache/geode/pull/673#discussion_r130646177 --- Diff: geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/utilities/ProtobufUtilities.java --- @@ -140,7 +141,7 @@ /** * This creates a MessageHeader - * + * --- End diff -- what's up with adding all this white trailing white space? Is this from spotless? --- 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 #673: GEODE-3284: New flow: getAvailableServers
Github user pivotal-amurmann commented on a diff in the pull request: https://github.com/apache/geode/pull/673#discussion_r130645759 --- Diff: geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/operations/GetAvailableServersOperationHandler.java --- @@ -0,0 +1,98 @@ +/* + * 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.protocol.protobuf.operations; + +import org.apache.commons.lang.StringUtils; +import org.apache.geode.cache.Cache; +import org.apache.geode.cache.client.internal.locator.GetAllServersRequest; +import org.apache.geode.cache.client.internal.locator.GetAllServersResponse; +import org.apache.geode.distributed.ConfigurationProperties; +import org.apache.geode.distributed.internal.InternalDistributedSystem; +import org.apache.geode.distributed.internal.ServerLocation; +import org.apache.geode.distributed.internal.tcpserver.TcpClient; +import org.apache.geode.internal.admin.remote.DistributionLocatorId; +import org.apache.geode.protocol.operations.OperationHandler; +import org.apache.geode.protocol.protobuf.BasicTypes; +import org.apache.geode.protocol.protobuf.Failure; +import org.apache.geode.protocol.protobuf.Result; +import org.apache.geode.protocol.protobuf.ServerAPI; +import org.apache.geode.protocol.protobuf.Success; +import org.apache.geode.serialization.SerializationService; + +import java.io.IOException; +import java.net.InetSocketAddress; +import java.util.Collection; +import java.util.HashSet; +import java.util.Properties; +import java.util.StringTokenizer; +import java.util.stream.Collectors; + +public class GetAvailableServersOperationHandler implements +OperationHandler { + + @Override + public Result process( + SerializationService serializationService, ServerAPI.GetAvailableServersRequest request, + Cache cache) { + +InternalDistributedSystem distributedSystem = +(InternalDistributedSystem) cache.getDistributedSystem(); +Properties properties = distributedSystem.getProperties(); +String locatorsString = properties.getProperty(ConfigurationProperties.LOCATORS); + +HashSet locators = new HashSet(); +StringTokenizer stringTokenizer = new StringTokenizer(locatorsString, ","); +while (stringTokenizer.hasMoreTokens()) { + String locator = stringTokenizer.nextToken(); + if (StringUtils.isNotEmpty(locator)) { +locators.add(new DistributionLocatorId(locator)); + } +} + +TcpClient tcpClient = getTcpClient(); +for (DistributionLocatorId locator : locators) { + try { +return getGetAvailableServersFromLocator(tcpClient, locator.getHost()); + } catch (IOException | ClassNotFoundException e) { +// try the next locator + } +} +return Failure +.of(BasicTypes.ErrorResponse.newBuilder().setMessage("Unable to find a locator").build()); + } + + private Result getGetAvailableServersFromLocator( + TcpClient tcpClient, InetSocketAddress address) throws IOException, ClassNotFoundException { +GetAllServersResponse getAllServersResponse = (GetAllServersResponse) tcpClient +.requestToServer(address, new GetAllServersRequest(), 1000, true); +Collection servers = +(Collection) getAllServersResponse.getServers().stream() +.map(serverLocation -> getServerProtobufMessage((ServerLocation) serverLocation)) +.collect(Collectors.toList()); +ServerAPI.GetAvailableServersResponse.Builder builder = + ServerAPI.GetAvailableServersResponse.newBuilder().addAllServers(servers); --- End diff -- could we incrementally build this up as part of the stream processing? --- 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
[GitHub] geode pull request #673: GEODE-3284: New flow: getAvailableServers
Github user pivotal-amurmann commented on a diff in the pull request: https://github.com/apache/geode/pull/673#discussion_r130644766 --- Diff: geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/operations/GetAvailableServersOperationHandler.java --- @@ -0,0 +1,98 @@ +/* + * 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.protocol.protobuf.operations; + +import org.apache.commons.lang.StringUtils; +import org.apache.geode.cache.Cache; +import org.apache.geode.cache.client.internal.locator.GetAllServersRequest; +import org.apache.geode.cache.client.internal.locator.GetAllServersResponse; +import org.apache.geode.distributed.ConfigurationProperties; +import org.apache.geode.distributed.internal.InternalDistributedSystem; +import org.apache.geode.distributed.internal.ServerLocation; +import org.apache.geode.distributed.internal.tcpserver.TcpClient; +import org.apache.geode.internal.admin.remote.DistributionLocatorId; +import org.apache.geode.protocol.operations.OperationHandler; +import org.apache.geode.protocol.protobuf.BasicTypes; +import org.apache.geode.protocol.protobuf.Failure; +import org.apache.geode.protocol.protobuf.Result; +import org.apache.geode.protocol.protobuf.ServerAPI; +import org.apache.geode.protocol.protobuf.Success; +import org.apache.geode.serialization.SerializationService; + +import java.io.IOException; +import java.net.InetSocketAddress; +import java.util.Collection; +import java.util.HashSet; +import java.util.Properties; +import java.util.StringTokenizer; +import java.util.stream.Collectors; + +public class GetAvailableServersOperationHandler implements +OperationHandler { + + @Override + public Result process( + SerializationService serializationService, ServerAPI.GetAvailableServersRequest request, + Cache cache) { + +InternalDistributedSystem distributedSystem = +(InternalDistributedSystem) cache.getDistributedSystem(); +Properties properties = distributedSystem.getProperties(); +String locatorsString = properties.getProperty(ConfigurationProperties.LOCATORS); + +HashSet locators = new HashSet(); +StringTokenizer stringTokenizer = new StringTokenizer(locatorsString, ","); +while (stringTokenizer.hasMoreTokens()) { + String locator = stringTokenizer.nextToken(); + if (StringUtils.isNotEmpty(locator)) { +locators.add(new DistributionLocatorId(locator)); + } +} + +TcpClient tcpClient = getTcpClient(); +for (DistributionLocatorId locator : locators) { --- End diff -- this is not super pretty. What do you think about using Stream#findFirst for 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 pull request #673: GEODE-3284: New flow: getAvailableServers
Github user pivotal-amurmann commented on a diff in the pull request: https://github.com/apache/geode/pull/673#discussion_r130642543 --- Diff: geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/operations/GetAvailableServersOperationHandler.java --- @@ -0,0 +1,98 @@ +/* + * 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.protocol.protobuf.operations; + +import org.apache.commons.lang.StringUtils; +import org.apache.geode.cache.Cache; +import org.apache.geode.cache.client.internal.locator.GetAllServersRequest; +import org.apache.geode.cache.client.internal.locator.GetAllServersResponse; +import org.apache.geode.distributed.ConfigurationProperties; +import org.apache.geode.distributed.internal.InternalDistributedSystem; +import org.apache.geode.distributed.internal.ServerLocation; +import org.apache.geode.distributed.internal.tcpserver.TcpClient; +import org.apache.geode.internal.admin.remote.DistributionLocatorId; +import org.apache.geode.protocol.operations.OperationHandler; +import org.apache.geode.protocol.protobuf.BasicTypes; +import org.apache.geode.protocol.protobuf.Failure; +import org.apache.geode.protocol.protobuf.Result; +import org.apache.geode.protocol.protobuf.ServerAPI; +import org.apache.geode.protocol.protobuf.Success; +import org.apache.geode.serialization.SerializationService; + +import java.io.IOException; +import java.net.InetSocketAddress; +import java.util.Collection; +import java.util.HashSet; +import java.util.Properties; +import java.util.StringTokenizer; +import java.util.stream.Collectors; + +public class GetAvailableServersOperationHandler implements +OperationHandler { + + @Override + public Result process( + SerializationService serializationService, ServerAPI.GetAvailableServersRequest request, + Cache cache) { + +InternalDistributedSystem distributedSystem = +(InternalDistributedSystem) cache.getDistributedSystem(); +Properties properties = distributedSystem.getProperties(); +String locatorsString = properties.getProperty(ConfigurationProperties.LOCATORS); --- End diff -- Could we pull the entire calculation of locators out either into a private method or a service object? I think that would make it much easier to understand what this method is doing. --- 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 #673: GEODE-3284: New flow: getAvailableServers
GitHub user WireBaron opened a pull request: https://github.com/apache/geode/pull/673 GEODE-3284: New flow: getAvailableServers @galen-pivotal @kohlmu-pivotal @bschuchardt @hiteshk25 @pivotal-amurmann Signed-off-by: Bruce Schuchardt 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? - [ ] 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? - [ ] 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/WireBaron/geode feature/GEODE-3284 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode/pull/673.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 #673 commit 6da72b504ed83d80fde60e8e661badab4d3d33bb Author: Udo Kohlmeyer Date: 2017-07-31T16:23:53Z GEODE-3284: New flow: getAvailableServers Signed-off-by: Bruce Schuchardt --- 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. ---