8
\$\begingroup\$

I'm working on implementing a custom TCP proxy server which acts like a load balancer. The proxy server will accept client requests and then forward them to available hosts.

I am concerned about the way that I am creating new client sockets between proxy server and host per request in the receiveHostResponse method and that if it throws a connection refused exception then I retry with another host. Is it okay to create new client sockets between proxy and hosts per request or should I create them once per process? I think there would be a problem with concurrency if I tried once-per-process.

What are some best practises here?

 public class ProxyServer extends Thread {

    final static Logger logger = Logger.getLogger(ProxyServer.class);

    private ServerSocket serverSocket;
    private static ArrayList<HostConnections> hostList;
    private int proxyServerPort;

    public ProxyServer( ) throws IOException {

      initialize();

    }

    /**
     * This method will initialize all the prerequisites
     * @throws IOException
     */
    private void initialize() throws IOException {

      PropertyConfigurator.configure("log4j.properties");
      logger.info("Successfully configured log4j. ");
      logger.info("************************* Started Initializing Proxy Server *************************");
      int port = getProxyServerPort();
      serverSocket = new ServerSocket(port);
      logger.info("Started Proxy Server on Port " + port);

      loadHostConfigurations();

      logger.info(
          "************************* Successfully initialized Proxy Server **********************");
    }

    /**
     * This is the runner which takes the socket server requests
     */
    public void run() {
      while (true) {
        try {

          logger.info("Waiting for client on port " + serverSocket.getLocalPort() + "...");
          Socket server = serverSocket.accept();

          logger.info("Just connected to " + server.getRemoteSocketAddress());
          DataInputStream in = new DataInputStream(server.getInputStream());

          ///receive client request message ()
          String requestMessage = in.readUTF();

          logger.info(requestMessage);

          /////////////////////////////////////////////////////////////////////////////////////////////
          // Chose available host by round robin
          HostResponse hostResponse = null;
          boolean reTry=false;
          while(!reTry) {

            HostConnections availableHost = getAvailableHostRoundRobin();

            // send host request X


            try {
              hostResponse = null;
              hostResponse = recieveHostResponse(availableHost, requestMessage);
              reTry=true;
            } catch (CCRuntimeException e) {
              logger.error("############" + e.getMessage());
              reTry=false;
            }


          }


          //////////////////////////////////////////////////////////////////////
          DataOutputStream out = new DataOutputStream(server.getOutputStream());

          // send recieved response from host X to client
          if(hostResponse!=null) {

            out.writeUTF(hostResponse.getMessage());

          }

          out.flush();
          out.close();
          in.close();

          server.close();

          logger.info("Server proxy.isClosed()    " + server.isClosed());

        } catch (SocketTimeoutException s) {

          logger.error("############Socket timed out!" + s.getMessage() + "Cause " + s.getCause());

        } catch (IOException e) {
          e.printStackTrace();

        }
      }
    }

    /**
     * This method will return the current available select by round robin way
     *
     * @return
     */
    private HostConnections getAvailableHostRoundRobin()   {

      HostConnections availableHost = hostList.get(0);
      //availableHost.isMarked();//==false;
      Collections.rotate(hostList, -1);

      logger.info("Found an Available host " + hostList.get(0));

      return availableHost;
    }


    public static void main(String[] args) {
      //int port = 2222;
      try {
        Thread t = new ProxyServer();
        t.start();
      } catch (IOException e) {
        e.printStackTrace();
      }
    }

    /**
     * This method will send client message to the selected host and will return the
     * host response
     *
     * @param availableHost
     * @param request
     * @return
     * @throws CCRuntimeException
     */
    public static HostResponse recieveHostResponse(HostConnections availableHost, String request)
        throws CCRuntimeException {

      logger.info("Invoked recieveHostResponse () :  " + availableHost.toString());

      HostResponse hostResponse = new HostResponse();

      String serverName = availableHost.getHostIpAddress();
      int port = availableHost.getHostPort();


      try {


        logger.info("Connecting to " + serverName + " on port " + port);
        Socket client = new Socket(serverName, port);

        logger.info("Just connected to " + client.getRemoteSocketAddress());

        OutputStream outToServer = client.getOutputStream();
        DataOutputStream out = new DataOutputStream(outToServer);

        logger.info("Sent Host Request " + request);
        out.writeUTF(request);
        out.flush();

        InputStream inFromServer = client.getInputStream();
        DataInputStream in = new DataInputStream(inFromServer);
        hostResponse.setMessage(in.readUTF());

        logger.info("Recieved host response " + hostResponse.getMessage());


        out.close();
        client.close();

        hostResponse.setSuccess(true);

      } catch (IOException e) {
        hostResponse.setSuccess(false);

        logger.error("############" + e.getMessage());
        throw  new CCRuntimeException(" Host is not reachable :" + availableHost.toString());
      }

      return hostResponse;

    }

    /**
     * This method will load all the host configurations which are included in conf.properties file
     *
     * host configurations has to be configured as
     * host_ipaddress1=192.168.60.65
     * host_port1=7881
     * host_ipaddress2=192.168.60.65
     * host_port2=7882
     *
     * new host ip and host ports has to appended to the above pattern meaning host_ipaddress+{++1} host_port+[++1]
     *
     * */
    private static void loadHostConfigurations() {

      logger.info("Invoked loadHostConfigurations () method . ");

      hostList = new ArrayList<HostConnections>();

      int hostCounter = 1;
      boolean hasHosts = true;

      while (hasHosts) {
        String hostIp = DataProperties.getInstance().getValue("host_ipaddress" + hostCounter);
        String hostPort = DataProperties.getInstance().getValue("host_port" + hostCounter);

        if (hostIp != null && hostPort != null) {
          hostList.add(new HostConnections(hostIp, Integer.valueOf(hostPort)));
          logger.info(" Loaded Host " + hostIp + " Port " + hostPort);
          hostCounter++;

        } else {
          hasHosts = false;

        }
      }

      logger.info("Number of host connections Loaded : " + (hostCounter-1));
      logger.info("Loaded Host Connections " + hostList.toString());
    }

    /**
     *
     * This method will return proxy server port which is configured on conf.properties file
     * if the port is not configured on the property file then this will return the default port 4444
     * @return
     */
    public int getProxyServerPort() {

      String sPort=DataProperties.getInstance().getValue(Constants.SERVER_PORT_KEY);

      if(sPort!=null){

        return Integer.valueOf(sPort);
      }

      return Constants.DEFAULT_SERVER_PORT;
    }
}
\$\endgroup\$
  • \$\begingroup\$ And what are your concerns? Which part of the code are you dubious about or think might be improved in some way? \$\endgroup\$ – itsbruce Dec 15 '14 at 9:57
  • \$\begingroup\$ @itsbruce i have doubt that im creating new client sockets between proxy server and host per request in receive host response method. and if it throws connection refused exception then i retry with another host .Is this ok to create new client sockets between proxy and hosts per request or create them once and process but then i think there will be a problem with concurrency what do you think \$\endgroup\$ – Harsh Dec 15 '14 at 10:13
  • \$\begingroup\$ For a start, I'd leave it at creating the connections. But there indeed is an issue with concurrency: You don't have it. You handle one client at a time. That will make your proxy a bottleneck rather than a load balancer. \$\endgroup\$ – Fildor Dec 16 '14 at 13:33
