timoninmaxim commented on code in PR #11855:
URL: https://github.com/apache/ignite/pull/11855#discussion_r1966502279
##########
modules/core/src/main/java/org/apache/ignite/internal/client/thin/ReliableChannel.java:
##########
@@ -656,14 +657,16 @@ synchronized void initChannelHolders() {
for (InetSocketAddress addr : h.getAddresses()) {
// If new endpoints contain at least one of channel
addresses, don't close this channel.
if (newAddrsSet.contains(addr)) {
- ClientChannelHolder oldHld =
curAddrs.putIfAbsent(addr, h);
-
- if (oldHld == null || oldHld == h) // If not duplicate.
- found = true;
+ found = true;
+ break;
}
}
- if (!found)
+ if (found) {
+ for (InetSocketAddress addr : h.getAddresses())
+ curAddrs.putIfAbsent(addr, h);
Review Comment:
Looks weird, you already put all this holders earlier, see line 644
##########
modules/core/src/main/java/org/apache/ignite/internal/client/thin/ReliableChannel.java:
##########
@@ -678,29 +681,25 @@ synchronized void initChannelHolders() {
int idx = curChIdx;
- if (idx != -1)
+ if (idx != -1 && holders != null)
currDfltHolder = holders.get(idx);
- for (List<InetSocketAddress> addrs : newAddrs) {
- ClientChannelHolder hld = null;
+ for (InetSocketAddress addr : newAddrsSet) {
+ ClientChannelHolder hld = curAddrs.get(addr);
- // Try to find already created channel holder.
- for (InetSocketAddress addr : addrs) {
- hld = curAddrs.get(addr);
+ if (hld == null) {
+ List<InetSocketAddress> addrList = new ArrayList<>();
- if (hld != null) {
- if (!hld.getAddresses().equals(addrs)) // Enrich holder
addresses.
- hld.setConfiguration(new
ClientChannelConfiguration(clientCfg, addrs));
+ addrList.add(addr);
- break;
- }
+ hld = new ClientChannelHolder(new
ClientChannelConfiguration(clientCfg, addrList));
}
+ else if (!hld.getAddresses().contains(addr)) {
+ List<InetSocketAddress> newAddrList = new
ArrayList<>(hld.getAddresses());
Review Comment:
looks like already stored list can be reused.
##########
modules/core/src/main/java/org/apache/ignite/internal/client/thin/ReliableChannel.java:
##########
@@ -631,22 +631,23 @@ synchronized void initChannelHolders() {
return;
}
+ Map<InetSocketAddress, ClientChannelHolder> curAddrs = new HashMap<>();
+
+ Set<InetSocketAddress> newAddrsSet =
newAddrs.stream().flatMap(Collection::stream).collect(Collectors.toSet());
Review Comment:
Looks like it duplicates `curAddrs.keySet()`.
##########
modules/core/src/main/java/org/apache/ignite/internal/client/thin/ReliableChannel.java:
##########
@@ -944,6 +943,10 @@ private int getRetryLimit() {
int size = holders.size();
+ // Essential to produce a retry connection on the channel after a
failure occurred on that single channel.
+ if (channelsCnt.get() == 0 && partitionAwarenessEnabled)
Review Comment:
Looks like you don't need this change, we want fix duplication first in this
patch.
--
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-unsubscr...@ignite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
