Qpid
  1. Qpid
  2. QPID-4533

Java broker performance tests should support writing results to database

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 0.20
    • Fix Version/s: 0.21
    • Component/s: Java Performance Tests
    • Labels:
      None

      Description

      We should allow perftest results to be written to a database to facilitate analysis, eg charting how our performance has varied over time

        Issue Links

          Activity

          Hide
          Philip Harvey added a comment -

          Attached patch for review.

          I will attach instructions on how to manually test the database persistence in a separate comment.

          Show
          Philip Harvey added a comment - Attached patch for review. I will attach instructions on how to manually test the database persistence in a separate comment.
          Hide
          Philip Harvey added a comment -

          Attached chartdef that can be used for manually testing graphs generated from the Derby database.

          Show
          Philip Harvey added a comment - Attached chartdef that can be used for manually testing graphs generated from the Derby database.
          Hide
          Philip Harvey added a comment -

          Hi Keith,

          Please can you review this change.

          Here is an easy way to manually test:

          • Run the perf tests using the new script in perftests/etc/run-perftests.sh, which writes results to a Derby DB in the current directory
          • Run the visualisation tool against those results using the (somewhat meaningless) timeseries.chartdef file attached to this Jira, by executing script perftests/etc/visualisation.sh. You will need to edit the script to uncomment the JDBC variables and to provide the location of the timeseries.chartdef file.

          I intend to add subsequently add useful timeseries chart definitions under a separate Jira.

          Show
          Philip Harvey added a comment - Hi Keith, Please can you review this change. Here is an easy way to manually test: Run the perf tests using the new script in perftests/etc/run-perftests.sh, which writes results to a Derby DB in the current directory Run the visualisation tool against those results using the (somewhat meaningless) timeseries.chartdef file attached to this Jira, by executing script perftests/etc/visualisation.sh. You will need to edit the script to uncomment the JDBC variables and to provide the location of the timeseries.chartdef file. I intend to add subsequently add useful timeseries chart definitions under a separate Jira.
          Hide
          Philip Harvey added a comment -

          attached patch

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

          Hi Phil,

          Just a couple of comments:

          1) The new files ResultsDbWriter, ResultsDbWriterTest, ResultsTestFixture are missing their licence.
          2) ResultsDbWriter - why do you call driverClass.newInstance()? Won't loading the class be sufficient to register the driver?
          3) Typo in ControllerRunner._resuResultsFileWriter
          4) TODO left in JdbcCsvSeriesBuilder (i agree with your suggestion to rename)
          5) JdbcCsvSeriesBuilder - misplaced member variable _callback
          6) JdbcCsvSeriesBuilder - zero arg constructor is unused
          7) How are we intending to constrain the growth of the database to prevent the need for manual intervention?

          cheers, Keith.

          Show
          Keith Wall added a comment - Hi Phil, Just a couple of comments: 1) The new files ResultsDbWriter, ResultsDbWriterTest, ResultsTestFixture are missing their licence. 2) ResultsDbWriter - why do you call driverClass.newInstance()? Won't loading the class be sufficient to register the driver? 3) Typo in ControllerRunner._resuResultsFileWriter 4) TODO left in JdbcCsvSeriesBuilder (i agree with your suggestion to rename) 5) JdbcCsvSeriesBuilder - misplaced member variable _callback 6) JdbcCsvSeriesBuilder - zero arg constructor is unused 7) How are we intending to constrain the growth of the database to prevent the need for manual intervention? cheers, Keith.
          Hide
          Philip Harvey added a comment -

          Thanks for reviewing this Keith. I've incorporated your comments and committed them.

          Regarding constraining the growth of the database, I intend that we'll "cron" some SQL scripts to remove old entries. If this proves inadequate or cumbersome we can implement something more automated.

          Show
          Philip Harvey added a comment - Thanks for reviewing this Keith. I've incorporated your comments and committed them. Regarding constraining the growth of the database, I intend that we'll "cron" some SQL scripts to remove old entries. If this proves inadequate or cumbersome we can implement something more automated.
          Hide
          Philip Harvey added a comment -

          Already reviewed by Keith.

          Show
          Philip Harvey added a comment - Already reviewed by Keith.
          Hide
          Philip Harvey added a comment -

          I now realise that one of the unit tests is timezone-specific.

          Show
          Philip Harvey added a comment - I now realise that one of the unit tests is timezone-specific.
          Hide
          Philip Harvey added a comment -

          Fixed timezone bug. Please could you review?

          Show
          Philip Harvey added a comment - Fixed timezone bug. Please could you review?
          Hide
          Philip Harvey added a comment -

          The committed code has broken the ability of Jdbc[Csv]SeriesBuilder to build series using the series directory. This is commonly used for reading baseline data so needs to be fixed. I will address this bug in the next few days.

          Show
          Philip Harvey added a comment - The committed code has broken the ability of Jdbc [Csv] SeriesBuilder to build series using the series directory. This is commonly used for reading baseline data so needs to be fixed. I will address this bug in the next few days.
          Hide
          Philip Harvey added a comment -

          The aforementioned bug is now fixed. Committed in revision 1444335.

          Show
          Philip Harvey added a comment - The aforementioned bug is now fixed. Committed in revision 1444335.
          Hide
          Keith Wall added a comment -

          No further comments.

          Show
          Keith Wall added a comment - No further comments.

            People

            • Assignee:
              Keith Wall
              Reporter:
              Philip Harvey
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development