4
\$\begingroup\$

Thread

Overriding Thread is an anti-pattern. Your class is not a Thread, but something that can be run on a thread. You should implement Runnable instead, and load your Runnable on to a thread.

public class ProxyServer implements Runnable {
    @Override
    public void run() {
        ...
    }
}

Initialization

Your ServerSocket should be a private-final instance field. Right now it is not final. Same with the port. The problem is that you have added an initialize() method instead od your constructor. Put your initialization in the constructor where it belongs, and it reduces a number of other problems.

Static Hosts

You maintain a static list of hosts. This is modified without any regard for concurrent access. If you have multiple ProxyServer instances then they will overwrite each other's host list. Since you have only one proxy server, you should just make the host list an instance (rather than a static) field.

Threading

As was mentioned in the comments, the connection you accept is processed in the same thread as the server. As a result, you cannot be listening for new connections while also processing a proxy.

Additionally, your reTry is fishy:

      boolean reTry=false;
      while(!reTry) {

        HostConnections availableHost = getAvailableHostRoundRobin();

        // send host request X
        try {
          hostResponse = null;
          hostResponse = recieveHostResponse(availableHost, requestMessage);
          reTry=true;
        } catch (CCRuntimeException e) {
          logger.error("############" + e.getMessage());
          reTry=false;
        }
      }

This code sets reTry to false, and then keeps retrying until it works, but, the boolean sense of the variable is hinky. You should retry when retry is true.

Right now the code read-sense id different to the actual logic.

      boolean reTry=true;
      while(reTry) {

        HostConnections availableHost = getAvailableHostRoundRobin();

        retry = true;
        try {
          hostResponse = recieveHostResponse(availableHost, requestMessage);
          // no need to retry something that worked.
          reTry=false;
        } catch (CCRuntimeException e) {
          logger.error("############" + e.getMessage());
        }
      }
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.