Uploaded image for project: '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
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.17
    • Component/s: JMS AMQP 0-x
    • 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
          rgodfrey 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
          rgodfrey 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
          gemmellr 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
          gemmellr 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
          rgodfrey 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
          rgodfrey 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
          gemmellr 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
          gemmellr 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
          rgodfrey 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
          rgodfrey 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
          gemmellr Robbie Gemmell added a comment -

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

          Show
          gemmellr Robbie Gemmell added a comment - I agree on that, but I think thats a general logging problem, not a thread name problem.
          Hide
          gemmellr 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
          gemmellr 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
          gemmellr Robbie Gemmell added a comment -

          Keith, can you review please?

          Thanks,
          Robbie

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

          Change look good, no comments.

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

            People

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

              Dates

              • Created:
                Updated:
                Resolved:

                Development