You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by "nva (via GitHub)" <gi...@apache.org> on 2025/11/21 08:59:17 UTC
[PR] Ignite 26893 [ignite-3]
nva opened a new pull request, #7031:
URL: https://github.com/apache/ignite-3/pull/7031
Thank you for submitting the pull request.
To streamline the review process of the patch and ensure better code quality
we ask both an author and a reviewer to verify the following:
### The Review Checklist
- [ ] **Formal criteria:** TC status, codestyle, mandatory documentation. Also make sure to complete the following:
\- There is a single JIRA ticket related to the pull request.
\- The web-link to the pull request is attached to the JIRA ticket.
\- The JIRA ticket has the Patch Available state.
\- The description of the JIRA ticket explains WHAT was made, WHY and HOW.
\- The pull request title is treated as the final commit message. The following pattern must be used: IGNITE-XXXX Change summary where XXXX - number of JIRA issue.
- [ ] **Design:** new code conforms with the design principles of the components it is added to.
- [ ] **Patch quality:** patch cannot be split into smaller pieces, its size must be reasonable.
- [ ] **Code quality:** code is clean and readable, necessary developer documentation is added if needed.
- [ ] **Tests code quality:** test set covers positive/negative scenarios, happy/edge cases. Tests are effective in terms of execution time and resources.
### Notes
- [Apache Ignite Coding Guidelines](https://cwiki.apache.org/confluence/display/IGNITE/Java+Code+Style+Guide)
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-26893 Java thin: improve DNS resolution logic [ignite-3]
Posted by "ptupitsyn (via GitHub)" <gi...@apache.org>.
ptupitsyn commented on code in PR #7031:
URL: https://github.com/apache/ignite-3/pull/7031#discussion_r2584932167
##########
modules/client/src/main/java/org/apache/ignite/internal/client/ReliableChannel.java:
##########
@@ -773,7 +800,14 @@ private void onObservableTimestampReceived(long newTs) {
}
private void onPartitionAssignmentChanged(long timestamp) {
- partitionAssignmentTimestamp.updateAndGet(curTs -> Math.max(curTs, timestamp));
+ var old = partitionAssignmentTimestamp.getAndUpdate(curTs -> Math.max(curTs, timestamp));
+
+ if (timestamp > old) {
+ // New assignment timestamp, topology change possible.
+ if (scheduledChannelsReinit.compareAndSet(false, true)) {
+ initChannelHolders();
Review Comment:
We are on a Netty thread here and we should not do any heavy lifting to avoid blocking the IO event loop.
We can either raise some flag to refresh later, on use a separate executor.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-26893 Java thin: improve DNS resolution logic [ignite-3]
Posted by "nva (via GitHub)" <gi...@apache.org>.
nva commented on code in PR #7031:
URL: https://github.com/apache/ignite-3/pull/7031#discussion_r2596977372
##########
modules/client/src/main/java/org/apache/ignite/internal/client/ReliableChannel.java:
##########
@@ -773,7 +800,14 @@ private void onObservableTimestampReceived(long newTs) {
}
private void onPartitionAssignmentChanged(long timestamp) {
- partitionAssignmentTimestamp.updateAndGet(curTs -> Math.max(curTs, timestamp));
+ var old = partitionAssignmentTimestamp.getAndUpdate(curTs -> Math.max(curTs, timestamp));
+
+ if (timestamp > old) {
+ // New assignment timestamp, topology change possible.
+ if (scheduledChannelsReinit.compareAndSet(false, true)) {
+ initChannelHolders();
Review Comment:
Fixed
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-26893 Java thin: improve DNS resolution logic [ignite-3]
Posted by "ptupitsyn (via GitHub)" <gi...@apache.org>.
ptupitsyn commented on code in PR #7031:
URL: https://github.com/apache/ignite-3/pull/7031#discussion_r2570836122
##########
modules/client/src/main/java/org/apache/ignite/internal/client/ReliableChannel.java:
##########
@@ -414,20 +423,46 @@ public CompletableFuture<ClientChannel> getChannelAsync(@Nullable String preferr
* @return host:port_range address lines parsed as {@link InetSocketAddress} as a key. Value is the amount of appearences of an address
* in {@code addrs} parameter.
*/
- private static Map<InetSocketAddress, Integer> parsedAddresses(String[] addrs) {
+ private static Map<InetSocketAddress, Integer> parsedAddresses(InetAddressResolver addressResolver, String[] addrs) {
if (addrs == null || addrs.length == 0) {
throw new IgniteException(CONFIGURATION_ERR, "Empty addresses");
}
- Collection<HostAndPort> ranges = new ArrayList<>(addrs.length);
+ Collection<HostAndPort> parsedAddrs = new ArrayList<>(addrs.length);
for (String a : addrs) {
- ranges.add(HostAndPort.parse(a, IgniteClientConfiguration.DFLT_PORT, "Failed to parse Ignite server address"));
+ parsedAddrs.add(HostAndPort.parse(a, IgniteClientConfiguration.DFLT_PORT, "Failed to parse Ignite server address"));
}
- return ranges.stream()
- .map(p -> InetSocketAddress.createUnresolved(p.host(), p.port()))
- .collect(Collectors.toMap(a -> a, a -> 1, Integer::sum));
+ var map = new HashMap<InetSocketAddress, Integer>(parsedAddrs.size());
+
+ for (HostAndPort addr : parsedAddrs) {
+ try {
+ // Special handling for "localhost" to avoid unnecessary DNS resolution.
+ if ("localhost".equalsIgnoreCase(addr.host())) {
+ map.merge(InetSocketAddress.createUnresolved(addr.host(), addr.port()), 1, Integer::sum);
+
+ continue;
Review Comment:
Does `getAllByName` really return multiple IPs for localhost?
##########
modules/client/src/main/java/org/apache/ignite/internal/client/ReliableChannel.java:
##########
@@ -414,20 +423,46 @@ public CompletableFuture<ClientChannel> getChannelAsync(@Nullable String preferr
* @return host:port_range address lines parsed as {@link InetSocketAddress} as a key. Value is the amount of appearences of an address
* in {@code addrs} parameter.
*/
- private static Map<InetSocketAddress, Integer> parsedAddresses(String[] addrs) {
+ private static Map<InetSocketAddress, Integer> parsedAddresses(InetAddressResolver addressResolver, String[] addrs) {
if (addrs == null || addrs.length == 0) {
throw new IgniteException(CONFIGURATION_ERR, "Empty addresses");
}
- Collection<HostAndPort> ranges = new ArrayList<>(addrs.length);
+ Collection<HostAndPort> parsedAddrs = new ArrayList<>(addrs.length);
for (String a : addrs) {
- ranges.add(HostAndPort.parse(a, IgniteClientConfiguration.DFLT_PORT, "Failed to parse Ignite server address"));
+ parsedAddrs.add(HostAndPort.parse(a, IgniteClientConfiguration.DFLT_PORT, "Failed to parse Ignite server address"));
}
- return ranges.stream()
- .map(p -> InetSocketAddress.createUnresolved(p.host(), p.port()))
- .collect(Collectors.toMap(a -> a, a -> 1, Integer::sum));
+ var map = new HashMap<InetSocketAddress, Integer>(parsedAddrs.size());
+
+ for (HostAndPort addr : parsedAddrs) {
+ try {
+ // Special handling for "localhost" to avoid unnecessary DNS resolution.
+ if ("localhost".equalsIgnoreCase(addr.host())) {
+ map.merge(InetSocketAddress.createUnresolved(addr.host(), addr.port()), 1, Integer::sum);
+
+ continue;
+ }
+
+ for (InetAddress inetAddr : addressResolver.getAllByName(addr.host())) {
+ // Preserve unresolved address if the resolved address equals to the original host string.
Review Comment:
Why does the order change?
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-26893 Java thin: improve DNS resolution logic. [ignite-3]
Posted by "nva (via GitHub)" <gi...@apache.org>.
nva commented on code in PR #7031:
URL: https://github.com/apache/ignite-3/pull/7031#discussion_r2570805356
##########
modules/client/src/main/java/org/apache/ignite/internal/client/ReliableChannel.java:
##########
@@ -414,20 +423,46 @@ public CompletableFuture<ClientChannel> getChannelAsync(@Nullable String preferr
* @return host:port_range address lines parsed as {@link InetSocketAddress} as a key. Value is the amount of appearences of an address
* in {@code addrs} parameter.
*/
- private static Map<InetSocketAddress, Integer> parsedAddresses(String[] addrs) {
+ private static Map<InetSocketAddress, Integer> parsedAddresses(InetAddressResolver addressResolver, String[] addrs) {
if (addrs == null || addrs.length == 0) {
throw new IgniteException(CONFIGURATION_ERR, "Empty addresses");
}
- Collection<HostAndPort> ranges = new ArrayList<>(addrs.length);
+ Collection<HostAndPort> parsedAddrs = new ArrayList<>(addrs.length);
for (String a : addrs) {
- ranges.add(HostAndPort.parse(a, IgniteClientConfiguration.DFLT_PORT, "Failed to parse Ignite server address"));
+ parsedAddrs.add(HostAndPort.parse(a, IgniteClientConfiguration.DFLT_PORT, "Failed to parse Ignite server address"));
}
- return ranges.stream()
- .map(p -> InetSocketAddress.createUnresolved(p.host(), p.port()))
- .collect(Collectors.toMap(a -> a, a -> 1, Integer::sum));
+ var map = new HashMap<InetSocketAddress, Integer>(parsedAddrs.size());
+
+ for (HostAndPort addr : parsedAddrs) {
+ try {
+ // Special handling for "localhost" to avoid unnecessary DNS resolution.
+ if ("localhost".equalsIgnoreCase(addr.host())) {
+ map.merge(InetSocketAddress.createUnresolved(addr.host(), addr.port()), 1, Integer::sum);
+
+ continue;
+ }
+
+ for (InetAddress inetAddr : addressResolver.getAllByName(addr.host())) {
+ // Preserve unresolved address if the resolved address equals to the original host string.
Review Comment:
All IP addresses remain unresolved to preserve connection order in tests. Without this fix, we need to fix RequestBalancingTest, ClientPartitionManagerTest, ...
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-26893 Java thin: improve DNS resolution logic [ignite-3]
Posted by "nva (via GitHub)" <gi...@apache.org>.
nva commented on code in PR #7031:
URL: https://github.com/apache/ignite-3/pull/7031#discussion_r2580796776
##########
modules/client/src/main/java/org/apache/ignite/internal/client/ReliableChannel.java:
##########
@@ -414,20 +423,46 @@ public CompletableFuture<ClientChannel> getChannelAsync(@Nullable String preferr
* @return host:port_range address lines parsed as {@link InetSocketAddress} as a key. Value is the amount of appearences of an address
* in {@code addrs} parameter.
*/
- private static Map<InetSocketAddress, Integer> parsedAddresses(String[] addrs) {
+ private static Map<InetSocketAddress, Integer> parsedAddresses(InetAddressResolver addressResolver, String[] addrs) {
if (addrs == null || addrs.length == 0) {
throw new IgniteException(CONFIGURATION_ERR, "Empty addresses");
}
- Collection<HostAndPort> ranges = new ArrayList<>(addrs.length);
+ Collection<HostAndPort> parsedAddrs = new ArrayList<>(addrs.length);
for (String a : addrs) {
- ranges.add(HostAndPort.parse(a, IgniteClientConfiguration.DFLT_PORT, "Failed to parse Ignite server address"));
+ parsedAddrs.add(HostAndPort.parse(a, IgniteClientConfiguration.DFLT_PORT, "Failed to parse Ignite server address"));
}
- return ranges.stream()
- .map(p -> InetSocketAddress.createUnresolved(p.host(), p.port()))
- .collect(Collectors.toMap(a -> a, a -> 1, Integer::sum));
+ var map = new HashMap<InetSocketAddress, Integer>(parsedAddrs.size());
+
+ for (HostAndPort addr : parsedAddrs) {
+ try {
+ // Special handling for "localhost" to avoid unnecessary DNS resolution.
+ if ("localhost".equalsIgnoreCase(addr.host())) {
+ map.merge(InetSocketAddress.createUnresolved(addr.host(), addr.port()), 1, Integer::sum);
+
+ continue;
+ }
+
+ for (InetAddress inetAddr : addressResolver.getAllByName(addr.host())) {
+ // Preserve unresolved address if the resolved address equals to the original host string.
Review Comment:
Since InetSocketAddress will now have a non-null value for the `addr` field, InetSocketAddressHolder#hashCode will return a different value.
InetSocketAddress#createUnresolved, which was used earlier, will create an InetSocketAddress with the 'addr` field equal to null.
I redesigned the resolution logic, keeping only the part related to the loopback. This should cover all cases in our tests
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-26893 Java thin: improve DNS resolution logic [ignite-3]
Posted by "nva (via GitHub)" <gi...@apache.org>.
nva commented on code in PR #7031:
URL: https://github.com/apache/ignite-3/pull/7031#discussion_r2580889217
##########
modules/client/src/main/java/org/apache/ignite/internal/client/ReliableChannel.java:
##########
@@ -414,20 +423,46 @@ public CompletableFuture<ClientChannel> getChannelAsync(@Nullable String preferr
* @return host:port_range address lines parsed as {@link InetSocketAddress} as a key. Value is the amount of appearences of an address
* in {@code addrs} parameter.
*/
- private static Map<InetSocketAddress, Integer> parsedAddresses(String[] addrs) {
+ private static Map<InetSocketAddress, Integer> parsedAddresses(InetAddressResolver addressResolver, String[] addrs) {
if (addrs == null || addrs.length == 0) {
throw new IgniteException(CONFIGURATION_ERR, "Empty addresses");
}
- Collection<HostAndPort> ranges = new ArrayList<>(addrs.length);
+ Collection<HostAndPort> parsedAddrs = new ArrayList<>(addrs.length);
for (String a : addrs) {
- ranges.add(HostAndPort.parse(a, IgniteClientConfiguration.DFLT_PORT, "Failed to parse Ignite server address"));
+ parsedAddrs.add(HostAndPort.parse(a, IgniteClientConfiguration.DFLT_PORT, "Failed to parse Ignite server address"));
}
- return ranges.stream()
- .map(p -> InetSocketAddress.createUnresolved(p.host(), p.port()))
- .collect(Collectors.toMap(a -> a, a -> 1, Integer::sum));
+ var map = new HashMap<InetSocketAddress, Integer>(parsedAddrs.size());
+
+ for (HostAndPort addr : parsedAddrs) {
+ try {
+ // Special handling for "localhost" to avoid unnecessary DNS resolution.
+ if ("localhost".equalsIgnoreCase(addr.host())) {
+ map.merge(InetSocketAddress.createUnresolved(addr.host(), addr.port()), 1, Integer::sum);
+
+ continue;
Review Comment:
Added
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-26893 Java thin: improve DNS resolution logic [ignite-3]
Posted by "nva (via GitHub)" <gi...@apache.org>.
nva merged PR #7031:
URL: https://github.com/apache/ignite-3/pull/7031
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-26893 Java thin: improve DNS resolution logic [ignite-3]
Posted by "ptupitsyn (via GitHub)" <gi...@apache.org>.
ptupitsyn commented on code in PR #7031:
URL: https://github.com/apache/ignite-3/pull/7031#discussion_r2580770060
##########
modules/client/src/main/java/org/apache/ignite/internal/client/ReliableChannel.java:
##########
@@ -414,20 +423,46 @@ public CompletableFuture<ClientChannel> getChannelAsync(@Nullable String preferr
* @return host:port_range address lines parsed as {@link InetSocketAddress} as a key. Value is the amount of appearences of an address
* in {@code addrs} parameter.
*/
- private static Map<InetSocketAddress, Integer> parsedAddresses(String[] addrs) {
+ private static Map<InetSocketAddress, Integer> parsedAddresses(InetAddressResolver addressResolver, String[] addrs) {
if (addrs == null || addrs.length == 0) {
throw new IgniteException(CONFIGURATION_ERR, "Empty addresses");
}
- Collection<HostAndPort> ranges = new ArrayList<>(addrs.length);
+ Collection<HostAndPort> parsedAddrs = new ArrayList<>(addrs.length);
for (String a : addrs) {
- ranges.add(HostAndPort.parse(a, IgniteClientConfiguration.DFLT_PORT, "Failed to parse Ignite server address"));
+ parsedAddrs.add(HostAndPort.parse(a, IgniteClientConfiguration.DFLT_PORT, "Failed to parse Ignite server address"));
}
- return ranges.stream()
- .map(p -> InetSocketAddress.createUnresolved(p.host(), p.port()))
- .collect(Collectors.toMap(a -> a, a -> 1, Integer::sum));
+ var map = new HashMap<InetSocketAddress, Integer>(parsedAddrs.size());
+
+ for (HostAndPort addr : parsedAddrs) {
+ try {
+ // Special handling for "localhost" to avoid unnecessary DNS resolution.
+ if ("localhost".equalsIgnoreCase(addr.host())) {
+ map.merge(InetSocketAddress.createUnresolved(addr.host(), addr.port()), 1, Integer::sum);
+
+ continue;
Review Comment:
Maybe we should use `isLoopbackAddress` to remove all loopback duplicates.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-26893 Java thin: improve DNS resolution logic. [ignite-3]
Posted by "ptupitsyn (via GitHub)" <gi...@apache.org>.
ptupitsyn commented on code in PR #7031:
URL: https://github.com/apache/ignite-3/pull/7031#discussion_r2567758169
##########
modules/client/src/main/java/org/apache/ignite/internal/client/ReliableChannel.java:
##########
@@ -414,20 +423,46 @@ public CompletableFuture<ClientChannel> getChannelAsync(@Nullable String preferr
* @return host:port_range address lines parsed as {@link InetSocketAddress} as a key. Value is the amount of appearences of an address
* in {@code addrs} parameter.
*/
- private static Map<InetSocketAddress, Integer> parsedAddresses(String[] addrs) {
+ private static Map<InetSocketAddress, Integer> parsedAddresses(InetAddressResolver addressResolver, String[] addrs) {
if (addrs == null || addrs.length == 0) {
throw new IgniteException(CONFIGURATION_ERR, "Empty addresses");
}
- Collection<HostAndPort> ranges = new ArrayList<>(addrs.length);
+ Collection<HostAndPort> parsedAddrs = new ArrayList<>(addrs.length);
for (String a : addrs) {
- ranges.add(HostAndPort.parse(a, IgniteClientConfiguration.DFLT_PORT, "Failed to parse Ignite server address"));
+ parsedAddrs.add(HostAndPort.parse(a, IgniteClientConfiguration.DFLT_PORT, "Failed to parse Ignite server address"));
}
- return ranges.stream()
- .map(p -> InetSocketAddress.createUnresolved(p.host(), p.port()))
- .collect(Collectors.toMap(a -> a, a -> 1, Integer::sum));
+ var map = new HashMap<InetSocketAddress, Integer>(parsedAddrs.size());
+
+ for (HostAndPort addr : parsedAddrs) {
+ try {
+ // Special handling for "localhost" to avoid unnecessary DNS resolution.
+ if ("localhost".equalsIgnoreCase(addr.host())) {
+ map.merge(InetSocketAddress.createUnresolved(addr.host(), addr.port()), 1, Integer::sum);
+
+ continue;
Review Comment:
Not sure about this, `localhost` can be customized in the hosts file for various reasons. And the OS should deal with that.
##########
modules/client/src/main/java/org/apache/ignite/internal/client/ReliableChannel.java:
##########
@@ -414,20 +423,46 @@ public CompletableFuture<ClientChannel> getChannelAsync(@Nullable String preferr
* @return host:port_range address lines parsed as {@link InetSocketAddress} as a key. Value is the amount of appearences of an address
* in {@code addrs} parameter.
*/
- private static Map<InetSocketAddress, Integer> parsedAddresses(String[] addrs) {
+ private static Map<InetSocketAddress, Integer> parsedAddresses(InetAddressResolver addressResolver, String[] addrs) {
if (addrs == null || addrs.length == 0) {
throw new IgniteException(CONFIGURATION_ERR, "Empty addresses");
}
- Collection<HostAndPort> ranges = new ArrayList<>(addrs.length);
+ Collection<HostAndPort> parsedAddrs = new ArrayList<>(addrs.length);
for (String a : addrs) {
- ranges.add(HostAndPort.parse(a, IgniteClientConfiguration.DFLT_PORT, "Failed to parse Ignite server address"));
+ parsedAddrs.add(HostAndPort.parse(a, IgniteClientConfiguration.DFLT_PORT, "Failed to parse Ignite server address"));
}
- return ranges.stream()
- .map(p -> InetSocketAddress.createUnresolved(p.host(), p.port()))
- .collect(Collectors.toMap(a -> a, a -> 1, Integer::sum));
+ var map = new HashMap<InetSocketAddress, Integer>(parsedAddrs.size());
+
+ for (HostAndPort addr : parsedAddrs) {
+ try {
+ // Special handling for "localhost" to avoid unnecessary DNS resolution.
+ if ("localhost".equalsIgnoreCase(addr.host())) {
+ map.merge(InetSocketAddress.createUnresolved(addr.host(), addr.port()), 1, Integer::sum);
+
+ continue;
+ }
+
+ for (InetAddress inetAddr : addressResolver.getAllByName(addr.host())) {
+ // Preserve unresolved address if the resolved address equals to the original host string.
Review Comment:
Please elaborate, why do we need this special case?
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-26893 Java thin: improve DNS resolution logic. [ignite-3]
Posted by "nva (via GitHub)" <gi...@apache.org>.
nva commented on code in PR #7031:
URL: https://github.com/apache/ignite-3/pull/7031#discussion_r2570821119
##########
modules/client/src/main/java/org/apache/ignite/internal/client/ReliableChannel.java:
##########
@@ -414,20 +423,46 @@ public CompletableFuture<ClientChannel> getChannelAsync(@Nullable String preferr
* @return host:port_range address lines parsed as {@link InetSocketAddress} as a key. Value is the amount of appearences of an address
* in {@code addrs} parameter.
*/
- private static Map<InetSocketAddress, Integer> parsedAddresses(String[] addrs) {
+ private static Map<InetSocketAddress, Integer> parsedAddresses(InetAddressResolver addressResolver, String[] addrs) {
if (addrs == null || addrs.length == 0) {
throw new IgniteException(CONFIGURATION_ERR, "Empty addresses");
}
- Collection<HostAndPort> ranges = new ArrayList<>(addrs.length);
+ Collection<HostAndPort> parsedAddrs = new ArrayList<>(addrs.length);
for (String a : addrs) {
- ranges.add(HostAndPort.parse(a, IgniteClientConfiguration.DFLT_PORT, "Failed to parse Ignite server address"));
+ parsedAddrs.add(HostAndPort.parse(a, IgniteClientConfiguration.DFLT_PORT, "Failed to parse Ignite server address"));
}
- return ranges.stream()
- .map(p -> InetSocketAddress.createUnresolved(p.host(), p.port()))
- .collect(Collectors.toMap(a -> a, a -> 1, Integer::sum));
+ var map = new HashMap<InetSocketAddress, Integer>(parsedAddrs.size());
+
+ for (HostAndPort addr : parsedAddrs) {
+ try {
+ // Special handling for "localhost" to avoid unnecessary DNS resolution.
+ if ("localhost".equalsIgnoreCase(addr.host())) {
+ map.merge(InetSocketAddress.createUnresolved(addr.host(), addr.port()), 1, Integer::sum);
+
+ continue;
Review Comment:
Looks like you're right. Potentially, we have more than one name (localhost, InetAddress.getLocalHost().getHostName()) for the loopback interface.
It might be a problem since the client will try to connect to all loopback addresses (127.0.0.1, 0:0:0:0:0:0:0:1) if you set "localhost".
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-26893 Java thin: improve DNS resolution logic [ignite-3]
Posted by "ptupitsyn (via GitHub)" <gi...@apache.org>.
ptupitsyn commented on code in PR #7031:
URL: https://github.com/apache/ignite-3/pull/7031#discussion_r2591886331
##########
modules/client/src/main/java/org/apache/ignite/internal/client/ReliableChannel.java:
##########
@@ -773,7 +800,14 @@ private void onObservableTimestampReceived(long newTs) {
}
private void onPartitionAssignmentChanged(long timestamp) {
- partitionAssignmentTimestamp.updateAndGet(curTs -> Math.max(curTs, timestamp));
+ var old = partitionAssignmentTimestamp.getAndUpdate(curTs -> Math.max(curTs, timestamp));
+
+ if (timestamp > old) {
+ // New assignment timestamp, topology change possible.
+ if (scheduledChannelsReinit.compareAndSet(false, true)) {
+ initChannelHolders();
Review Comment:
I think we can use `clientCfg.asyncContinuationExecutor()`, let's avoid starting more threads.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-26893 Java thin: improve DNS resolution logic [ignite-3]
Posted by "nva (via GitHub)" <gi...@apache.org>.
nva commented on code in PR #7031:
URL: https://github.com/apache/ignite-3/pull/7031#discussion_r2580726449
##########
modules/client/src/main/java/org/apache/ignite/internal/client/ReliableChannel.java:
##########
@@ -414,20 +423,46 @@ public CompletableFuture<ClientChannel> getChannelAsync(@Nullable String preferr
* @return host:port_range address lines parsed as {@link InetSocketAddress} as a key. Value is the amount of appearences of an address
* in {@code addrs} parameter.
*/
- private static Map<InetSocketAddress, Integer> parsedAddresses(String[] addrs) {
+ private static Map<InetSocketAddress, Integer> parsedAddresses(InetAddressResolver addressResolver, String[] addrs) {
if (addrs == null || addrs.length == 0) {
throw new IgniteException(CONFIGURATION_ERR, "Empty addresses");
}
- Collection<HostAndPort> ranges = new ArrayList<>(addrs.length);
+ Collection<HostAndPort> parsedAddrs = new ArrayList<>(addrs.length);
for (String a : addrs) {
- ranges.add(HostAndPort.parse(a, IgniteClientConfiguration.DFLT_PORT, "Failed to parse Ignite server address"));
+ parsedAddrs.add(HostAndPort.parse(a, IgniteClientConfiguration.DFLT_PORT, "Failed to parse Ignite server address"));
}
- return ranges.stream()
- .map(p -> InetSocketAddress.createUnresolved(p.host(), p.port()))
- .collect(Collectors.toMap(a -> a, a -> 1, Integer::sum));
+ var map = new HashMap<InetSocketAddress, Integer>(parsedAddrs.size());
+
+ for (HostAndPort addr : parsedAddrs) {
+ try {
+ // Special handling for "localhost" to avoid unnecessary DNS resolution.
+ if ("localhost".equalsIgnoreCase(addr.host())) {
+ map.merge(InetSocketAddress.createUnresolved(addr.host(), addr.port()), 1, Integer::sum);
+
+ continue;
Review Comment:
Yes (127.0.0.1, 0:0:0:0:0:0:0:1)
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-26893 Java thin: improve DNS resolution logic. [ignite-3]
Posted by "zstan (via GitHub)" <gi...@apache.org>.
zstan commented on code in PR #7031:
URL: https://github.com/apache/ignite-3/pull/7031#discussion_r2570832025
##########
modules/client/src/main/java/org/apache/ignite/internal/client/ReliableChannel.java:
##########
@@ -414,20 +423,46 @@ public CompletableFuture<ClientChannel> getChannelAsync(@Nullable String preferr
* @return host:port_range address lines parsed as {@link InetSocketAddress} as a key. Value is the amount of appearences of an address
* in {@code addrs} parameter.
*/
- private static Map<InetSocketAddress, Integer> parsedAddresses(String[] addrs) {
+ private static Map<InetSocketAddress, Integer> parsedAddresses(InetAddressResolver addressResolver, String[] addrs) {
if (addrs == null || addrs.length == 0) {
throw new IgniteException(CONFIGURATION_ERR, "Empty addresses");
}
- Collection<HostAndPort> ranges = new ArrayList<>(addrs.length);
+ Collection<HostAndPort> parsedAddrs = new ArrayList<>(addrs.length);
for (String a : addrs) {
- ranges.add(HostAndPort.parse(a, IgniteClientConfiguration.DFLT_PORT, "Failed to parse Ignite server address"));
+ parsedAddrs.add(HostAndPort.parse(a, IgniteClientConfiguration.DFLT_PORT, "Failed to parse Ignite server address"));
}
- return ranges.stream()
- .map(p -> InetSocketAddress.createUnresolved(p.host(), p.port()))
- .collect(Collectors.toMap(a -> a, a -> 1, Integer::sum));
+ var map = new HashMap<InetSocketAddress, Integer>(parsedAddrs.size());
Review Comment:
org.apache.ignite.internal.util.IgniteUtils#newHashMap need to be used if you want to predict the sizing
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-26893 Java thin: improve DNS resolution logic [ignite-3]
Posted by "nva (via GitHub)" <gi...@apache.org>.
nva commented on code in PR #7031:
URL: https://github.com/apache/ignite-3/pull/7031#discussion_r2588816633
##########
modules/client/src/main/java/org/apache/ignite/internal/client/ReliableChannel.java:
##########
@@ -773,7 +800,14 @@ private void onObservableTimestampReceived(long newTs) {
}
private void onPartitionAssignmentChanged(long timestamp) {
- partitionAssignmentTimestamp.updateAndGet(curTs -> Math.max(curTs, timestamp));
+ var old = partitionAssignmentTimestamp.getAndUpdate(curTs -> Math.max(curTs, timestamp));
+
+ if (timestamp > old) {
+ // New assignment timestamp, topology change possible.
+ if (scheduledChannelsReinit.compareAndSet(false, true)) {
+ initChannelHolders();
Review Comment:
Added a separate executor
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org