Skip to main content
Notice removed Canonical answer required by CommunityBot
Bounty Ended with no winning answer by CommunityBot
Tweeted twitter.com/StackCodeReview/status/945947324085350400
Notice added Canonical answer required by david
Bounty Started worth 50 reputation by david
edited tags
Link
david
  • 1.9k
  • 8
  • 32
  • 48
deleted 54 characters in body
Source Link
Jamal
  • 34.9k
  • 13
  • 133
  • 237

I have around 60 sockets, 20 business threads and 1 pinger timer thread and I want to make sure each of those thread works on different socket everytimeevery time so I don't want to share same socket between two threads at all.

Now all the business threads will call getNextSocket() method of the SocketManager class concurrently to get the next live available socket to send the business message on that socket. And business threads are sending data very fast, just fyiFYI. So I have two types of messages which I am sending on a socket:

Below is my SocketManager class:

And here is my SendToQueue class:

  // this method will be called by multiple business threads (around 20) concurrently to send the data
  public boolean sendAsync(final long address, final byte[] encodedRecords) {
    PendingMessage m = new PendingMessage(address, encodedRecords, true);
    cache.put(address, m);
    return doSendAsync(m);
  }

  private boolean doSendAsync(final PendingMessage pendingMessage) {
    Optional<SocketHolder> liveSocket = SocketManager.getInstance().getNextSocket();
    if (!liveSocket.isPresent()) {
      return false;
    }       
    ZMsg msg = new ZMsg();
    msg.add(pendingMessage.getEncodedRecords());
    try {
      // send data on a socket
      // this returns instantly which doesn't mean that socket is live or dead
      // so we can't rely on this boolean to check whether socket is live or dead.
      // using socket as its own lock
      synchronized (socket) {
        return msg.send(liveSocket.get().getSocket());
      }
    } finally {
      msg.destroy();
    }
  }

  public boolean send(final long address, final byte[] encodedRecords, final Socket socket) {
    PendingMessage m = new PendingMessage(address, encodedRecords, socket, false);
    cache.put(address, m);
    try {
      if (doSendAsync(m, socket)) {
        return m.waitForAck();
      }
      return false;
    } finally {
      cache.invalidate(address);
    }
  }
  

Problem Statement

One solution I can think of is using synchronization on a socket while sending the data but if many threads uses the same socket, resources aren't well utilized. Moreover If, if msg.send(socket); is blocked (technically it shouldn't) all threads waiting for this socket are blocked. So I guess there might be a better way to ensure that every thread uses a different single live socket at the same time instead of synchronization on a particular socket.

In my above answer, I am synchronizing on the socket both from pinger timer thread and business thread but it may not be the optimal solution as I mentioned above. As my above code is working fine so opting for code review to see if there is any better way to do this.

Note: I am working with Java7Java 7.

I have around 60 sockets, 20 business threads and 1 pinger timer thread and I want to make sure each of those thread works on different socket everytime so I don't want to share same socket between two threads at all.

Now all the business threads will call getNextSocket() method of SocketManager class concurrently to get the next live available socket to send the business message on that socket. And business threads are sending data very fast, just fyi. So I have two types of messages which I am sending on a socket:

Below is my SocketManager class:

And here is my SendToQueue class:

  // this method will be called by multiple business threads (around 20) concurrently to send the data
  public boolean sendAsync(final long address, final byte[] encodedRecords) {
    PendingMessage m = new PendingMessage(address, encodedRecords, true);
    cache.put(address, m);
    return doSendAsync(m);
  }

  private boolean doSendAsync(final PendingMessage pendingMessage) {
    Optional<SocketHolder> liveSocket = SocketManager.getInstance().getNextSocket();
    if (!liveSocket.isPresent()) {
      return false;
    }       
    ZMsg msg = new ZMsg();
    msg.add(pendingMessage.getEncodedRecords());
    try {
      // send data on a socket
      // this returns instantly which doesn't mean that socket is live or dead
      // so we can't rely on this boolean to check whether socket is live or dead.
      // using socket as its own lock
      synchronized (socket) {
        return msg.send(liveSocket.get().getSocket());
      }
    } finally {
      msg.destroy();
    }
  }

  public boolean send(final long address, final byte[] encodedRecords, final Socket socket) {
    PendingMessage m = new PendingMessage(address, encodedRecords, socket, false);
    cache.put(address, m);
    try {
      if (doSendAsync(m, socket)) {
        return m.waitForAck();
      }
      return false;
    } finally {
      cache.invalidate(address);
    }
  }
  

Problem Statement

One solution I can think of is using synchronization on a socket while sending the data but if many threads uses the same socket, resources aren't well utilized. Moreover If msg.send(socket); is blocked (technically it shouldn't) all threads waiting for this socket are blocked. So I guess there might be a better way to ensure that every thread uses a different single live socket at the same time instead of synchronization on a particular socket.

In my above answer, I am synchronizing on the socket both from pinger timer thread and business thread but it may not be the optimal solution as I mentioned above. As my above code is working fine so opting for code review to see if there is any better way to do this.

Note: I am working with Java7.

I have around 60 sockets, 20 business threads and 1 pinger timer thread and I want to make sure each of those thread works on different socket every time so I don't want to share same socket between two threads at all.

Now all the business threads will call getNextSocket() method of the SocketManager class concurrently to get the next live available socket to send the business message on that socket. And business threads are sending data very fast, just FYI. So I have two types of messages which I am sending on a socket:

SocketManager class:

SendToQueue class:

  // this method will be called by multiple business threads (around 20) concurrently to send the data
  public boolean sendAsync(final long address, final byte[] encodedRecords) {
    PendingMessage m = new PendingMessage(address, encodedRecords, true);
    cache.put(address, m);
    return doSendAsync(m);
  }

  private boolean doSendAsync(final PendingMessage pendingMessage) {
    Optional<SocketHolder> liveSocket = SocketManager.getInstance().getNextSocket();
    if (!liveSocket.isPresent()) {
      return false;
    }       
    ZMsg msg = new ZMsg();
    msg.add(pendingMessage.getEncodedRecords());
    try {
      // send data on a socket
      // this returns instantly which doesn't mean that socket is live or dead
      // so we can't rely on this boolean to check whether socket is live or dead.
      // using socket as its own lock
      synchronized (socket) {
        return msg.send(liveSocket.get().getSocket());
      }
    } finally {
      msg.destroy();
    }
  }

  public boolean send(final long address, final byte[] encodedRecords, final Socket socket) {
    PendingMessage m = new PendingMessage(address, encodedRecords, socket, false);
    cache.put(address, m);
    try {
      if (doSendAsync(m, socket)) {
        return m.waitForAck();
      }
      return false;
    } finally {
      cache.invalidate(address);
    }
  }

One solution I can think of is using synchronization on a socket while sending the data but if many threads uses the same socket, resources aren't well utilized. Moreover, if msg.send(socket); is blocked (technically it shouldn't) all threads waiting for this socket are blocked. So I guess there might be a better way to ensure that every thread uses a different single live socket at the same time instead of synchronization on a particular socket.

In my above answer, I am synchronizing on the socket both from pinger timer thread and business thread but it may not be the optimal solution as I mentioned above. As my code is working fine so opting for code review to see if there is any better way to do this.

Note: I am working with Java 7.

edited title
Link
david
  • 1.9k
  • 8
  • 32
  • 48

How to make sure that I am not sharing same Send data on a socket between twofrom multiple threads at same time?

Source Link
david
  • 1.9k
  • 8
  • 32
  • 48
Loading