[
https://issues.apache.org/jira/browse/GEODE-8872?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17294031#comment-17294031
]
ASF GitHub Bot commented on GEODE-8872:
---------------------------------------
DonalEvans commented on a change in pull request #5948:
URL: https://github.com/apache/geode/pull/5948#discussion_r585940678
##########
File path: geode-core/src/main/java/org/apache/geode/cache/client/Pool.java
##########
@@ -207,6 +207,13 @@ default boolean getThreadLocalConnections() {
*/
boolean getMultiuserAuthentication();
+ /**
+ * Returns true ir request locator internal address is enabled on this pool.
+ *
+ * @see PoolFactory#setRequestLocatorInternalAddress(boolean)
+ * @since Geode 1.14
+ */
+ boolean getRequestLocatorInternalAddress();
Review comment:
This method name is a little confusing, as it might be mistaken for a
getter for the locator internal address itself. It might be better as
"isRequestLocatorInternalAddressEnabled", and the corresponding setter as
"setRequestLocatorInternalAddressEnabled".
##########
File path:
geode-core/src/main/java/org/apache/geode/internal/cache/GridAdvisor.java
##########
@@ -257,6 +257,9 @@ public Set adviseProfileRemove() {
private String host;
+ /** valid only for ControllerProfile */
+ private String internalHost;
Review comment:
It doesn't seem correct to have a field in a parent class that only
belongs to one of its child classes. Can this be moved into ControllerProfile?
##########
File path:
geode-core/src/main/java/org/apache/geode/cache/client/PoolFactory.java
##########
@@ -220,6 +220,14 @@
* @since GemFire 6.5
*/
boolean DEFAULT_MULTIUSER_AUTHENTICATION = false;
+ /**
+ * The default value for whether to request locator internal address.
Review comment:
Slightly more detail would be good here, explaining when exactly the
locator internal address would be requested if this was set to true.
##########
File path: geode-core/src/main/java/org/apache/geode/cache/client/Pool.java
##########
@@ -207,6 +207,13 @@ default boolean getThreadLocalConnections() {
*/
boolean getMultiuserAuthentication();
+ /**
+ * Returns true ir request locator internal address is enabled on this pool.
Review comment:
Typo here, should be "Returns true if..."
##########
File path:
geode-dunit/src/main/java/org/apache/geode/cache/client/internal/LocatorTestBase.java
##########
@@ -141,6 +141,34 @@ protected int startLocator(final String hostName, final
String otherLocators) th
return locator.getPort();
}
+ protected int startLocator(final String hostName, final String otherLocators,
Review comment:
This method is never used.
##########
File path:
geode-core/src/main/java/org/apache/geode/internal/cache/GridAdvisor.java
##########
@@ -432,6 +447,17 @@ public boolean equals(Object obj) {
if (this.gp.getPort() == other.gp.getPort()) {
final String thisHost = this.gp.getHost();
final String otherHost = other.gp.getHost();
+ final String thisInternalHost = this.gp.getInternalHost();
Review comment:
If internalHost is being included in `equals()` calculations, it should
probably also be included in hashCode()`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Add client option, to request locators internal host addresses
> ---------------------------------------------------------------
>
> Key: GEODE-8872
> URL: https://issues.apache.org/jira/browse/GEODE-8872
> Project: Geode
> Issue Type: New Feature
> Components: client/server
> Reporter: Mario Ivanac
> Assignee: Mario Ivanac
> Priority: Major
> Labels: pull-request-available
>
> Additional option for clients, when pool is created, to request locators
> internal host addresses.
> When sending LocatorListRequest in some cases we need to get internal host
> addresses.
> To describe use case:
> When deploying geode, for locator we define hostname-for-clients. This is
> external address used for WAN, and external clients.
> If we also deploy some clients in internal network (for example in the same
> kubernetes cluster as geode), then for those clients to connect with locator,
> communication will go from internal network to external, then from external
> to internal. This increases latency, and we should communicate over the
> shortest path.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
