You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by GitBox <gi...@apache.org> on 2022/06/26 13:39:06 UTC

[GitHub] [ignite] wkhappy1 opened a new pull request, #10110: when pod restart, ip was changed,so channel also need refresh

wkhappy1 opened a new pull request, #10110:
URL: https://github.com/apache/ignite/pull/10110

   Thank you for submitting the pull request to the Apache Ignite.
   
   In order to streamline the review of the contribution 
   we ask you to ensure the following steps have been taken:
   
   ### The Contribution Checklist
   - [ ] 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 pull request body describes changes that have been made. 
   The description explains _WHAT_ and _WHY_ was made instead of _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.
   - [ ] A reviewer has been mentioned through the JIRA comments 
   (see [the Maintainers list](https://cwiki.apache.org/confluence/display/IGNITE/How+to+Contribute#HowtoContribute-ReviewProcessandMaintainers)) 
   - [ ] The pull request has been checked by the Teamcity Bot and 
   the `green visa` attached to the JIRA ticket (see [TC.Bot: Check PR](https://mtcga.gridgain.com/prs.html))
   
   ### Notes
   - [How to Contribute](https://cwiki.apache.org/confluence/display/IGNITE/How+to+Contribute)
   - [Coding abbreviation rules](https://cwiki.apache.org/confluence/display/IGNITE/Abbreviation+Rules)
   - [Coding Guidelines](https://cwiki.apache.org/confluence/display/IGNITE/Coding+Guidelines)
   - [Apache Ignite Teamcity Bot](https://cwiki.apache.org/confluence/display/IGNITE/Apache+Ignite+Teamcity+Bot)
   
   If you need any help, please email dev@ignite.apache.org or ask anу advice on http://asf.slack.com _#ignite_ channel.
   


-- 
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


[GitHub] [ignite] ptupitsyn commented on a diff in pull request #10110: IGNITE-17217 Java thin: reload addresses from ClientAddressFinder on connection loss

Posted by GitBox <gi...@apache.org>.
ptupitsyn commented on code in PR #10110:
URL: https://github.com/apache/ignite/pull/10110#discussion_r908191759


##########
modules/kubernetes/src/test/java/org/apache/ignite/kubernetes/discovery/TestClusterClientConnection.java:
##########
@@ -49,4 +49,44 @@ public void testClientConnectsToCluster() throws Exception {
         cache.put(1, 2);
         assertEquals(2, cache.get(1));
     }
+
+    @Test
+    public void testClientReConnectsToClusterAfterPodIpChange() throws Exception {
+        mockServerResponse();
+
+        IgniteConfiguration cfg = getConfiguration(getTestIgniteInstanceName(), false);
+
+        IgniteEx crd = startGrid(cfg);
+        String crdAddr = crd.localNode().addresses().iterator().next();
+
+        mockServerResponse(crdAddr);
+
+        ClientConfiguration ccfg = new ClientConfiguration();
+        ccfg.setAddressesFinder(new ThinClientKubernetesAddressFinder(prepareConfiguration()));
+        IgniteClient client = Ignition.startClient(ccfg);
+
+        ClientCache cache = client.createCache("cache");
+        cache.put(1, 2);
+        assertEquals(2, cache.get(1));
+
+        //stop node and change port still can connect
+        Ignition.stop(crd.name(), true);
+        int newPort = 10801;
+        ccfg.setAddressesFinder(new ThinClientKubernetesAddressFinder(prepareConfiguration(),newPort));
+        mockServerResponse(5,crdAddr);
+
+        cfg = getConfiguration(getTestIgniteInstanceName(), false);
+        cfg.setSqlConnectorConfiguration(new SqlConnectorConfiguration().setPort(newPort));

Review Comment:
   Not sure I understand the problem. Replacing `setSqlConnectorConfiguration` with `setClientConnectorConfiguration` seems to work, the test passes. I've pushed the change to you branch, please review.



-- 
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


[GitHub] [ignite] wkhappy1 commented on a diff in pull request #10110: IGNITE-17217 Java thin: reload addresses from ClientAddressFinder on connection loss

Posted by GitBox <gi...@apache.org>.
wkhappy1 commented on code in PR #10110:
URL: https://github.com/apache/ignite/pull/10110#discussion_r908207166


##########
modules/kubernetes/src/test/java/org/apache/ignite/kubernetes/discovery/TestClusterClientConnection.java:
##########
@@ -49,4 +49,44 @@ public void testClientConnectsToCluster() throws Exception {
         cache.put(1, 2);
         assertEquals(2, cache.get(1));
     }
+
+    @Test
+    public void testClientReConnectsToClusterAfterPodIpChange() throws Exception {
+        mockServerResponse();
+
+        IgniteConfiguration cfg = getConfiguration(getTestIgniteInstanceName(), false);
+
+        IgniteEx crd = startGrid(cfg);
+        String crdAddr = crd.localNode().addresses().iterator().next();
+
+        mockServerResponse(crdAddr);
+
+        ClientConfiguration ccfg = new ClientConfiguration();
+        ccfg.setAddressesFinder(new ThinClientKubernetesAddressFinder(prepareConfiguration()));
+        IgniteClient client = Ignition.startClient(ccfg);
+
+        ClientCache cache = client.createCache("cache");
+        cache.put(1, 2);
+        assertEquals(2, cache.get(1));
+
+        //stop node and change port still can connect
+        Ignition.stop(crd.name(), true);
+        int newPort = 10801;
+        ccfg.setAddressesFinder(new ThinClientKubernetesAddressFinder(prepareConfiguration(),newPort));
+        mockServerResponse(5,crdAddr);
+
+        cfg = getConfiguration(getTestIgniteInstanceName(), false);
+        cfg.setSqlConnectorConfiguration(new SqlConnectorConfiguration().setPort(newPort));

Review Comment:
   > Not sure I understand the problem. Replacing `setSqlConnectorConfiguration` with `setClientConnectorConfiguration` seems to work, the test passes. I've pushed the change to you branch, please review.
   thank you reply。i debug cfg.getClientConnectorConfiguration() ,and type is ClientConnectorConfigurationEx,and i use  cfg.getClientConnectorConfiguration().setPort() so it not success。
   thank you
   



-- 
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


[GitHub] [ignite] wkhappy1 commented on a diff in pull request #10110: IGNITE-17217 Java thin: reload addresses from ClientAddressFinder on connection loss

Posted by GitBox <gi...@apache.org>.
wkhappy1 commented on code in PR #10110:
URL: https://github.com/apache/ignite/pull/10110#discussion_r908010521


##########
modules/kubernetes/src/test/java/org/apache/ignite/kubernetes/discovery/TestClusterClientConnection.java:
##########
@@ -49,4 +49,44 @@ public void testClientConnectsToCluster() throws Exception {
         cache.put(1, 2);
         assertEquals(2, cache.get(1));
     }
+
+    @Test
+    public void testClientReConnectsToClusterAfterPodIpChange() throws Exception {
+        mockServerResponse();
+
+        IgniteConfiguration cfg = getConfiguration(getTestIgniteInstanceName(), false);
+
+        IgniteEx crd = startGrid(cfg);
+        String crdAddr = crd.localNode().addresses().iterator().next();
+
+        mockServerResponse(crdAddr);
+
+        ClientConfiguration ccfg = new ClientConfiguration();
+        ccfg.setAddressesFinder(new ThinClientKubernetesAddressFinder(prepareConfiguration()));
+        IgniteClient client = Ignition.startClient(ccfg);
+
+        ClientCache cache = client.createCache("cache");
+        cache.put(1, 2);
+        assertEquals(2, cache.get(1));
+
+        //stop node and change port still can connect
+        Ignition.stop(crd.name(), true);
+        int newPort = 10801;
+        ccfg.setAddressesFinder(new ThinClientKubernetesAddressFinder(prepareConfiguration(),newPort));
+        mockServerResponse(5,crdAddr);
+
+        cfg = getConfiguration(getTestIgniteInstanceName(), false);
+        cfg.setSqlConnectorConfiguration(new SqlConnectorConfiguration().setPort(newPort));

Review Comment:
   > `setSqlConnectorConfiguration` is deprecated, please use `setClientConnectorConfiguration` instead.
   i'm sorry. when i use setClientConnectorConfiguration  to set a newPort then startGrid(cfg), i found ClientListenerProcessor method isNotDefault(cliConnCfg)  will check ClientConnectorConfiguration isDefault then it will return false,then   cliConnCfg = new ClientConnectorConfiguration();
      so i set newPort will not take effect .then i use SqlConnectorConfiguration to set newPort. I don't know other method to set a newPort
   



-- 
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


[GitHub] [ignite] wkhappy1 commented on a diff in pull request #10110: IGNITE-17217 Java thin: reload addresses from ClientAddressFinder on connection loss

Posted by GitBox <gi...@apache.org>.
wkhappy1 commented on code in PR #10110:
URL: https://github.com/apache/ignite/pull/10110#discussion_r907987050


##########
modules/kubernetes/src/test/java/org/apache/ignite/kubernetes/discovery/TestClusterClientConnection.java:
##########
@@ -49,4 +49,44 @@ public void testClientConnectsToCluster() throws Exception {
         cache.put(1, 2);
         assertEquals(2, cache.get(1));
     }
+
+    @Test
+    public void testClientReConnectsToClusterAfterPodIpChange() throws Exception {
+        mockServerResponse();
+
+        IgniteConfiguration cfg = getConfiguration(getTestIgniteInstanceName(), false);
+
+        IgniteEx crd = startGrid(cfg);
+        String crdAddr = crd.localNode().addresses().iterator().next();
+
+        mockServerResponse(crdAddr);
+
+        ClientConfiguration ccfg = new ClientConfiguration();
+        ccfg.setAddressesFinder(new ThinClientKubernetesAddressFinder(prepareConfiguration()));
+        IgniteClient client = Ignition.startClient(ccfg);
+
+        ClientCache cache = client.createCache("cache");
+        cache.put(1, 2);
+        assertEquals(2, cache.get(1));
+
+        //stop node and change port still can connect
+        Ignition.stop(crd.name(), true);
+        int newPort = 10801;
+        ccfg.setAddressesFinder(new ThinClientKubernetesAddressFinder(prepareConfiguration(),newPort));
+        mockServerResponse(5,crdAddr);
+
+        cfg = getConfiguration(getTestIgniteInstanceName(), false);
+        cfg.setSqlConnectorConfiguration(new SqlConnectorConfiguration().setPort(newPort));

Review Comment:
   > `setSqlConnectorConfiguration` is deprecated, please use `setClientConnectorConfiguration` instead.
   
   i found ClientListenerProcessor method isNotDefault(cliConnCfg)  will check ClientConnectorConfiguration isDefault then it will return false,then   cliConnCfg = new ClientConnectorConfiguration();
   so i set newPort will not take effect .then i use SqlConnectorConfiguration to set newPort. I don't know other method to set a newPort



-- 
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


[GitHub] [ignite] wkhappy1 commented on a diff in pull request #10110: IGNITE-17217 Java thin: reload addresses from ClientAddressFinder on connection loss

Posted by GitBox <gi...@apache.org>.
wkhappy1 commented on code in PR #10110:
URL: https://github.com/apache/ignite/pull/10110#discussion_r908010521


##########
modules/kubernetes/src/test/java/org/apache/ignite/kubernetes/discovery/TestClusterClientConnection.java:
##########
@@ -49,4 +49,44 @@ public void testClientConnectsToCluster() throws Exception {
         cache.put(1, 2);
         assertEquals(2, cache.get(1));
     }
+
+    @Test
+    public void testClientReConnectsToClusterAfterPodIpChange() throws Exception {
+        mockServerResponse();
+
+        IgniteConfiguration cfg = getConfiguration(getTestIgniteInstanceName(), false);
+
+        IgniteEx crd = startGrid(cfg);
+        String crdAddr = crd.localNode().addresses().iterator().next();
+
+        mockServerResponse(crdAddr);
+
+        ClientConfiguration ccfg = new ClientConfiguration();
+        ccfg.setAddressesFinder(new ThinClientKubernetesAddressFinder(prepareConfiguration()));
+        IgniteClient client = Ignition.startClient(ccfg);
+
+        ClientCache cache = client.createCache("cache");
+        cache.put(1, 2);
+        assertEquals(2, cache.get(1));
+
+        //stop node and change port still can connect
+        Ignition.stop(crd.name(), true);
+        int newPort = 10801;
+        ccfg.setAddressesFinder(new ThinClientKubernetesAddressFinder(prepareConfiguration(),newPort));
+        mockServerResponse(5,crdAddr);
+
+        cfg = getConfiguration(getTestIgniteInstanceName(), false);
+        cfg.setSqlConnectorConfiguration(new SqlConnectorConfiguration().setPort(newPort));

Review Comment:
   > `setSqlConnectorConfiguration` is deprecated, please use `setClientConnectorConfiguration` instead.
   
   i'm sorry. when i use setClientConnectorConfiguration  to set a newPort then startGrid(cfg), i found ClientListenerProcessor method isNotDefault(cliConnCfg)  will check ClientConnectorConfiguration isDefault then it will return false,then   cliConnCfg = new ClientConnectorConfiguration();
      so i set newPort will not take effect .then i use SqlConnectorConfiguration to set newPort. I don't know other method to set a newPort
   



-- 
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


[GitHub] [ignite] ptupitsyn commented on pull request #10110: when pod restart, ip was changed,so channel also need refresh

Posted by GitBox <gi...@apache.org>.
ptupitsyn commented on PR #10110:
URL: https://github.com/apache/ignite/pull/10110#issuecomment-1166962054

   Please make sure that the code compiles (all Travis CI checks pass). Right now there is a missing import in `TestClusterClientConnection`.


-- 
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


[GitHub] [ignite] ptupitsyn commented on a diff in pull request #10110: when pod restart, ip was changed,so channel also need refresh

Posted by GitBox <gi...@apache.org>.
ptupitsyn commented on code in PR #10110:
URL: https://github.com/apache/ignite/pull/10110#discussion_r907038662


##########
modules/core/src/main/java/org/apache/ignite/internal/client/thin/ReliableChannel.java:
##########
@@ -527,10 +527,23 @@ private void onChannelFailure(ClientChannelHolder hld, ClientChannel ch) {
         rollCurrentChannel(hld);
 
         // For partiton awareness it's already initializing asynchronously in #onTopologyChanged.
-        if (scheduledChannelsReinit.get() && !partitionAwarenessEnabled)
+        if (addressChanged() || (scheduledChannelsReinit.get() && !partitionAwarenessEnabled))
             channelsInit();
     }
 
+    /**
+     * check ip address whether changed,after pod restart
+     */
+    private boolean addressChanged(){

Review Comment:
   ```suggestion
       private boolean addressFinderAddressesChanged() {
   ```



##########
modules/kubernetes/src/test/java/org/apache/ignite/kubernetes/discovery/TestClusterClientConnection.java:
##########
@@ -49,4 +49,44 @@ public void testClientConnectsToCluster() throws Exception {
         cache.put(1, 2);
         assertEquals(2, cache.get(1));
     }
+
+    @Test
+    public void testClientReConnectsToClusterAfterPodIpChange() throws Exception {
+        mockServerResponse();
+
+        IgniteConfiguration cfg = getConfiguration(getTestIgniteInstanceName(), false);
+
+        IgniteEx crd = startGrid(cfg);
+        String crdAddr = crd.localNode().addresses().iterator().next();
+
+        mockServerResponse(crdAddr);
+
+        ClientConfiguration ccfg = new ClientConfiguration();
+        ccfg.setAddressesFinder(new ThinClientKubernetesAddressFinder(prepareConfiguration()));
+        IgniteClient client = Ignition.startClient(ccfg);
+
+        ClientCache cache = client.createCache("cache");
+        cache.put(1, 2);
+        assertEquals(2, cache.get(1));
+
+        //stop node and change port still can connect
+        Ignition.stop(crd.name(), true);
+        int newPort = 10801;
+        ccfg.setAddressesFinder(new ThinClientKubernetesAddressFinder(prepareConfiguration(),newPort));
+        mockServerResponse(5,crdAddr);
+
+        cfg = getConfiguration(getTestIgniteInstanceName(), false);
+        cfg.setSqlConnectorConfiguration(new SqlConnectorConfiguration().setPort(newPort));

Review Comment:
   `setSqlConnectorConfiguration` is deprecated, please use `setClientConnectorConfiguration` instead.



##########
modules/core/src/main/java/org/apache/ignite/internal/client/thin/ReliableChannel.java:
##########
@@ -527,10 +527,23 @@ private void onChannelFailure(ClientChannelHolder hld, ClientChannel ch) {
         rollCurrentChannel(hld);
 
         // For partiton awareness it's already initializing asynchronously in #onTopologyChanged.
-        if (scheduledChannelsReinit.get() && !partitionAwarenessEnabled)
+        if (addressChanged() || (scheduledChannelsReinit.get() && !partitionAwarenessEnabled))
             channelsInit();
     }
 
+    /**
+     * check ip address whether changed,after pod restart
+     */
+    private boolean addressChanged(){
+        if (clientCfg.getAddressesFinder() != null) {
+            String[] hostAddrs = clientCfg.getAddressesFinder().getAddresses();
+            if(!Arrays.equals(hostAddrs, prevHostAddrs)){
+                return true;
+            }

Review Comment:
   ```suggestion
               return !Arrays.equals(hostAddrs, prevHostAddrs);
   ```



-- 
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


[GitHub] [ignite] ptupitsyn merged pull request #10110: IGNITE-17217 Java thin: reload addresses from ClientAddressFinder on connection loss

Posted by GitBox <gi...@apache.org>.
ptupitsyn merged PR #10110:
URL: https://github.com/apache/ignite/pull/10110


-- 
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


[GitHub] [ignite] wkhappy1 commented on a diff in pull request #10110: IGNITE-17217 Java thin: reload addresses from ClientAddressFinder on connection loss

Posted by GitBox <gi...@apache.org>.
wkhappy1 commented on code in PR #10110:
URL: https://github.com/apache/ignite/pull/10110#discussion_r907987050


##########
modules/kubernetes/src/test/java/org/apache/ignite/kubernetes/discovery/TestClusterClientConnection.java:
##########
@@ -49,4 +49,44 @@ public void testClientConnectsToCluster() throws Exception {
         cache.put(1, 2);
         assertEquals(2, cache.get(1));
     }
+
+    @Test
+    public void testClientReConnectsToClusterAfterPodIpChange() throws Exception {
+        mockServerResponse();
+
+        IgniteConfiguration cfg = getConfiguration(getTestIgniteInstanceName(), false);
+
+        IgniteEx crd = startGrid(cfg);
+        String crdAddr = crd.localNode().addresses().iterator().next();
+
+        mockServerResponse(crdAddr);
+
+        ClientConfiguration ccfg = new ClientConfiguration();
+        ccfg.setAddressesFinder(new ThinClientKubernetesAddressFinder(prepareConfiguration()));
+        IgniteClient client = Ignition.startClient(ccfg);
+
+        ClientCache cache = client.createCache("cache");
+        cache.put(1, 2);
+        assertEquals(2, cache.get(1));
+
+        //stop node and change port still can connect
+        Ignition.stop(crd.name(), true);
+        int newPort = 10801;
+        ccfg.setAddressesFinder(new ThinClientKubernetesAddressFinder(prepareConfiguration(),newPort));
+        mockServerResponse(5,crdAddr);
+
+        cfg = getConfiguration(getTestIgniteInstanceName(), false);
+        cfg.setSqlConnectorConfiguration(new SqlConnectorConfiguration().setPort(newPort));

Review Comment:
   > `setSqlConnectorConfiguration` is deprecated, please use `setClientConnectorConfiguration` instead.
   
   when i use setClientConnectorConfiguration  to set a newPort, i found ClientListenerProcessor method isNotDefault(cliConnCfg)  will check ClientConnectorConfiguration isDefault then it will return false,then   cliConnCfg = new ClientConnectorConfiguration();
   so i set newPort will not take effect .then i use SqlConnectorConfiguration to set newPort. I don't know other method to set a newPort



-- 
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


[GitHub] [ignite] wkhappy1 commented on a diff in pull request #10110: IGNITE-17217 Java thin: reload addresses from ClientAddressFinder on connection loss

Posted by GitBox <gi...@apache.org>.
wkhappy1 commented on code in PR #10110:
URL: https://github.com/apache/ignite/pull/10110#discussion_r908207166


##########
modules/kubernetes/src/test/java/org/apache/ignite/kubernetes/discovery/TestClusterClientConnection.java:
##########
@@ -49,4 +49,44 @@ public void testClientConnectsToCluster() throws Exception {
         cache.put(1, 2);
         assertEquals(2, cache.get(1));
     }
+
+    @Test
+    public void testClientReConnectsToClusterAfterPodIpChange() throws Exception {
+        mockServerResponse();
+
+        IgniteConfiguration cfg = getConfiguration(getTestIgniteInstanceName(), false);
+
+        IgniteEx crd = startGrid(cfg);
+        String crdAddr = crd.localNode().addresses().iterator().next();
+
+        mockServerResponse(crdAddr);
+
+        ClientConfiguration ccfg = new ClientConfiguration();
+        ccfg.setAddressesFinder(new ThinClientKubernetesAddressFinder(prepareConfiguration()));
+        IgniteClient client = Ignition.startClient(ccfg);
+
+        ClientCache cache = client.createCache("cache");
+        cache.put(1, 2);
+        assertEquals(2, cache.get(1));
+
+        //stop node and change port still can connect
+        Ignition.stop(crd.name(), true);
+        int newPort = 10801;
+        ccfg.setAddressesFinder(new ThinClientKubernetesAddressFinder(prepareConfiguration(),newPort));
+        mockServerResponse(5,crdAddr);
+
+        cfg = getConfiguration(getTestIgniteInstanceName(), false);
+        cfg.setSqlConnectorConfiguration(new SqlConnectorConfiguration().setPort(newPort));

Review Comment:
   > Not sure I understand the problem. Replacing `setSqlConnectorConfiguration` with `setClientConnectorConfiguration` seems to work, the test passes. I've pushed the change to you branch, please review.
   
   thank you reply。i debug cfg.getClientConnectorConfiguration() ,and type is ClientConnectorConfigurationEx,and i use  cfg.getClientConnectorConfiguration().setPort() so it not success。
   thank you
   



-- 
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


[GitHub] [ignite] ptupitsyn commented on a diff in pull request #10110: when pod restart, ip was changed,so channel also need refresh

Posted by GitBox <gi...@apache.org>.
ptupitsyn commented on code in PR #10110:
URL: https://github.com/apache/ignite/pull/10110#discussion_r907038429


##########
modules/core/src/main/java/org/apache/ignite/internal/client/thin/ReliableChannel.java:
##########
@@ -527,10 +527,23 @@ private void onChannelFailure(ClientChannelHolder hld, ClientChannel ch) {
         rollCurrentChannel(hld);
 
         // For partiton awareness it's already initializing asynchronously in #onTopologyChanged.
-        if (scheduledChannelsReinit.get() && !partitionAwarenessEnabled)
+        if (addressChanged() || (scheduledChannelsReinit.get() && !partitionAwarenessEnabled))
             channelsInit();
     }
 
+    /**
+     * check ip address whether changed,after pod restart
+     */
+    private boolean addressChanged(){

Review Comment:
   ```suggestion
       private boolean addressFindeAddressesChanged() {
   ```



-- 
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


[GitHub] [ignite] wkhappy1 commented on a diff in pull request #10110: IGNITE-17217 Java thin: reload addresses from ClientAddressFinder on connection loss

Posted by GitBox <gi...@apache.org>.
wkhappy1 commented on code in PR #10110:
URL: https://github.com/apache/ignite/pull/10110#discussion_r907987050


##########
modules/kubernetes/src/test/java/org/apache/ignite/kubernetes/discovery/TestClusterClientConnection.java:
##########
@@ -49,4 +49,44 @@ public void testClientConnectsToCluster() throws Exception {
         cache.put(1, 2);
         assertEquals(2, cache.get(1));
     }
+
+    @Test
+    public void testClientReConnectsToClusterAfterPodIpChange() throws Exception {
+        mockServerResponse();
+
+        IgniteConfiguration cfg = getConfiguration(getTestIgniteInstanceName(), false);
+
+        IgniteEx crd = startGrid(cfg);
+        String crdAddr = crd.localNode().addresses().iterator().next();
+
+        mockServerResponse(crdAddr);
+
+        ClientConfiguration ccfg = new ClientConfiguration();
+        ccfg.setAddressesFinder(new ThinClientKubernetesAddressFinder(prepareConfiguration()));
+        IgniteClient client = Ignition.startClient(ccfg);
+
+        ClientCache cache = client.createCache("cache");
+        cache.put(1, 2);
+        assertEquals(2, cache.get(1));
+
+        //stop node and change port still can connect
+        Ignition.stop(crd.name(), true);
+        int newPort = 10801;
+        ccfg.setAddressesFinder(new ThinClientKubernetesAddressFinder(prepareConfiguration(),newPort));
+        mockServerResponse(5,crdAddr);
+
+        cfg = getConfiguration(getTestIgniteInstanceName(), false);
+        cfg.setSqlConnectorConfiguration(new SqlConnectorConfiguration().setPort(newPort));

Review Comment:
   > `setSqlConnectorConfiguration` is deprecated, please use `setClientConnectorConfiguration` instead.
   
   when i use setClientConnectorConfiguration  to set a newPort then startGrid(cfg), i found ClientListenerProcessor method isNotDefault(cliConnCfg)  will check ClientConnectorConfiguration isDefault then it will return false,then   cliConnCfg = new ClientConnectorConfiguration();
   so i set newPort will not take effect .then i use SqlConnectorConfiguration to set newPort. I don't know other method to set a newPort



-- 
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


[GitHub] [ignite] wkhappy1 commented on a diff in pull request #10110: IGNITE-17217 Java thin: reload addresses from ClientAddressFinder on connection loss

Posted by GitBox <gi...@apache.org>.
wkhappy1 commented on code in PR #10110:
URL: https://github.com/apache/ignite/pull/10110#discussion_r907987050


##########
modules/kubernetes/src/test/java/org/apache/ignite/kubernetes/discovery/TestClusterClientConnection.java:
##########
@@ -49,4 +49,44 @@ public void testClientConnectsToCluster() throws Exception {
         cache.put(1, 2);
         assertEquals(2, cache.get(1));
     }
+
+    @Test
+    public void testClientReConnectsToClusterAfterPodIpChange() throws Exception {
+        mockServerResponse();
+
+        IgniteConfiguration cfg = getConfiguration(getTestIgniteInstanceName(), false);
+
+        IgniteEx crd = startGrid(cfg);
+        String crdAddr = crd.localNode().addresses().iterator().next();
+
+        mockServerResponse(crdAddr);
+
+        ClientConfiguration ccfg = new ClientConfiguration();
+        ccfg.setAddressesFinder(new ThinClientKubernetesAddressFinder(prepareConfiguration()));
+        IgniteClient client = Ignition.startClient(ccfg);
+
+        ClientCache cache = client.createCache("cache");
+        cache.put(1, 2);
+        assertEquals(2, cache.get(1));
+
+        //stop node and change port still can connect
+        Ignition.stop(crd.name(), true);
+        int newPort = 10801;
+        ccfg.setAddressesFinder(new ThinClientKubernetesAddressFinder(prepareConfiguration(),newPort));
+        mockServerResponse(5,crdAddr);
+
+        cfg = getConfiguration(getTestIgniteInstanceName(), false);
+        cfg.setSqlConnectorConfiguration(new SqlConnectorConfiguration().setPort(newPort));

Review Comment:
   > `setSqlConnectorConfiguration` is deprecated, please use `setClientConnectorConfiguration` instead.
   
   when i use setClientConnectorConfiguration  to set a newPort then startGrid(cfg), i found ClientListenerProcessor method isNotDefault(cliConnCfg)  will check ClientConnectorConfiguration isDefault then it will return false,then   cliConnCfg = new ClientConnectorConfiguration();
   so i set newPort will not take effect .then i use SqlConnectorConfiguration to set newPort. I don't know other method to set a newPort



-- 
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