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/06/29 19:03:25 UTC

[GitHub] [ignite] aleksey-kurinov opened a new pull request #7977: Ignite 13040

aleksey-kurinov opened a new pull request #7977:
URL: https://github.com/apache/ignite/pull/7977


   changes:
   1. "TcpDiscoveryAbstractMessage msg" parameter removed from TcpDiscoverySpi.writeToSocket()
   2. writeToSocket() method calls updated in ServerImpl, TcpDiscoverySpi classes
   3. tests updated:
   
   a)
    @Override protected void writeToSocket(Socket sock, TcpDiscoveryAbstractMessage msg, byte[] data, long timeout)
   replaced with 
   @Override protected void writeToSocket(Socket sock, TcpDiscoveryAbstractMessage msg, byte[] data, long timeout)
   in IgniteClientRejoinTest, IgniteCacheTopologySplitAbstractTest, IgniteTcpCommunicationConnectOnInitTest, TcpDiscoveryFailedJoinTest, TcpDiscoveryPendingMessageDeliveryTest
   
   b)
   @Override protected void writeToSocket(Socket sock, TcpDiscoveryAbstractMessage msg, byte[] data,long timeout)
   replaced with 
   protected void writeToSocket(Socket sock, TcpDiscoveryAbstractMessage msg, byte[] data, long timeout)
   in
   FailDiscoverySpi, BlockTcpDiscoverySpi, TcpClientDiscoverySpiFailureTimeoutSelfTest, TcpClientDiscoverySpiSelfTest, TcpDiscoveryCoordinatorFailureTest
   
   


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



[GitHub] [ignite] Vladsz83 commented on a change in pull request #7977: Ignite 13040

Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on a change in pull request #7977:
URL: https://github.com/apache/ignite/pull/7977#discussion_r470096717



