Qpid
  1. Qpid
  2. QPID-4124

[Java Broker] TransactionTimeout logging has duplication and some erroneously uses default Object.toString

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Trivial Trivial
    • Resolution: Fixed
    • Affects Version/s: 0.14, 0.15, 0.16, 0.17, 0.18
    • Fix Version/s: 0.19
    • Component/s: Java Broker
    • Labels:
      None

      Description

      In AMQChannel, code such as

      _logger.warn("IDLE TRANSACTION ALERT " + _logSubject.toString() ...

      should be replaced by

      _logger.warn("IDLE TRANSACTION ALERT " + _logSubject.toLogString() ...

      because the first example uses Object.toString, which is not very useful for logging.

        Activity

        Hide
        Philip Harvey added a comment -

        attached patch

        Show
        Philip Harvey added a comment - attached patch
        Hide
        Philip Harvey added a comment -

        please review and commit if you're happy.

        Show
        Philip Harvey added a comment - please review and commit if you're happy.
        Hide
        Philip Harvey added a comment -

        After further discussion, we've decided that this logging should only be produced if operational logging is switched off (to avoid duplication of log messages).

        Show
        Philip Harvey added a comment - After further discussion, we've decided that this logging should only be produced if operational logging is switched off (to avoid duplication of log messages).
        Hide
        Alex Rudyy added a comment -

        Attached patch allowing to have either operational or standard logs to avoid duplication in transaction timeout functionality on 0-9.x and 0.10 code paths.

        Show
        Alex Rudyy added a comment - Attached patch allowing to have either operational or standard logs to avoid duplication in transaction timeout functionality on 0-9.x and 0.10 code paths.
        Hide
        Robbie Gemmell added a comment -

        Keith and I reviewed the patch, and have made some modifications before committing it:

        • Use an instance of the helper per-channel instead of creating 2 new ones every time the check is performed.
        • Add some tests for the case when the timeouts are 0, and the operational logging is disabled.
        • (issue with the existing code) Move the closures right after the respective idle/open checks such that you only get the relevant log messages before the close operation is performed, reducing duplication.
        Show
        Robbie Gemmell added a comment - Keith and I reviewed the patch, and have made some modifications before committing it: Use an instance of the helper per-channel instead of creating 2 new ones every time the check is performed. Add some tests for the case when the timeouts are 0, and the operational logging is disabled. (issue with the existing code) Move the closures right after the respective idle/open checks such that you only get the relevant log messages before the close operation is performed, reducing duplication.

          People

          • Assignee:
            Philip Harvey
            Reporter:
            Philip Harvey
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development