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 2020/03/11 17:08:59 UTC

[GitHub] [ignite] Vladsz83 opened a new pull request #7514: Ignite 12773

Vladsz83 opened a new pull request #7514: Ignite 12773
URL: https://github.com/apache/ignite/pull/7514
 
 
   IGNITE-12773 : Reduce number of cluster deactivation methods in internal API.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [ignite] NSAmelchev commented on a change in pull request #7514: IGNITE-12773 : Reduce number of cluster deactivation methods in internal API.

Posted by GitBox <gi...@apache.org>.
NSAmelchev commented on a change in pull request #7514: IGNITE-12773 : Reduce number of cluster deactivation methods in internal API.
URL: https://github.com/apache/ignite/pull/7514#discussion_r391596634
 
 

 ##########
 File path: modules/core/src/main/java/org/apache/ignite/internal/cluster/IgniteClusterImpl.java
 ##########
 @@ -313,7 +315,7 @@ public IgniteClusterImpl(GridKernalContext ctx) {
         guard();
 
         try {
-            ctx.state().changeGlobalState(active, serverNodes(), false).get();
+            ctx.state().changeGlobalState(active ? ACTIVE : INACTIVE, true, serverNodes(), false, false).get();
 
 Review comment:
   Why `forceDeactivation` should be `true` for cluster activate?

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [ignite] Vladsz83 commented on a change in pull request #7514: IGNITE-12773 : Reduce number of cluster deactivation methods in internal API.

Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on a change in pull request #7514: IGNITE-12773 : Reduce number of cluster deactivation methods in internal API.
URL: https://github.com/apache/ignite/pull/7514#discussion_r391668470
 
 

 ##########
 File path: modules/clients/src/test/java/org/apache/ignite/internal/processors/rest/ChangeStateCommandHandlerTest.java
 ##########
 @@ -87,23 +89,23 @@
     public void testActivateDeActivate() throws GridClientException {
         GridClientClusterState state = client.state();
 
-        boolean active = state.active();
+        boolean active = state.state() == ACTIVE;
 
         assertTrue(active);
 
-        state.active(false);
+        state.state(INACTIVE, true);
 
         IgniteEx ig1 = grid(0);
         IgniteEx ig2 = grid(1);
 
         assertFalse(ig1.active());
         assertFalse(ig2.active());
-        assertFalse(state.active());
+        assertEquals(state.state(), INACTIVE);
 
 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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [ignite] NSAmelchev commented on a change in pull request #7514: IGNITE-12773 : Reduce number of cluster deactivation methods in internal API.

Posted by GitBox <gi...@apache.org>.
NSAmelchev commented on a change in pull request #7514: IGNITE-12773 : Reduce number of cluster deactivation methods in internal API.
URL: https://github.com/apache/ignite/pull/7514#discussion_r391671294
 
 

 ##########
 File path: modules/clients/src/test/java/org/apache/ignite/internal/processors/rest/ChangeStateCommandHandlerTest.java
 ##########
 @@ -87,23 +89,23 @@
     public void testActivateDeActivate() throws GridClientException {
         GridClientClusterState state = client.state();
 
-        boolean active = state.active();
+        boolean active = state.state() == ACTIVE;
 
 Review comment:
   `ClusterState.active(ClusterState state)` was used.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [ignite] NSAmelchev commented on a change in pull request #7514: IGNITE-12773 : Reduce number of cluster deactivation methods in internal API.

Posted by GitBox <gi...@apache.org>.
NSAmelchev commented on a change in pull request #7514: IGNITE-12773 : Reduce number of cluster deactivation methods in internal API.
URL: https://github.com/apache/ignite/pull/7514#discussion_r391594754
 
 

 ##########
 File path: modules/core/src/main/java/org/apache/ignite/internal/client/impl/GridClientClusterStateImpl.java
 ##########
 @@ -54,16 +53,6 @@ public GridClientClusterStateImpl(
         super(client, nodes, filter, balancer);
     }
 
-    /** {@inheritDoc} */
-    @Override public void active(final boolean active) throws GridClientException {
-        state(active ? ACTIVE : INACTIVE, true);
-    }
-
-    /** {@inheritDoc} */
-    @Override public boolean active() throws GridClientException {
-        return withReconnectHandling(GridClientConnection::currentState).get();
 
 Review comment:
   `GridClientConnection::currentState` now unused. Seems can be removed too

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [ignite] NSAmelchev commented on a change in pull request #7514: IGNITE-12773 : Reduce number of cluster deactivation methods in internal API.

Posted by GitBox <gi...@apache.org>.
NSAmelchev commented on a change in pull request #7514: IGNITE-12773 : Reduce number of cluster deactivation methods in internal API.
URL: https://github.com/apache/ignite/pull/7514#discussion_r391665433
 
 

 ##########
 File path: modules/clients/src/test/java/org/apache/ignite/internal/processors/rest/ChangeStateCommandHandlerTest.java
 ##########
 @@ -87,23 +89,23 @@
     public void testActivateDeActivate() throws GridClientException {
         GridClientClusterState state = client.state();
 
-        boolean active = state.active();
+        boolean active = state.state() == ACTIVE;
 
         assertTrue(active);
 
-        state.active(false);
+        state.state(INACTIVE, true);
 
         IgniteEx ig1 = grid(0);
         IgniteEx ig2 = grid(1);
 
         assertFalse(ig1.active());
         assertFalse(ig2.active());
-        assertFalse(state.active());
+        assertEquals(state.state(), INACTIVE);
 
 Review comment:
   Mixed up expected-actual

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [ignite] Vladsz83 edited a comment on issue #7514: Ignite 12773

Posted by GitBox <gi...@apache.org>.
Vladsz83 edited a comment on issue #7514: Ignite 12773
URL: https://github.com/apache/ignite/pull/7514#issuecomment-597755820
 
 
   Major code changes:
   
   1.	Removed `GridClientClusterState#active()`
   
   2.	Removed `GridClientClusterState#active(boolean active)`
   
   3.	Removed
   `IGridClusterStateProcessor#changeGlobalState(
       boolean activate,
       Collection<? extends BaselineNode> baselineNodes,
       boolean forceChangeBaselineTopology
   )`
   
   4.	Removed
   `GridClusterStateProcessor#changeGlobalState(
       final boolean activate,
       Collection<? extends BaselineNode> baselineNodes,
       boolean forceChangeBaselineTopology,
       boolean isAutoAdjust
   )`
   
   5.	Removed
   `GridClusterStateProcessor#changeGlobalState(
       final boolean activate,
       Collection<? extends BaselineNode> baselineNodes,
       boolean forceChangeBaselineTopology
   )`
   
   6.	Remove d
   `GridClusterStateProcessor#changeGlobalState(
       ClusterState state,
       Collection<? extends BaselineNode> baselineNodes,
       boolean forceChangeBaselineTopology
   )`
   
   7.	Added boolean isAutoAdjust to 
   `IGridClusterStateProcessor#changeGlobalState(
       ClusterState state,
       Collection<? extends BaselineNode> baselineNodes,
       boolean forceChangeBaselineTopology,
      /* here */ boolean isAutoAdjust /* here */
   )`
   
   8.	Added @Override to 
   @Override
   `IGridClusterStateProcessor#changeGlobalState(
           ClusterState state,
           Collection<? extends BaselineNode> baselineNodes,
           boolean forceChangeBaselineTopology,
           boolean isAutoAdjust
       )`
   
   9.      Removed, combined with #8:
   `IGridClusterStateProcessor#changeGlobalState0(
           ClusterState state,
           BaselineTopology blt,
           boolean forceChangeBaselineTopology,
           boolean isAutoAdjust
       )`

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [ignite] nizhikov merged pull request #7514: IGNITE-12773 : Reduce number of cluster deactivation methods in internal API.

Posted by GitBox <gi...@apache.org>.
nizhikov merged pull request #7514: IGNITE-12773 : Reduce number of cluster deactivation methods in internal API.
URL: https://github.com/apache/ignite/pull/7514
 
 
   

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [ignite] NSAmelchev commented on a change in pull request #7514: IGNITE-12773 : Reduce number of cluster deactivation methods in internal API.

Posted by GitBox <gi...@apache.org>.
NSAmelchev commented on a change in pull request #7514: IGNITE-12773 : Reduce number of cluster deactivation methods in internal API.
URL: https://github.com/apache/ignite/pull/7514#discussion_r391597542
 
 

 ##########
 File path: modules/core/src/main/java/org/apache/ignite/internal/commandline/ActivateCommand.java
 ##########
 @@ -49,7 +50,7 @@
         try (GridClient client = Command.startClient(cfg)) {
             GridClientClusterState state = client.state();
 
-            state.active(true);
+            state.state(ACTIVE, true);
 
 Review comment:
   Why `forceDeactivation` should be `true` for cluster activate?

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [ignite] NSAmelchev commented on a change in pull request #7514: IGNITE-12773 : Reduce number of cluster deactivation methods in internal API.

Posted by GitBox <gi...@apache.org>.
NSAmelchev commented on a change in pull request #7514: IGNITE-12773 : Reduce number of cluster deactivation methods in internal API.
URL: https://github.com/apache/ignite/pull/7514#discussion_r391592482
 
 

 ##########
 File path: modules/clients/src/test/java/org/apache/ignite/internal/processors/rest/ChangeStateCommandHandlerTest.java
 ##########
 @@ -87,23 +89,23 @@
     public void testActivateDeActivate() throws GridClientException {
         GridClientClusterState state = client.state();
 
-        boolean active = state.active();
+        boolean active = state.state() == ACTIVE;
 
         assertTrue(active);
 
-        state.active(false);
+        state.state(INACTIVE, true);
 
         IgniteEx ig1 = grid(0);
         IgniteEx ig2 = grid(1);
 
         assertFalse(ig1.active());
         assertFalse(ig2.active());
-        assertFalse(state.active());
+        assertEquals(state.state(), INACTIVE);
 
-        state.active(true);
+        state.state(ACTIVE, true);
 
 Review comment:
   There is no cluster deactivation, `forceDeactivation` should be false.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [ignite] Vladsz83 edited a comment on issue #7514: Ignite 12773

Posted by GitBox <gi...@apache.org>.
Vladsz83 edited a comment on issue #7514: Ignite 12773
URL: https://github.com/apache/ignite/pull/7514#issuecomment-597755820
 
 
   Major code changes:
   
   1.	Removed `GridClientClusterState#active()`
   
   2.	Removed `GridClientClusterState#active(boolean active)`
   
   3.	Removed
   `IGridClusterStateProcessor#changeGlobalState(
       boolean activate,
       Collection<? extends BaselineNode> baselineNodes,
       boolean forceChangeBaselineTopology
   )`
   
   4.	Removed
   `GridClusterStateProcessor#changeGlobalState(
       final boolean activate,
       Collection<? extends BaselineNode> baselineNodes,
       boolean forceChangeBaselineTopology,
       boolean isAutoAdjust
   )`
   
   5.	Removed
   `GridClusterStateProcessor#changeGlobalState(
       final boolean activate,
       Collection<? extends BaselineNode> baselineNodes,
       boolean forceChangeBaselineTopology
   )`
   
   6.	Removed
   `GridClusterStateProcessor#changeGlobalState(
       ClusterState state,
       Collection<? extends BaselineNode> baselineNodes,
       boolean forceChangeBaselineTopology
   )`
   
   7.	Added `boolean isAutoAdjust` to 
   `IGridClusterStateProcessor#changeGlobalState(
       ClusterState state,
       Collection<? extends BaselineNode> baselineNodes,
       boolean forceChangeBaselineTopology
   )`
   
   8.	Added `@Override` to 
   `IGridClusterStateProcessor#changeGlobalState(
           ClusterState state,
           Collection<? extends BaselineNode> baselineNodes,
           boolean forceChangeBaselineTopology,
           boolean isAutoAdjust
       )`
   
   9.      Removed, combined with ##8
   `IGridClusterStateProcessor#changeGlobalState0(
           ClusterState state,
           BaselineTopology blt,
           boolean forceChangeBaselineTopology,
           boolean isAutoAdjust
       )`

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [ignite] Vladsz83 edited a comment on issue #7514: Ignite 12773

Posted by GitBox <gi...@apache.org>.
Vladsz83 edited a comment on issue #7514: Ignite 12773
URL: https://github.com/apache/ignite/pull/7514#issuecomment-597755820
 
 
   Major code changes:
   
   1.	Removed `GridClientClusterState#active()`
   
   2.	Removed `GridClientClusterState#active(boolean active)`
   
   3.	Removed
   `IGridClusterStateProcessor#changeGlobalState(
       boolean activate,
       Collection<? extends BaselineNode> baselineNodes,
       boolean forceChangeBaselineTopology
   )`
   
   4.	Removed
   `GridClusterStateProcessor#changeGlobalState(
       final boolean activate,
       Collection<? extends BaselineNode> baselineNodes,
       boolean forceChangeBaselineTopology,
       boolean isAutoAdjust
   )`
   
   5.	Removed
   `GridClusterStateProcessor#changeGlobalState(
       final boolean activate,
       Collection<? extends BaselineNode> baselineNodes,
       boolean forceChangeBaselineTopology
   )`
   
   6.	Remove d
   `GridClusterStateProcessor#changeGlobalState(
       ClusterState state,
       Collection<? extends BaselineNode> baselineNodes,
       boolean forceChangeBaselineTopology
   )`
   
   7.	Added boolean isAutoAdjust to 
   `IGridClusterStateProcessor#changeGlobalState(
       ClusterState state,
       Collection<? extends BaselineNode> baselineNodes,
       boolean forceChangeBaselineTopology,
      /* here */ boolean isAutoAdjust /* here */
   )`
   
   8.	Added @Override to 
   `/* here */ @Override /* here */
   IGridClusterStateProcessor#changeGlobalState(
           ClusterState state,
           Collection<? extends BaselineNode> baselineNodes,
           boolean forceChangeBaselineTopology,
           boolean isAutoAdjust
       )`
   
   9.      Removed, combined with #8:
   `IGridClusterStateProcessor#changeGlobalState0(
           ClusterState state,
           BaselineTopology blt,
           boolean forceChangeBaselineTopology,
           boolean isAutoAdjust
       )`

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [ignite] Vladsz83 commented on a change in pull request #7514: IGNITE-12773 : Reduce number of cluster deactivation methods in internal API.

Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on a change in pull request #7514: IGNITE-12773 : Reduce number of cluster deactivation methods in internal API.
URL: https://github.com/apache/ignite/pull/7514#discussion_r391602596
 
 

 ##########
 File path: modules/clients/src/test/java/org/apache/ignite/internal/processors/rest/ChangeStateCommandHandlerTest.java
 ##########
 @@ -87,23 +89,23 @@
     public void testActivateDeActivate() throws GridClientException {
         GridClientClusterState state = client.state();
 
-        boolean active = state.active();
+        boolean active = state.state() == ACTIVE;
 
         assertTrue(active);
 
-        state.active(false);
+        state.state(INACTIVE, true);
 
         IgniteEx ig1 = grid(0);
         IgniteEx ig2 = grid(1);
 
         assertFalse(ig1.active());
         assertFalse(ig2.active());
-        assertFalse(state.active());
+        assertEquals(state.state(), INACTIVE);
 
-        state.active(true);
+        state.state(ACTIVE, true);
 
 Review comment:
   > There is no cluster deactivation, `forceDeactivation` should be false.
   
   Previous invocations were considered forced regardless of certain state. Shouldn't we do the same way?

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [ignite] Vladsz83 edited a comment on issue #7514: Ignite 12773

Posted by GitBox <gi...@apache.org>.
Vladsz83 edited a comment on issue #7514: Ignite 12773
URL: https://github.com/apache/ignite/pull/7514#issuecomment-597755820
 
 
   Major code changes:
   
   1.	Removed `GridClientClusterState#active()`
   
   2.	Removed `GridClientClusterState#active(boolean active)`
   
   3.	Removed
   `IGridClusterStateProcessor#changeGlobalState(
       boolean activate,
       Collection<? extends BaselineNode> baselineNodes,
       boolean forceChangeBaselineTopology
   )`
   
   4.	Removed
   `GridClusterStateProcessor#changeGlobalState(
       final boolean activate,
       Collection<? extends BaselineNode> baselineNodes,
       boolean forceChangeBaselineTopology,
       boolean isAutoAdjust
   )`
   
   5.	Removed
   `GridClusterStateProcessor#changeGlobalState(
       final boolean activate,
       Collection<? extends BaselineNode> baselineNodes,
       boolean forceChangeBaselineTopology
   )`
   
   6.	Remove d
   `GridClusterStateProcessor#changeGlobalState(
       ClusterState state,
       Collection<? extends BaselineNode> baselineNodes,
       boolean forceChangeBaselineTopology
   )`
   
   7.	Added boolean isAutoAdjust to 
   `IGridClusterStateProcessor#changeGlobalState(
       ClusterState state,
       Collection<? extends BaselineNode> baselineNodes,
       boolean forceChangeBaselineTopology
   )`
   
   8.	Added @Override to 
   `IGridClusterStateProcessor#changeGlobalState(
           ClusterState state,
           Collection<? extends BaselineNode> baselineNodes,
           boolean forceChangeBaselineTopology,
           boolean isAutoAdjust
       )`
   
   9.      Removed, combined with #8:
   `IGridClusterStateProcessor#changeGlobalState0(
           ClusterState state,
           BaselineTopology blt,
           boolean forceChangeBaselineTopology,
           boolean isAutoAdjust
       )`

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [ignite] Vladsz83 edited a comment on issue #7514: Ignite 12773

Posted by GitBox <gi...@apache.org>.
Vladsz83 edited a comment on issue #7514: Ignite 12773
URL: https://github.com/apache/ignite/pull/7514#issuecomment-597755820
 
 
   Major code changes:
   
   1.	Removed `GridClientClusterState#active()`
   
   2.	Removed `GridClientClusterState#active(boolean active)`
   
   3.	Removed
   `IGridClusterStateProcessor#changeGlobalState(
       boolean activate,
       Collection<? extends BaselineNode> baselineNodes,
       boolean forceChangeBaselineTopology
   )`
   
   4.	Removed
   `GridClusterStateProcessor#changeGlobalState(
       final boolean activate,
       Collection<? extends BaselineNode> baselineNodes,
       boolean forceChangeBaselineTopology,
       boolean isAutoAdjust
   )`
   
   5.	Removed
   `GridClusterStateProcessor#changeGlobalState(
       final boolean activate,
       Collection<? extends BaselineNode> baselineNodes,
       boolean forceChangeBaselineTopology
   )`
   
   6.	Remove d
   `GridClusterStateProcessor#changeGlobalState(
       ClusterState state,
       Collection<? extends BaselineNode> baselineNodes,
       boolean forceChangeBaselineTopology
   )`
   
   7.	Added boolean isAutoAdjust to 
   `IGridClusterStateProcessor#changeGlobalState(
       ClusterState state,
       Collection<? extends BaselineNode> baselineNodes,
       boolean forceChangeBaselineTopology
   )`
   
   8.	Added @Override to 
   `IGridClusterStateProcessor#changeGlobalState(
           ClusterState state,
           Collection<? extends BaselineNode> baselineNodes,
           boolean forceChangeBaselineTopology,
           boolean isAutoAdjust
       )`
   
   9.      Removed, combined with 8
   `IGridClusterStateProcessor#changeGlobalState0(
           ClusterState state,
           BaselineTopology blt,
           boolean forceChangeBaselineTopology,
           boolean isAutoAdjust
       )`

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [ignite] NSAmelchev commented on a change in pull request #7514: IGNITE-12773 : Reduce number of cluster deactivation methods in internal API.

Posted by GitBox <gi...@apache.org>.
NSAmelchev commented on a change in pull request #7514: IGNITE-12773 : Reduce number of cluster deactivation methods in internal API.
URL: https://github.com/apache/ignite/pull/7514#discussion_r391640663
 
 

 ##########
 File path: modules/clients/src/test/java/org/apache/ignite/internal/processors/rest/ChangeStateCommandHandlerTest.java
 ##########
 @@ -87,23 +89,23 @@
     public void testActivateDeActivate() throws GridClientException {
         GridClientClusterState state = client.state();
 
-        boolean active = state.active();
+        boolean active = state.state() == ACTIVE;
 
         assertTrue(active);
 
-        state.active(false);
+        state.state(INACTIVE, true);
 
         IgniteEx ig1 = grid(0);
         IgniteEx ig2 = grid(1);
 
         assertFalse(ig1.active());
         assertFalse(ig2.active());
-        assertFalse(state.active());
+        assertEquals(state.state(), INACTIVE);
 
-        state.active(true);
+        state.state(ACTIVE, true);
 
 Review comment:
   Previous deprecated call was forced internally for both cases. I suggest do not forcing new public method call to cluster activation.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [ignite] Vladsz83 commented on a change in pull request #7514: IGNITE-12773 : Reduce number of cluster deactivation methods in internal API.

Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on a change in pull request #7514: IGNITE-12773 : Reduce number of cluster deactivation methods in internal API.
URL: https://github.com/apache/ignite/pull/7514#discussion_r391604982
 
 

 ##########
 File path: modules/core/src/main/java/org/apache/ignite/internal/client/impl/GridClientClusterStateImpl.java
 ##########
 @@ -54,16 +53,6 @@ public GridClientClusterStateImpl(
         super(client, nodes, filter, balancer);
     }
 
-    /** {@inheritDoc} */
-    @Override public void active(final boolean active) throws GridClientException {
-        state(active ? ACTIVE : INACTIVE, true);
-    }
-
-    /** {@inheritDoc} */
-    @Override public boolean active() throws GridClientException {
-        return withReconnectHandling(GridClientConnection::currentState).get();
 
 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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [ignite] Vladsz83 edited a comment on issue #7514: Ignite 12773

Posted by GitBox <gi...@apache.org>.
Vladsz83 edited a comment on issue #7514: Ignite 12773
URL: https://github.com/apache/ignite/pull/7514#issuecomment-597755820
 
 
   
   1.	Removed `GridClientClusterState#active()`
   
   2.	Removed `GridClientClusterState#active(boolean active)`
   
   3.	Removed
   `IGridClusterStateProcessor#changeGlobalState(
       boolean activate,
       Collection<? extends BaselineNode> baselineNodes,
       boolean forceChangeBaselineTopology
   )`
   
   4.	Removed
   `GridClusterStateProcessor#changeGlobalState(
       final boolean activate,
       Collection<? extends BaselineNode> baselineNodes,
       boolean forceChangeBaselineTopology,
       boolean isAutoAdjust
   )`
   
   5.	Removed
   `GridClusterStateProcessor#changeGlobalState(
       final boolean activate,
       Collection<? extends BaselineNode> baselineNodes,
       boolean forceChangeBaselineTopology
   )`
   
   6.	Remove d
   `GridClusterStateProcessor#changeGlobalState(
       ClusterState state,
       Collection<? extends BaselineNode> baselineNodes,
       boolean forceChangeBaselineTopology
   )`
   
   7.	Added boolean isAutoAdjust to 
   `IGridClusterStateProcessor#changeGlobalState(
       ClusterState state,
       Collection<? extends BaselineNode> baselineNodes,
       boolean forceChangeBaselineTopology,
      /* here */ boolean isAutoAdjust /* here */
   )`
   
   8.	Added @Override to 
   `/* here */ @Override /* here */
   IGridClusterStateProcessor#changeGlobalState(
           ClusterState state,
           Collection<? extends BaselineNode> baselineNodes,
           boolean forceChangeBaselineTopology,
           boolean isAutoAdjust
       )`
   
   9.      Removed, combined with #8:
   `IGridClusterStateProcessor#changeGlobalState0(
           ClusterState state,
           BaselineTopology blt,
           boolean forceChangeBaselineTopology,
           boolean isAutoAdjust
       )`

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [ignite] Vladsz83 edited a comment on issue #7514: Ignite 12773

Posted by GitBox <gi...@apache.org>.
Vladsz83 edited a comment on issue #7514: Ignite 12773
URL: https://github.com/apache/ignite/pull/7514#issuecomment-597755820
 
 
   Major code changes:
   
   1.	Removed `GridClientClusterState#active()`
   
   2.	Removed `GridClientClusterState#active(boolean active)`
   
   3.	Removed
   `IGridClusterStateProcessor#changeGlobalState(
       boolean activate,
       Collection<? extends BaselineNode> baselineNodes,
       boolean forceChangeBaselineTopology
   )`
   
   4.	Removed
   `GridClusterStateProcessor#changeGlobalState(
       final boolean activate,
       Collection<? extends BaselineNode> baselineNodes,
       boolean forceChangeBaselineTopology,
       boolean isAutoAdjust
   )`
   
   5.	Removed
   `GridClusterStateProcessor#changeGlobalState(
       final boolean activate,
       Collection<? extends BaselineNode> baselineNodes,
       boolean forceChangeBaselineTopology
   )`
   
   6.	Remove d
   `GridClusterStateProcessor#changeGlobalState(
       ClusterState state,
       Collection<? extends BaselineNode> baselineNodes,
       boolean forceChangeBaselineTopology
   )`
   
   7.	Added boolean isAutoAdjust to 
   `IGridClusterStateProcessor#changeGlobalState(
       ClusterState state,
       Collection<? extends BaselineNode> baselineNodes,
       boolean forceChangeBaselineTopology,
      /* here */ boolean isAutoAdjust /* here */
   )`
   
   8.	Added @Override to 
   /* here */ @Override /* here */
   `IGridClusterStateProcessor#changeGlobalState(
           ClusterState state,
           Collection<? extends BaselineNode> baselineNodes,
           boolean forceChangeBaselineTopology,
           boolean isAutoAdjust
       )`
   
   9.      Removed, combined with #8:
   `IGridClusterStateProcessor#changeGlobalState0(
           ClusterState state,
           BaselineTopology blt,
           boolean forceChangeBaselineTopology,
           boolean isAutoAdjust
       )`

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [ignite] Vladsz83 commented on a change in pull request #7514: IGNITE-12773 : Reduce number of cluster deactivation methods in internal API.

Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on a change in pull request #7514: IGNITE-12773 : Reduce number of cluster deactivation methods in internal API.
URL: https://github.com/apache/ignite/pull/7514#discussion_r391605843
 
 

 ##########
 File path: modules/core/src/main/java/org/apache/ignite/internal/cluster/IgniteClusterImpl.java
 ##########
 @@ -313,7 +315,7 @@ public IgniteClusterImpl(GridKernalContext ctx) {
         guard();
 
         try {
-            ctx.state().changeGlobalState(active, serverNodes(), false).get();
+            ctx.state().changeGlobalState(active ? ACTIVE : INACTIVE, true, serverNodes(), false, false).get();
 
 Review comment:
   All previous internal invocations were considered forced. I think we should not change it where not required. Not-forced call are considered only for user commands through command.sh/Rest .

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [ignite] Vladsz83 edited a comment on issue #7514: Ignite 12773

Posted by GitBox <gi...@apache.org>.
Vladsz83 edited a comment on issue #7514: Ignite 12773
URL: https://github.com/apache/ignite/pull/7514#issuecomment-597755820
 
 
   Major code changes:
   
   1.	Removed `GridClientClusterState#active()`
   
   2.	Removed `GridClientClusterState#active(boolean active)`
   
   3.	Removed
   `IGridClusterStateProcessor#changeGlobalState(
       boolean activate,
       Collection<? extends BaselineNode> baselineNodes,
       boolean forceChangeBaselineTopology
   )`
   
   4.	Removed
   `GridClusterStateProcessor#changeGlobalState(
       final boolean activate,
       Collection<? extends BaselineNode> baselineNodes,
       boolean forceChangeBaselineTopology,
       boolean isAutoAdjust
   )`
   
   5.	Removed
   `GridClusterStateProcessor#changeGlobalState(
       final boolean activate,
       Collection<? extends BaselineNode> baselineNodes,
       boolean forceChangeBaselineTopology
   )`
   
   6.	Remove d
   `GridClusterStateProcessor#changeGlobalState(
       ClusterState state,
       Collection<? extends BaselineNode> baselineNodes,
       boolean forceChangeBaselineTopology
   )`
   
   7.	Added boolean isAutoAdjust to 
   `IGridClusterStateProcessor#changeGlobalState(
       ClusterState state,
       Collection<? extends BaselineNode> baselineNodes,
       boolean forceChangeBaselineTopology
   )`
   
   8.	Added `@Override` to 
   `IGridClusterStateProcessor#changeGlobalState(
           ClusterState state,
           Collection<? extends BaselineNode> baselineNodes,
           boolean forceChangeBaselineTopology,
           boolean isAutoAdjust
       )`
   
   9.      Removed, combined with 8
   IGridClusterStateProcessor#changeGlobalState0(
           ClusterState state,
           BaselineTopology blt,
           boolean forceChangeBaselineTopology,
           boolean isAutoAdjust
       )

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [ignite] Vladsz83 commented on a change in pull request #7514: IGNITE-12773 : Reduce number of cluster deactivation methods in internal API.

Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on a change in pull request #7514: IGNITE-12773 : Reduce number of cluster deactivation methods in internal API.
URL: https://github.com/apache/ignite/pull/7514#discussion_r391668418
 
 

 ##########
 File path: modules/clients/src/test/java/org/apache/ignite/internal/processors/rest/ChangeStateCommandHandlerTest.java
 ##########
 @@ -87,23 +89,23 @@
     public void testActivateDeActivate() throws GridClientException {
         GridClientClusterState state = client.state();
 
-        boolean active = state.active();
+        boolean active = state.state() == ACTIVE;
 
         assertTrue(active);
 
-        state.active(false);
+        state.state(INACTIVE, true);
 
         IgniteEx ig1 = grid(0);
         IgniteEx ig2 = grid(1);
 
         assertFalse(ig1.active());
         assertFalse(ig2.active());
-        assertFalse(state.active());
+        assertEquals(state.state(), INACTIVE);
 
-        state.active(true);
+        state.state(ACTIVE, false);
 
         assertTrue(ig1.active());
         assertTrue(ig2.active());
-        assertTrue(state.active());
+        assertEquals(state.state(), ACTIVE);
 
 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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [ignite] Vladsz83 commented on a change in pull request #7514: IGNITE-12773 : Reduce number of cluster deactivation methods in internal API.

Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on a change in pull request #7514: IGNITE-12773 : Reduce number of cluster deactivation methods in internal API.
URL: https://github.com/apache/ignite/pull/7514#discussion_r391676594
 
 

 ##########
 File path: modules/clients/src/test/java/org/apache/ignite/internal/processors/rest/ChangeStateCommandHandlerTest.java
 ##########
 @@ -87,23 +89,23 @@
     public void testActivateDeActivate() throws GridClientException {
         GridClientClusterState state = client.state();
 
-        boolean active = state.active();
+        boolean active = state.state() == ACTIVE;
 
 Review comment:
   > `ClusterState.active(ClusterState state)` was used.
   
   `GridClientClusterState.active()` was used, no? Changed to `GridClientClusterState.state()`. Isn't is ok?

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [ignite] Vladsz83 edited a comment on issue #7514: Ignite 12773

Posted by GitBox <gi...@apache.org>.
Vladsz83 edited a comment on issue #7514: Ignite 12773
URL: https://github.com/apache/ignite/pull/7514#issuecomment-597755820
 
 
   Major code changes:
   
   1.	Removed `GridClientClusterState#active()`
   
   2.	Removed `GridClientClusterState#active(boolean active)`
   
   3.	Removed
   `IGridClusterStateProcessor#changeGlobalState(
       boolean activate,
       Collection<? extends BaselineNode> baselineNodes,
       boolean forceChangeBaselineTopology
   )`
   
   4.	Removed
   `GridClusterStateProcessor#changeGlobalState(
       final boolean activate,
       Collection<? extends BaselineNode> baselineNodes,
       boolean forceChangeBaselineTopology,
       boolean isAutoAdjust
   )`
   
   5.	Removed
   `GridClusterStateProcessor#changeGlobalState(
       final boolean activate,
       Collection<? extends BaselineNode> baselineNodes,
       boolean forceChangeBaselineTopology
   )`
   
   6.	Remove d
   `GridClusterStateProcessor#changeGlobalState(
       ClusterState state,
       Collection<? extends BaselineNode> baselineNodes,
       boolean forceChangeBaselineTopology
   )`
   
   7.	Added boolean isAutoAdjust to 
   `IGridClusterStateProcessor#changeGlobalState(
       ClusterState state,
       Collection<? extends BaselineNode> baselineNodes,
       boolean forceChangeBaselineTopology
   )`
   
   8.	Added `@Override` to 
   `IGridClusterStateProcessor#changeGlobalState(
           ClusterState state,
           Collection<? extends BaselineNode> baselineNodes,
           boolean forceChangeBaselineTopology,
           boolean isAutoAdjust
       )`
   
   9.      Removed, combined with 8
   `IGridClusterStateProcessor#changeGlobalState0(
           ClusterState state,
           BaselineTopology blt,
           boolean forceChangeBaselineTopology,
           boolean isAutoAdjust
       )`

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [ignite] Vladsz83 commented on issue #7514: Ignite 12773

Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on issue #7514: Ignite 12773
URL: https://github.com/apache/ignite/pull/7514#issuecomment-597755820
 
 
   {code:java}
   1.	Removed
   GridClientClusterState#active()
   
   2.	Removed
   GridClientClusterState#active(boolean active)
   
   3.	Removed
   IGridClusterStateProcessor#changeGlobalState(
       boolean activate,
       Collection<? extends BaselineNode> baselineNodes,
       boolean forceChangeBaselineTopology
   )
   
   4.	Removed
   GridClusterStateProcessor#changeGlobalState(
       final boolean activate,
       Collection<? extends BaselineNode> baselineNodes,
       boolean forceChangeBaselineTopology,
       boolean isAutoAdjust
   )
   
   5.	Removed
   GridClusterStateProcessor#changeGlobalState(
       final boolean activate,
       Collection<? extends BaselineNode> baselineNodes,
       boolean forceChangeBaselineTopology
   )
   
   6.	Remove 
   GridClusterStateProcessor#changeGlobalState(
       ClusterState state,
       Collection<? extends BaselineNode> baselineNodes,
       boolean forceChangeBaselineTopology
   )
   
   7.	Added boolean isAutoAdjust to 
   IGridClusterStateProcessor#changeGlobalState(
       ClusterState state,
       Collection<? extends BaselineNode> baselineNodes,
       boolean forceChangeBaselineTopology,
      /* here */ boolean isAutoAdjust /* here */
   )
   
   8.	Added @Override to 
   /* here */ @Override /* here */
   IGridClusterStateProcessor#changeGlobalState(
           ClusterState state,
           Collection<? extends BaselineNode> baselineNodes,
           boolean forceChangeBaselineTopology,
           boolean isAutoAdjust
       )
   
   9.      Removed, combined with #8:
   IGridClusterStateProcessor#changeGlobalState0(
           ClusterState state,
           BaselineTopology blt,
           boolean forceChangeBaselineTopology,
           boolean isAutoAdjust
       ) 
   {code}
   
   - [ ] 

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [ignite] NSAmelchev commented on a change in pull request #7514: IGNITE-12773 : Reduce number of cluster deactivation methods in internal API.

Posted by GitBox <gi...@apache.org>.
NSAmelchev commented on a change in pull request #7514: IGNITE-12773 : Reduce number of cluster deactivation methods in internal API.
URL: https://github.com/apache/ignite/pull/7514#discussion_r391593804
 
 

 ##########
 File path: modules/clients/src/test/java/org/apache/ignite/jdbc/thin/MvccJdbcTransactionFinishOnDeactivatedClusterSelfTest.java
 ##########
 @@ -153,7 +154,7 @@ private void deactivateThroughClient() throws Exception {
 
             log.info(">>> Try to deactivate ...");
 
-            state.active(false);
+            state.state(ClusterState.INACTIVE, true);
 
 Review comment:
   Can static import be used? like previuos test used

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [ignite] Vladsz83 commented on a change in pull request #7514: IGNITE-12773 : Reduce number of cluster deactivation methods in internal API.

Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on a change in pull request #7514: IGNITE-12773 : Reduce number of cluster deactivation methods in internal API.
URL: https://github.com/apache/ignite/pull/7514#discussion_r391603268
 
 

 ##########
 File path: modules/clients/src/test/java/org/apache/ignite/jdbc/thin/MvccJdbcTransactionFinishOnDeactivatedClusterSelfTest.java
 ##########
 @@ -153,7 +154,7 @@ private void deactivateThroughClient() throws Exception {
 
             log.info(">>> Try to deactivate ...");
 
-            state.active(false);
+            state.state(ClusterState.INACTIVE, true);
 
 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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [ignite] Vladsz83 commented on a change in pull request #7514: IGNITE-12773 : Reduce number of cluster deactivation methods in internal API.

Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on a change in pull request #7514: IGNITE-12773 : Reduce number of cluster deactivation methods in internal API.
URL: https://github.com/apache/ignite/pull/7514#discussion_r391606156
 
 

 ##########
 File path: modules/core/src/main/java/org/apache/ignite/internal/commandline/ActivateCommand.java
 ##########
 @@ -49,7 +50,7 @@
         try (GridClient client = Command.startClient(cfg)) {
             GridClientClusterState state = client.state();
 
-            state.active(true);
+            state.state(ACTIVE, true);
 
 Review comment:
   Same. Previous invocations were considered forced. I think we should not change it where not required.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [ignite] NSAmelchev commented on a change in pull request #7514: IGNITE-12773 : Reduce number of cluster deactivation methods in internal API.

Posted by GitBox <gi...@apache.org>.
NSAmelchev commented on a change in pull request #7514: IGNITE-12773 : Reduce number of cluster deactivation methods in internal API.
URL: https://github.com/apache/ignite/pull/7514#discussion_r391665493
 
 

 ##########
 File path: modules/clients/src/test/java/org/apache/ignite/internal/processors/rest/ChangeStateCommandHandlerTest.java
 ##########
 @@ -87,23 +89,23 @@
     public void testActivateDeActivate() throws GridClientException {
         GridClientClusterState state = client.state();
 
-        boolean active = state.active();
+        boolean active = state.state() == ACTIVE;
 
         assertTrue(active);
 
-        state.active(false);
+        state.state(INACTIVE, true);
 
         IgniteEx ig1 = grid(0);
         IgniteEx ig2 = grid(1);
 
         assertFalse(ig1.active());
         assertFalse(ig2.active());
-        assertFalse(state.active());
+        assertEquals(state.state(), INACTIVE);
 
-        state.active(true);
+        state.state(ACTIVE, false);
 
         assertTrue(ig1.active());
         assertTrue(ig2.active());
-        assertTrue(state.active());
+        assertEquals(state.state(), ACTIVE);
 
 Review comment:
   Mixed up expected-actual

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [ignite] Vladsz83 edited a comment on issue #7514: Ignite 12773

Posted by GitBox <gi...@apache.org>.
Vladsz83 edited a comment on issue #7514: Ignite 12773
URL: https://github.com/apache/ignite/pull/7514#issuecomment-597755820
 
 
   
   1.	Removed `GridClientClusterState#active()`
   
   2.	Removed `GridClientClusterState#active(boolean active)`
   
   3.	Removed
   `IGridClusterStateProcessor#changeGlobalState(
       boolean activate,
       Collection<? extends BaselineNode> baselineNodes,
       boolean forceChangeBaselineTopology
   )`
   
   4.	Removed
   `GridClusterStateProcessor#changeGlobalState(
       final boolean activate,
       Collection<? extends BaselineNode> baselineNodes,
       boolean forceChangeBaselineTopology,
       boolean isAutoAdjust
   )`
   
   5.	Removed
   GridClusterStateProcessor#changeGlobalState(
       final boolean activate,
       Collection<? extends BaselineNode> baselineNodes,
       boolean forceChangeBaselineTopology
   )
   
   6.	Remove d
   `GridClusterStateProcessor#changeGlobalState(
       ClusterState state,
       Collection<? extends BaselineNode> baselineNodes,
       boolean forceChangeBaselineTopology
   )`
   
   7.	Added boolean isAutoAdjust to 
   `IGridClusterStateProcessor#changeGlobalState(
       ClusterState state,
       Collection<? extends BaselineNode> baselineNodes,
       boolean forceChangeBaselineTopology,
      /* here */ boolean isAutoAdjust /* here */
   )`
   
   8.	Added @Override to 
   `/* here */ @Override /* here */
   IGridClusterStateProcessor#changeGlobalState(
           ClusterState state,
           Collection<? extends BaselineNode> baselineNodes,
           boolean forceChangeBaselineTopology,
           boolean isAutoAdjust
       )`
   
   9.      Removed, combined with #8:
   `IGridClusterStateProcessor#changeGlobalState0(
           ClusterState state,
           BaselineTopology blt,
           boolean forceChangeBaselineTopology,
           boolean isAutoAdjust
       )`

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services