##########
File path: modules/core/src/test/java/org/apache/ignite/spi/discovery/tcp/TcpDiscoveryCoordinatorFailureTest.java
##########
@@ -282,17 +282,17 @@ public void awaitDrop() throws InterruptedException, IgniteCheckedException {
         /** {@inheritDoc} */
         @Override protected void writeToSocket(
             Socket sock,
-            TcpDiscoveryAbstractMessage msg,
             byte[] data,
             long timeout
         ) throws IOException {
-            if (isDrop(msg)) {
-                // Replace logic routine message with a stub to update last-sent-time to avoid segmentation on
-                // connRecoveryTimeout.
-                msg = new TcpDiscoveryConnectionCheckMessage(locNode);
-            }
 
-            super.writeToSocket(sock, msg, data, timeout);
+            // Replace logic routine message with a stub to update last-sent-time to avoid segmentation on

Review comment:
       @aleksey-kurinov I'm sure we don't need this failure emulation in `FailingDiscoverySpi` any nore. It passes no message. This is idea of the ticket. The `msg` is always null. Check it. This `writeToSocket` works with `data`. The test goes well without emulation a failure here.




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



[GitHub] [ignite] Vladsz83 commented on a change in pull request #7977: Ignite 13040

Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on a change in pull request #7977:
URL: https://github.com/apache/ignite/pull/7977#discussion_r470123562



##########
File path: modules/core/src/test/java/org/apache/ignite/spi/discovery/tcp/TcpClientDiscoverySpiSelfTest.java
##########
@@ -69,9 +69,11 @@
 import org.apache.ignite.spi.discovery.tcp.ipfinder.vm.TcpDiscoveryVmIpFinder;
 import org.apache.ignite.spi.discovery.tcp.messages.TcpDiscoveryAbstractMessage;
 import org.apache.ignite.spi.discovery.tcp.messages.TcpDiscoveryClientReconnectMessage;
+import org.apache.ignite.spi.discovery.tcp.messages.TcpDiscoveryConnectionCheckMessage;
 import org.apache.ignite.spi.discovery.tcp.messages.TcpDiscoveryJoinRequestMessage;
 import org.apache.ignite.spi.discovery.tcp.messages.TcpDiscoveryNodeAddFinishedMessage;
 import org.apache.ignite.spi.discovery.tcp.messages.TcpDiscoveryNodeAddedMessage;
+

Review comment:
       This opening has to get removed.




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



[GitHub] [ignite] Vladsz83 commented on a change in pull request #7977: Ignite 13040

Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on a change in pull request #7977:
URL: https://github.com/apache/ignite/pull/7977#discussion_r470573184



##########
File path: modules/core/src/test/java/org/apache/ignite/spi/discovery/tcp/TcpClientDiscoverySpiSelfTest.java
##########
@@ -2578,15 +2578,23 @@ private void pauseResumeOperation(boolean isPause, AtomicBoolean... locks) {
                 afterWrite.apply(msg, sock);
         }
 
-        /** {@inheritDoc} */

Review comment:
       I see particular case as understanding what failed test does and how to fix it. Or, at least, how to locally return broken logic. Skipping certain messages in this case, not skipping others. I would start with logging messages in failed test on master-branch. Which of them are skipped and what is the corresponding raw data (byte[]). We might slightly re-write skipping messages basing on the raw data, not on the message unused.




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



[GitHub] [ignite] Vladsz83 commented on a change in pull request #7977: Ignite 13040

Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on a change in pull request #7977:
URL: https://github.com/apache/ignite/pull/7977#discussion_r470140435



##########
File path: modules/core/src/test/java/org/apache/ignite/spi/discovery/tcp/TcpClientDiscoverySpiSelfTest.java
##########
@@ -2578,15 +2578,23 @@ private void pauseResumeOperation(boolean isPause, AtomicBoolean... locks) {
                 afterWrite.apply(msg, sock);
         }
 
-        /** {@inheritDoc} */

Review comment:
       I'm afraid, sending TcpDiscoveryConnectionCheckMessage won't help us here. Such trick was involved by me accurately where it could work. Unfortunately, it not going to work everywhere. DiscoverySpi is quite complex.




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



[GitHub] [ignite] aleksey-kurinov commented on a change in pull request #7977: Ignite 13040

Posted by GitBox <gi...@apache.org>.
aleksey-kurinov commented on a change in pull request #7977:
URL: https://github.com/apache/ignite/pull/7977#discussion_r465167794



##########
File path: modules/core/src/test/java/org/apache/ignite/spi/discovery/tcp/TcpDiscoveryCoordinatorFailureTest.java
##########
@@ -273,8 +273,16 @@ public void awaitDrop() throws InterruptedException, IgniteCheckedException {
                 throw new IgniteCheckedException("Failed to wait for NodeAddFinishedMessage");
         }
 
-        /** {@inheritDoc} */

Review comment:
       fixed. i've reviewd code obe more time and i've veririden method instead of overloading. so doc is inherited now.
   TcpDiscoveryConnectionCheckMessage is used as a stub instead of msg which was pased via parameter according to your comment:
   // Replace logic routine message with a stub to update last-sent-time to avoid segmentation on
   // connRecoveryTimeout.




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



[GitHub] [ignite] aleksey-kurinov commented on a change in pull request #7977: Ignite 13040

Posted by GitBox <gi...@apache.org>.
aleksey-kurinov commented on a change in pull request #7977:
URL: https://github.com/apache/ignite/pull/7977#discussion_r465167378



##########
File path: modules/core/src/test/java/org/apache/ignite/internal/IgniteClientRejoinTest.java
##########
@@ -354,12 +354,12 @@ public void testClientsReconnectDisabled() throws Exception {
      */
     private class DiscoverySpi extends TcpDiscoverySpi {
         /** {@inheritDoc} */
-        @Override protected void writeToSocket(Socket sock, TcpDiscoveryAbstractMessage msg, byte[] data,
-            long timeout) throws IOException {
+        @Override protected void writeToSocket(Socket sock, byte[] data,

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



[GitHub] [ignite] aleksey-kurinov commented on a change in pull request #7977: Ignite 13040

Posted by GitBox <gi...@apache.org>.
aleksey-kurinov commented on a change in pull request #7977:
URL: https://github.com/apache/ignite/pull/7977#discussion_r465167204



##########
File path: modules/core/src/test/java/org/apache/ignite/spi/discovery/tcp/TcpDiscoveryPendingMessageDeliveryTest.java
##########
@@ -251,10 +251,10 @@ private void sendDummyCustomMessage(TcpDiscoverySpi disco, IgniteUuid id) {
      */
     private class DyingDiscoverySpi extends TcpDiscoverySpi {
         /** {@inheritDoc} */
-        @Override protected void writeToSocket(Socket sock, TcpDiscoveryAbstractMessage msg, byte[] data,
-            long timeout) throws IOException {
+        @Override protected void writeToSocket(Socket sock, byte[] data,
+                                               long timeout) throws IOException {

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



[GitHub] [ignite] Vladsz83 commented on a change in pull request #7977: Ignite 13040

Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on a change in pull request #7977:
URL: https://github.com/apache/ignite/pull/7977#discussion_r470140435



##########
File path: modules/core/src/test/java/org/apache/ignite/spi/discovery/tcp/TcpClientDiscoverySpiSelfTest.java
##########
@@ -2578,15 +2578,23 @@ private void pauseResumeOperation(boolean isPause, AtomicBoolean... locks) {
                 afterWrite.apply(msg, sock);
         }
 
-        /** {@inheritDoc} */

Review comment:
       I'm afraid, sending TcpDiscoveryConnectionCheckMessage won't help us here. Such trick was involved by me accurately where it could work. Unfortunately, it's not going to work everywhere. DiscoverySpi is quite complex.




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



[GitHub] [ignite] Vladsz83 commented on a change in pull request #7977: Ignite 13040

Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on a change in pull request #7977:
URL: https://github.com/apache/ignite/pull/7977#discussion_r470096717



##########
File path: modules/core/src/test/java/org/apache/ignite/spi/discovery/tcp/TcpDiscoveryCoordinatorFailureTest.java
##########
@@ -282,17 +282,17 @@ public void awaitDrop() throws InterruptedException, IgniteCheckedException {
         /** {@inheritDoc} */
         @Override protected void writeToSocket(
             Socket sock,
-            TcpDiscoveryAbstractMessage msg,
             byte[] data,
             long timeout
         ) throws IOException {
-            if (isDrop(msg)) {
-                // Replace logic routine message with a stub to update last-sent-time to avoid segmentation on
-                // connRecoveryTimeout.
-                msg = new TcpDiscoveryConnectionCheckMessage(locNode);
-            }
 
-            super.writeToSocket(sock, msg, data, timeout);
+            // Replace logic routine message with a stub to update last-sent-time to avoid segmentation on

Review comment:
       @aleksey-kurinov I think we don't need this failure emulation in `FailingDiscoverySpi` any more. It passes no message. This is idea of the ticket. The `msg` is always null. This `writeToSocket` works with `data`. The test goes well without emulation a failure here. Can you check it?




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



[GitHub] [ignite] Vladsz83 commented on a change in pull request #7977: Ignite 13040

Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on a change in pull request #7977:
URL: https://github.com/apache/ignite/pull/7977#discussion_r470098303



##########
File path: modules/core/src/test/java/org/apache/ignite/spi/discovery/tcp/TcpDiscoveryCoordinatorFailureTest.java
##########
@@ -375,7 +375,7 @@ private boolean isDrop(TcpDiscoveryAbstractMessage msg) {
                 }
             }
 
-            if (drop)
+            if (drop && msg != null)

Review comment:
       @aleksey-kurinov , same here. Try removing `FailingDiscoverySpi.writeToSocket(Socket sock, TcpDiscoveryAbstractMessage msg, byte[] data, long timeout )`. Onve we do it, we'd not need this change. Am I right?




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



[GitHub] [ignite] Vladsz83 commented on a change in pull request #7977: Ignite 13040

Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on a change in pull request #7977:
URL: https://github.com/apache/ignite/pull/7977#discussion_r470133039



##########
File path: modules/core/src/test/java/org/apache/ignite/spi/discovery/tcp/TcpClientDiscoverySpiSelfTest.java
##########
@@ -2579,14 +2581,16 @@ private void pauseResumeOperation(boolean isPause, AtomicBoolean... locks) {
         }
 
         /** {@inheritDoc} */
-        @Override protected void writeToSocket(Socket sock, TcpDiscoveryAbstractMessage msg, byte[] msgBytes,

Review comment:
       @aleksey-kurinov , I think we dont need this message-drop emulation at all (`TestTcpDiscoverySpi.oid writeToSocket(Socket sock, byte[] msgBytes, long timeout)`) because `TcpDiscoverySpi` doesn't send the message. It uses raw bites instead. This is the idea of the ticket. But several tests can fail after this change. It might become a challenge to understand what and why the tests simulate and how to fix them.




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



[GitHub] [ignite] Vladsz83 commented on a change in pull request #7977: Ignite 13040

Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on a change in pull request #7977:
URL: https://github.com/apache/ignite/pull/7977#discussion_r470096717



##########
File path: modules/core/src/test/java/org/apache/ignite/spi/discovery/tcp/TcpDiscoveryCoordinatorFailureTest.java
##########
@@ -282,17 +282,17 @@ public void awaitDrop() throws InterruptedException, IgniteCheckedException {
         /** {@inheritDoc} */
         @Override protected void writeToSocket(
             Socket sock,
-            TcpDiscoveryAbstractMessage msg,
             byte[] data,
             long timeout
         ) throws IOException {
-            if (isDrop(msg)) {
-                // Replace logic routine message with a stub to update last-sent-time to avoid segmentation on
-                // connRecoveryTimeout.
-                msg = new TcpDiscoveryConnectionCheckMessage(locNode);
-            }
 
-            super.writeToSocket(sock, msg, data, timeout);
+            // Replace logic routine message with a stub to update last-sent-time to avoid segmentation on

Review comment:
       @aleksey-kurinov I'm sure we don't need this failure-emulation method in `FailingDiscoverySpi` at all because it passes no message. The `msg` is always null. This `writeToSocket` works with `data`. The test works well without emulation a failure here.




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



[GitHub] [ignite] aleksey-kurinov commented on pull request #7977: Ignite 13040

Posted by GitBox <gi...@apache.org>.
aleksey-kurinov commented on pull request #7977:
URL: https://github.com/apache/ignite/pull/7977#issuecomment-675308613


   hank you for review
   I've removed any mesage related code in writeToSocket(Socket sock, byte[] data, long timeout) methods in tests. please take a look.


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



[GitHub] [ignite] aleksey-kurinov commented on a change in pull request #7977: Ignite 13040

Posted by GitBox <gi...@apache.org>.
aleksey-kurinov commented on a change in pull request #7977:
URL: https://github.com/apache/ignite/pull/7977#discussion_r465162513



##########
File path: modules/core/src/test/java/org/apache/ignite/internal/IgniteClientRejoinTest.java
##########
@@ -354,12 +354,12 @@ public void testClientsReconnectDisabled() throws Exception {
      */
     private class DiscoverySpi extends TcpDiscoverySpi {
         /** {@inheritDoc} */
-        @Override protected void writeToSocket(Socket sock, TcpDiscoveryAbstractMessage msg, byte[] data,
-            long timeout) throws IOException {
+        @Override protected void writeToSocket(Socket sock, byte[] data,

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



[GitHub] [ignite] aleksey-kurinov commented on a change in pull request #7977: Ignite 13040

Posted by GitBox <gi...@apache.org>.
aleksey-kurinov commented on a change in pull request #7977:
URL: https://github.com/apache/ignite/pull/7977#discussion_r465167107



##########
File path: modules/core/src/test/java/org/apache/ignite/spi/discovery/tcp/TcpDiscoveryFailedJoinTest.java
##########
@@ -200,10 +200,10 @@ private void assertStartFailed(final String name) {
      */
     private static class DropTcpDiscoverySpi extends TcpDiscoverySpi {
         /** {@inheritDoc} */
-        @Override protected void writeToSocket(Socket sock, TcpDiscoveryAbstractMessage msg, byte[] data,
-            long timeout) throws IOException {
+        @Override protected void writeToSocket(Socket sock, byte[] data,
+                                               long timeout) throws IOException {

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



[GitHub] [ignite] aleksey-kurinov commented on a change in pull request #7977: Ignite 13040

Posted by GitBox <gi...@apache.org>.
aleksey-kurinov commented on a change in pull request #7977:
URL: https://github.com/apache/ignite/pull/7977#discussion_r470566694



##########
File path: modules/core/src/test/java/org/apache/ignite/spi/discovery/tcp/TcpClientDiscoverySpiSelfTest.java
##########
@@ -2578,15 +2578,23 @@ private void pauseResumeOperation(boolean isPause, AtomicBoolean... locks) {
                 afterWrite.apply(msg, sock);
         }
 
-        /** {@inheritDoc} */

Review comment:
       so what's better to use  In this particular case? null? 




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



[GitHub] [ignite] aleksey-kurinov commented on a change in pull request #7977: Ignite 13040

Posted by GitBox <gi...@apache.org>.
aleksey-kurinov commented on a change in pull request #7977:
URL: https://github.com/apache/ignite/pull/7977#discussion_r465166120



##########
File path: modules/core/src/test/java/org/apache/ignite/internal/IgniteDiscoveryMassiveNodeFailTest.java
##########
@@ -301,15 +301,23 @@ public void testRecoveryOnDisconnect() throws Exception {
      *
      */
     private class FailDiscoverySpi extends TcpDiscoverySpi {
-        /** {@inheritDoc} */
-        @Override protected void writeToSocket(Socket sock, TcpDiscoveryAbstractMessage msg, byte[] data,
+        /**
+         * Writes message to the socket.
+         *
+         * @param sock Socket.
+         * @param msg Message.

Review comment:
       fixed. i've reviewd code obe more time and i've veririden method instead of overloading. so doc is inherited now.
   TcpDiscoveryConnectionCheckMessage is used as a stub instead of msg which was pased via parameter according to your comment:
   // Replace logic routine message with a stub to update last-sent-time to avoid segmentation on
   // connRecoveryTimeout.
   




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



[GitHub] [ignite] aleksey-kurinov commented on a change in pull request #7977: Ignite 13040

Posted by GitBox <gi...@apache.org>.
aleksey-kurinov commented on a change in pull request #7977:
URL: https://github.com/apache/ignite/pull/7977#discussion_r465166217



##########
File path: modules/core/src/test/java/org/apache/ignite/internal/processors/cache/distributed/dht/IgniteCacheTopologySplitAbstractTest.java
##########
@@ -217,14 +217,13 @@ protected void checkSegmented(InetSocketAddress sockAddr, long timeout) throws S
 
         /** {@inheritDoc} */
         @Override protected void writeToSocket(
-            Socket sock,
-            TcpDiscoveryAbstractMessage msg,
-            byte[] data,
-            long timeout
+                Socket sock,
+                byte[] data,

Review comment:
       fixed

##########
File path: modules/core/src/test/java/org/apache/ignite/spi/discovery/tcp/BlockTcpDiscoverySpi.java
##########
@@ -74,8 +74,16 @@ private synchronized void apply(ClusterNode addr, TcpDiscoveryAbstractMessage ms
             clo.apply(addr, delegate);
     }
 
-    /** {@inheritDoc} */
-    @Override protected void writeToSocket(
+    /**
+     * Writes message to the socket.
+     *
+     * @param sock Socket.

Review comment:
       fixed. i've reviewd code obe more time and i've veririden method instead of overloading. so doc is inherited now.
   TcpDiscoveryConnectionCheckMessage is used as a stub instead of msg which was pased via parameter according to your comment:
   // Replace logic routine message with a stub to update last-sent-time to avoid segmentation on
   // connRecoveryTimeout.

##########
File path: modules/core/src/test/java/org/apache/ignite/spi/discovery/tcp/TcpClientDiscoverySpiFailureTimeoutSelfTest.java
##########
@@ -449,7 +449,7 @@ public void clientReconnectOnCoordinatorRouterFail(int srvNodes) throws Exceptio
         private Exception err;
 
         /**  */
-        @Override protected void writeToSocket(

Review comment:
       fixed. i've reviewd code obe more time and i've veririden method instead of overloading. so doc is inherited now.
   TcpDiscoveryConnectionCheckMessage is used as a stub instead of msg which was pased via parameter according to your comment:
   // Replace logic routine message with a stub to update last-sent-time to avoid segmentation on
   // connRecoveryTimeout.

##########
File path: modules/core/src/test/java/org/apache/ignite/spi/discovery/tcp/TcpClientDiscoverySpiSelfTest.java
##########
@@ -2578,15 +2578,23 @@ private void pauseResumeOperation(boolean isPause, AtomicBoolean... locks) {
                 afterWrite.apply(msg, sock);
         }
 
-        /** {@inheritDoc} */

Review comment:
       fixed. i've reviewd code obe more time and i've veririden method instead of overloading. so doc is inherited now.
   TcpDiscoveryConnectionCheckMessage is used as a stub instead of msg which was pased via parameter according to your comment:
   // Replace logic routine message with a stub to update last-sent-time to avoid segmentation on
   // connRecoveryTimeout.

##########
File path: modules/core/src/test/java/org/apache/ignite/spi/discovery/tcp/TcpDiscoveryFailedJoinTest.java
##########
@@ -200,10 +200,10 @@ private void assertStartFailed(final String name) {
      */
     private static class DropTcpDiscoverySpi extends TcpDiscoverySpi {
         /** {@inheritDoc} */
-        @Override protected void writeToSocket(Socket sock, TcpDiscoveryAbstractMessage msg, byte[] data,
-            long timeout) throws IOException {
+        @Override protected void writeToSocket(Socket sock, byte[] data,
+                                               long timeout) throws IOException {

Review comment:
       fixed

##########
File path: modules/core/src/test/java/org/apache/ignite/spi/discovery/tcp/TcpDiscoveryPendingMessageDeliveryTest.java
##########
@@ -251,10 +251,10 @@ private void sendDummyCustomMessage(TcpDiscoverySpi disco, IgniteUuid id) {
      */
     private class DyingDiscoverySpi extends TcpDiscoverySpi {
         /** {@inheritDoc} */
-        @Override protected void writeToSocket(Socket sock, TcpDiscoveryAbstractMessage msg, byte[] data,
-            long timeout) throws IOException {
+        @Override protected void writeToSocket(Socket sock, byte[] data,
+                                               long timeout) throws IOException {

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



[GitHub] [ignite] Vladsz83 commented on a change in pull request #7977: Ignite 13040

Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on a change in pull request #7977:
URL: https://github.com/apache/ignite/pull/7977#discussion_r461397699



##########
File path: modules/core/src/test/java/org/apache/ignite/internal/IgniteClientRejoinTest.java
##########
@@ -354,12 +354,12 @@ public void testClientsReconnectDisabled() throws Exception {
      */
     private class DiscoverySpi extends TcpDiscoverySpi {
         /** {@inheritDoc} */
-        @Override protected void writeToSocket(Socket sock, TcpDiscoveryAbstractMessage msg, byte[] data,
-            long timeout) throws IOException {
+        @Override protected void writeToSocket(Socket sock, byte[] data,

Review comment:
       @aleksey-kurinov , should it get on-lined?

##########
File path: modules/core/src/test/java/org/apache/ignite/internal/IgniteDiscoveryMassiveNodeFailTest.java
##########
@@ -301,15 +301,23 @@ public void testRecoveryOnDisconnect() throws Exception {
      *
      */
     private class FailDiscoverySpi extends TcpDiscoverySpi {
-        /** {@inheritDoc} */
-        @Override protected void writeToSocket(Socket sock, TcpDiscoveryAbstractMessage msg, byte[] data,
+        /**
+         * Writes message to the socket.
+         *
+         * @param sock Socket.
+         * @param msg Message.

Review comment:
       @aleksey-kurinov , why replaced the inherited doc?

##########
File path: modules/core/src/test/java/org/apache/ignite/internal/processors/cache/distributed/dht/IgniteCacheTopologySplitAbstractTest.java
##########
@@ -217,14 +217,13 @@ protected void checkSegmented(InetSocketAddress sockAddr, long timeout) throws S
 
         /** {@inheritDoc} */
         @Override protected void writeToSocket(
-            Socket sock,
-            TcpDiscoveryAbstractMessage msg,
-            byte[] data,
-            long timeout
+                Socket sock,
+                byte[] data,

Review comment:
       @aleksey-kurinov , this change looks like extra tabulation.

##########
File path: modules/core/src/test/java/org/apache/ignite/spi/discovery/tcp/BlockTcpDiscoverySpi.java
##########
@@ -74,8 +74,16 @@ private synchronized void apply(ClusterNode addr, TcpDiscoveryAbstractMessage ms
             clo.apply(addr, delegate);
     }
 
-    /** {@inheritDoc} */
-    @Override protected void writeToSocket(
+    /**
+     * Writes message to the socket.
+     *
+     * @param sock Socket.

Review comment:
       Why replaced the inherited doc?

##########
File path: modules/core/src/test/java/org/apache/ignite/spi/discovery/tcp/TcpClientDiscoverySpiFailureTimeoutSelfTest.java
##########
@@ -449,7 +449,7 @@ public void clientReconnectOnCoordinatorRouterFail(int srvNodes) throws Exceptio
         private Exception err;
 
         /**  */
-        @Override protected void writeToSocket(

Review comment:
       Why removed `@Override` ?

##########
File path: modules/core/src/test/java/org/apache/ignite/spi/discovery/tcp/TcpClientDiscoverySpiSelfTest.java
##########
@@ -2578,15 +2578,23 @@ private void pauseResumeOperation(boolean isPause, AtomicBoolean... locks) {
                 afterWrite.apply(msg, sock);
         }
 
-        /** {@inheritDoc} */

Review comment:
       Why replaced the inherited doc?

##########
File path: modules/core/src/test/java/org/apache/ignite/spi/discovery/tcp/TcpDiscoveryCoordinatorFailureTest.java
##########
@@ -273,8 +273,16 @@ public void awaitDrop() throws InterruptedException, IgniteCheckedException {
                 throw new IgniteCheckedException("Failed to wait for NodeAddFinishedMessage");
         }
 
-        /** {@inheritDoc} */

Review comment:
       Why replaced the inherited doc?

##########
File path: modules/core/src/test/java/org/apache/ignite/spi/discovery/tcp/TcpDiscoveryFailedJoinTest.java
##########
@@ -200,10 +200,10 @@ private void assertStartFailed(final String name) {
      */
     private static class DropTcpDiscoverySpi extends TcpDiscoverySpi {
         /** {@inheritDoc} */
-        @Override protected void writeToSocket(Socket sock, TcpDiscoveryAbstractMessage msg, byte[] data,

Review comment:
       Looks like to be on-lined.

##########
File path: modules/core/src/test/java/org/apache/ignite/spi/discovery/tcp/TcpDiscoveryFailedJoinTest.java
##########
@@ -200,10 +200,10 @@ private void assertStartFailed(final String name) {
      */
     private static class DropTcpDiscoverySpi extends TcpDiscoverySpi {
         /** {@inheritDoc} */
-        @Override protected void writeToSocket(Socket sock, TcpDiscoveryAbstractMessage msg, byte[] data,
-            long timeout) throws IOException {
+        @Override protected void writeToSocket(Socket sock, byte[] data,
+                                               long timeout) throws IOException {

Review comment:
       Looks like to be on-lined.

##########
File path: modules/core/src/test/java/org/apache/ignite/spi/discovery/tcp/TcpDiscoveryPendingMessageDeliveryTest.java
##########
@@ -251,10 +251,10 @@ private void sendDummyCustomMessage(TcpDiscoverySpi disco, IgniteUuid id) {
      */
     private class DyingDiscoverySpi extends TcpDiscoverySpi {
         /** {@inheritDoc} */
-        @Override protected void writeToSocket(Socket sock, TcpDiscoveryAbstractMessage msg, byte[] data,
-            long timeout) throws IOException {
+        @Override protected void writeToSocket(Socket sock, byte[] data,
+                                               long timeout) throws IOException {

Review comment:
       Looks like to be on-lined.




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