Custom TCP Java proxy socket load balancer

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 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());
DataIn

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());
        }
      }

StackExchange Code Review Q#73684, answer score: 4