Qpid
  1. Qpid
  2. QPID-4033

[Java client] add a connection id to the session dispatcher thread name to avoid name overloading

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.17
    • Component/s: Java Client
    • Labels:
      None

      Description

      The session dispatcher thread is currently called "Dispatcher-Channel-<channel number>", this leads to name overloading if there is mroe than one connection present as the channel numbers are not unique on each connection. We should add a connection id number to allow differentiating the names of dispatcher threads on different connections.

        Issue Links

          Activity

          Hide
          Rob Godfrey added a comment -

          By connection-id do you mean the remote IP addr/hostname and port, and local port?
          The string "Channel" also seems redundant in the name

          Show
          Rob Godfrey added a comment - By connection-id do you mean the remote IP addr/hostname and port, and local port? The string "Channel" also seems redundant in the name
          Hide
          Robbie Gemmell added a comment - - edited

          No, I just mean something that allows differentiating between the same channel numbers in use on different connections, as opposed to something that really identifies the TCP connection.

          Host:port has the potential to be very long, and has additional issues such as changing due to failover occurring etc etc, which would require updating the thread names after the fact. I'm not really looking to get into that, so much as provide a means to separate one connection from another, wherever it may be connected. In this case I just added (missed the commit?) a long value that is incremented with each new AMQConnection instance created, and is logged (at debug, could make it info?) along with the connection URL provided (which includes user, and any host:port combinations provided for failover) to allow at least some path for further identification.

          The new names are currently e.g Dispatcher-Channel-1-Conn-1. I guess Channel (and now Conn) is a little redundant I guess, I just left the qualifiers because it makes it easier to distinguish what each number is what without looking up the code. "Dispatcher-1-Conn-1" instead perhaps?

          Show
          Robbie Gemmell added a comment - - edited No, I just mean something that allows differentiating between the same channel numbers in use on different connections, as opposed to something that really identifies the TCP connection. Host:port has the potential to be very long, and has additional issues such as changing due to failover occurring etc etc, which would require updating the thread names after the fact. I'm not really looking to get into that, so much as provide a means to separate one connection from another, wherever it may be connected. In this case I just added (missed the commit?) a long value that is incremented with each new AMQConnection instance created, and is logged (at debug, could make it info?) along with the connection URL provided (which includes user, and any host:port combinations provided for failover) to allow at least some path for further identification. The new names are currently e.g Dispatcher-Channel-1-Conn-1. I guess Channel (and now Conn) is a little redundant I guess, I just left the qualifiers because it makes it easier to distinguish what each number is what without looking up the code. "Dispatcher-1-Conn-1" instead perhaps?
          Hide
          Rob Godfrey added a comment -

          I guess it really depends where we thinkn this will be most useful... If you have full debug logs then just the numbers are fine... if you are trying to do any sort of post incident analysis based on just an exception stack trace in the log, then having all the information available to you is pretty invaluable. Accept that the length might be an issue though I would think that Disp-127.0.0.1:1234/192.168.1.1:5672[1] would be short enough.

          Accept the point about failover

          Show
          Rob Godfrey added a comment - I guess it really depends where we thinkn this will be most useful... If you have full debug logs then just the numbers are fine... if you are trying to do any sort of post incident analysis based on just an exception stack trace in the log, then having all the information available to you is pretty invaluable. Accept that the length might be an issue though I would think that Disp-127.0.0.1:1234/192.168.1.1:5672 [1] would be short enough. Accept the point about failover
          Hide
          Robbie Gemmell added a comment -

          I agree having all the information sounds nice, but it also seems to make things a bit convoluted. I think we would still need to add the 'connection number' identifier I just introduced, because the local port information above is going to change when failover occurs and that would leave it complete pain to track a Dispatcher within a complete log set. I think I still prefer the current simple name (dropping the "Channel").

          (Keith also points out that IPv6 addresses would possibly make things entertaining too).

          Show
          Robbie Gemmell added a comment - I agree having all the information sounds nice, but it also seems to make things a bit convoluted. I think we would still need to add the 'connection number' identifier I just introduced, because the local port information above is going to change when failover occurs and that would leave it complete pain to track a Dispatcher within a complete log set. I think I still prefer the current simple name (dropping the "Channel"). (Keith also points out that IPv6 addresses would possibly make things entertaining too).
          Hide
          Rob Godfrey added a comment -

          not a huge issue for me... but I find that in general our use of identifiers that can't easily be linked back to anything the user / operational staff can identify is less than ideal.

          Show
          Rob Godfrey added a comment - not a huge issue for me... but I find that in general our use of identifiers that can't easily be linked back to anything the user / operational staff can identify is less than ideal.
          Hide
          Robbie Gemmell added a comment -

          I agree on that, but I think thats a general logging problem, not a thread name problem.

          Show
          Robbie Gemmell added a comment - I agree on that, but I think thats a general logging problem, not a thread name problem.
          Hide
          Robbie Gemmell added a comment -

          I have added logging via QPID-4038 to display the connection id number and associated local+remote addresses after each [re]connection.

          Show
          Robbie Gemmell added a comment - I have added logging via QPID-4038 to display the connection id number and associated local+remote addresses after each [re] connection.
          Hide
          Robbie Gemmell added a comment -

          Keith, can you review please?

          Thanks,
          Robbie

          Show
          Robbie Gemmell added a comment - Keith, can you review please? Thanks, Robbie
          Hide
          Keith Wall added a comment -

          Change look good, no comments.

          Show
          Keith Wall added a comment - Change look good, no comments.

            People

            • Assignee:
              Keith Wall
              Reporter:
              Robbie Gemmell
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development