MINA SSHD
  1. MINA SSHD
  2. SSHD-136

Reduce consumption of ClientInputStreamPump threads and NioProcessor threads.

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Won't Fix
    • Affects Version/s: 0.5.0
    • Fix Version/s: None
    • Labels:
      None

      Description

      When using the library to work with 1000 concurrent ssh connections, it takes 1000 ClientInputStreamPump threads. 1 thread for 1 channel. It would be nice to be able to configure this behavior, for example use 1 ClientInputStreamPump for all channels of sshclient or session.

      1. src.zip
        242 kB
        Alexey Polbitsyn
      2. src-diff.zip
        6 kB
        Alexey Polbitsyn
      3. SSHD-136.patch
        22 kB
        Guillaume Nodet

        Issue Links

          Activity

          Hide
          Alexey Polbitsyn added a comment - - edited

          possible solution:

          src-diff - diff files
          src - sources

          the ability to customize count of NioProcessor threads before start SshClient:
          SshClient.setNioProcessorCount
          the ability to customize count of ClientInputStreamPump threads.
          1) 1 ClientInputStreamPump on channel
          2) 1 client ClientInputStreamPump on session
          3) 1 client ClientInputStreamPump on SshClient
          4) turn off automatically pumping

          public enum PumpingMethod

          { SELF,/* class will pump streams/stream */ PARENT,/* delegating pumping to parent code(session for channel,client for session,external code for client) OFF /* torn off auto pumping */ }

          add public API to channel, session and client:
          public boolean pump(); // try to pump input stream for channel, pump all channels with PumpingMethod=PARENT for session, pump all sessions with PumpingMethod=PARENT for client. Return true if at least one stream was pumped.
          public void setPumpingMethod(PumpingMethod pumpingMethod) - set pumping method
          public PumpingMethod getPumpingMethod()

          add implementation for SshClient pumping:
          use one thread for pumping all sessions with PumpingMethod=PARENT and accordingly all channels with PumpingMethod=PARENT

          Show
          Alexey Polbitsyn added a comment - - edited possible solution: src-diff - diff files src - sources the ability to customize count of NioProcessor threads before start SshClient: SshClient.setNioProcessorCount the ability to customize count of ClientInputStreamPump threads. 1) 1 ClientInputStreamPump on channel 2) 1 client ClientInputStreamPump on session 3) 1 client ClientInputStreamPump on SshClient 4) turn off automatically pumping public enum PumpingMethod { SELF,/* class will pump streams/stream */ PARENT,/* delegating pumping to parent code(session for channel,client for session,external code for client) OFF /* torn off auto pumping */ } add public API to channel, session and client: public boolean pump(); // try to pump input stream for channel, pump all channels with PumpingMethod=PARENT for session, pump all sessions with PumpingMethod=PARENT for client. Return true if at least one stream was pumped. public void setPumpingMethod(PumpingMethod pumpingMethod) - set pumping method public PumpingMethod getPumpingMethod() add implementation for SshClient pumping: use one thread for pumping all sessions with PumpingMethod=PARENT and accordingly all channels with PumpingMethod=PARENT
          Hide
          Guillaume Nodet added a comment -

          Reworked patch with a single file.

          Show
          Guillaume Nodet added a comment - Reworked patch with a single file.
          Hide
          Guillaume Nodet added a comment -

          I wonder if there is any need to have the pump method configurable. Why not simply moving everything to the session instead ?

          Show
          Guillaume Nodet added a comment - I wonder if there is any need to have the pump method configurable. Why not simply moving everything to the session instead ?
          Hide
          Guillaume Nodet added a comment -

          One real problem is that such a solution leads to requirements on the InputStream / OutputStreams used: the InputStream#available() method has to report accurate data, but more importantantly, there's no way to know when the stream is finished or not.

          
              public boolean pump() {
                  try {
                      if (!closeFuture.isClosed()) {
                          int len = Math.min(in.available(), remoteWindow.getPacketSize());
                          if (len > 0 && remoteWindow.consumeIfAvailable(len)) {
                              Buffer buffer = session.createBuffer(SshConstants.Message.SSH_MSG_CHANNEL_DATA, 0);
                              buffer.putInt(recipient);
                              buffer.putInt(len);
                              int wpos = buffer.wpos(); // keep buffer position for data write
                              buffer.wpos(wpos + remoteWindow.getPacketSize()); // Make room
                              securedRead(in, buffer.array(), wpos, len); // read data into buffer
                              log.debug("Send SSH_MSG_CHANNEL_DATA on channel {}", id);
                              session.writePacket(buffer);
                              return true;
                          }
                      }
                  } catch (Exception e) {
                      if (!closing) {
                          log.info("Caught exception", e);
                          close(false);
                      }
                  }
                  return false;
              }
          

          The above code has the mentioned problem, because eof() is not sent anymore.

          I think for the proposed logic to work, we need to use enhanced interfaces instead of InputStream / OutputStream (making sure we have a way to detect real eof).

          Show
          Guillaume Nodet added a comment - One real problem is that such a solution leads to requirements on the InputStream / OutputStreams used: the InputStream#available() method has to report accurate data, but more importantantly, there's no way to know when the stream is finished or not. public boolean pump() { try { if (!closeFuture.isClosed()) { int len = Math .min(in.available(), remoteWindow.getPacketSize()); if (len > 0 && remoteWindow.consumeIfAvailable(len)) { Buffer buffer = session.createBuffer(SshConstants.Message.SSH_MSG_CHANNEL_DATA, 0); buffer.putInt(recipient); buffer.putInt(len); int wpos = buffer.wpos(); // keep buffer position for data write buffer.wpos(wpos + remoteWindow.getPacketSize()); // Make room securedRead(in, buffer.array(), wpos, len); // read data into buffer log.debug( "Send SSH_MSG_CHANNEL_DATA on channel {}" , id); session.writePacket(buffer); return true ; } } } catch (Exception e) { if (!closing) { log.info( "Caught exception" , e); close( false ); } } return false ; } The above code has the mentioned problem, because eof() is not sent anymore. I think for the proposed logic to work, we need to use enhanced interfaces instead of InputStream / OutputStream (making sure we have a way to detect real eof).
          Hide
          Guillaume Nodet added a comment -

          I wonder if we could somehow leverage nio channels with the Pipe class and a selector instead of spawning threads.
          We could maybe keep both APIs or a simple wrapper to ease the work for the user using blocking io.

          Show
          Guillaume Nodet added a comment - I wonder if we could somehow leverage nio channels with the Pipe class and a selector instead of spawning threads. We could maybe keep both APIs or a simple wrapper to ease the work for the user using blocking io.
          Hide
          Guillaume Nodet added a comment -

          Using the inverted input stream get rids of the problem.

          Show
          Guillaume Nodet added a comment - Using the inverted input stream get rids of the problem.

            People

            • Assignee:
              Guillaume Nodet
              Reporter:
              Alexey Polbitsyn
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development