Uploaded image for project: 'Cassandra'
  1. Cassandra
  2. CASSANDRA-11425

Add prepared query parameter to trace for "Execute CQL3 prepared query" session

    Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Minor
    • Resolution: Fixed
    • Fix Version/s: 3.8
    • Component/s: CQL
    • Labels:
      None

      Description

      For now, the system_traces.sessions rows for "Execute CQL3 prepared query" do not show us any information about the prepared query which is executed on the session. So we can't see what query is the session executing.
      I think this makes performance tuning difficult on Cassandra.

      So, In this ticket, I'd like to add the prepared query parameter on Execute session trace like this.

      cqlsh:system_traces> select * from sessions ;
      
       session_id                           | client    | command | coordinator | duration | parameters                                                                                                                                           | request                     | started_at
      --------------------------------------+-----------+---------+-------------+----------+------------------------------------------------------------------------------------------------------------------------------------------------------+-----------------------------+---------------------------------
       a001ec00-f1c5-11e5-b14a-6fe1292cf9f1 | 127.0.0.1 |   QUERY |   127.0.0.1 |      666 |      \{'consistency_level': 'ONE', 'page_size': '5000', 'query': 'SELECT * FROM test.test2 WHERE id=? LIMIT 1', 'serial_consistency_level': 'SERIAL'\} | Execute CQL3 prepared query | 2016-03-24 13:38:00.000000+0000
       a0019de0-f1c5-11e5-b14a-6fe1292cf9f1 | 127.0.0.1 |   QUERY |   127.0.0.1 |      109 |                                                                                             {'query': 'SELECT * FROM test.test2 WHERE id=? LIMIT 1'} |        Preparing CQL3 query | 2016-03-24 13:37:59.998000+0000
       a0014fc0-f1c5-11e5-b14a-6fe1292cf9f1 | 127.0.0.1 |   QUERY |   127.0.0.1 |      126 |                                                                                           {'query': 'INSERT INTO test.test2(id,value) VALUES (?,?)'} |        Preparing CQL3 query | 2016-03-24 13:37:59.996000+0000
       a0019de1-f1c5-11e5-b14a-6fe1292cf9f1 | 127.0.0.1 |   QUERY |   127.0.0.1 |      764 |      {'consistency_level': 'ONE', 'page_size': '5000', 'query': 'SELECT * FROM test.test2 WHERE id=? LIMIT 1', 'serial_consistency_level': 'SERIAL'} | Execute CQL3 prepared query | 2016-03-24 13:37:59.998000+0000
       a00176d0-f1c5-11e5-b14a-6fe1292cf9f1 | 127.0.0.1 |   QUERY |   127.0.0.1 |      857 | {'consistency_level': 'QUORUM', 'page_size': '5000', 'query': 'INSERT INTO test.test2(id,value) VALUES (?,?)', 'serial_consistency_level': 'SERIAL'} | Execute CQL3 prepared query | 2016-03-24 13:37:59.997000+0000
      

      Now, "Execute CQL3 prepared query" session displays its query.
      I believe that this additional information would help operators a lot.

        Issue Links

          Activity

          Hide
          Yasuharu Yasuharu Goto added a comment -

          Robert Stupp Thank you very much for the modifying and the merge!
          I'll be careful of unit tests next time.
          I'm interested in CASSANDRA-11719, but there is a challenger already. I'm going to watch the ticket.

          Thanks.

          Show
          Yasuharu Yasuharu Goto added a comment - Robert Stupp Thank you very much for the modifying and the merge! I'll be careful of unit tests next time. I'm interested in CASSANDRA-11719 , but there is a challenger already. I'm going to watch the ticket. Thanks.
          Hide
          snazy Robert Stupp added a comment -

          Patch LGTM.
          Adding rawCQLStatement to weighing is fine.
          To verify that the patch works as expected, I created a simple unit test (please add one yourself in the future ).

          While reviewing your patch, I found that the old TODO in org.apache.cassandra.transport.messages.ExecuteMessage#execute is now possible and opened CASSANDRA-11719 for this. Yasuharu Goto Do you want to tackle 11719, too?

          Rebased branch plus unit test
          testall
          dtest

          Utests + dtests look good. Committed as 07385b6ce90d3a230f00f2812b22e3ff158cc2d6 to trunk.
          Thanks for the patch!

          Show
          snazy Robert Stupp added a comment - Patch LGTM. Adding rawCQLStatement to weighing is fine. To verify that the patch works as expected, I created a simple unit test (please add one yourself in the future ). While reviewing your patch, I found that the old TODO in org.apache.cassandra.transport.messages.ExecuteMessage#execute is now possible and opened CASSANDRA-11719 for this. Yasuharu Goto Do you want to tackle 11719, too? Rebased branch plus unit test testall dtest Utests + dtests look good. Committed as 07385b6ce90d3a230f00f2812b22e3ff158cc2d6 to trunk. Thanks for the patch!
          Hide
          snazy Robert Stupp added a comment -

          CASSANDRA-11555 could help with the cached prepared statement restriction.
          I'd really like the ability to have the CQL source available - the also allows us to produce better debug logs and also log more information with exceptions.

          Show
          snazy Robert Stupp added a comment - CASSANDRA-11555 could help with the cached prepared statement restriction. I'd really like the ability to have the CQL source available - the also allows us to produce better debug logs and also log more information with exceptions.
          Hide
          Yasuharu Yasuharu Goto added a comment -

          Tyler Hobbs Thanks for your response!

          I concern about the memory consumption too. I have no idea if we should trim queries to save memory.
          For a good memory management, I think we might have to add query string into EntryWeigher.weightOf() calculation (and increase MAX_CACHE_PREPARED_MEMORY ?).

          Show
          Yasuharu Yasuharu Goto added a comment - Tyler Hobbs Thanks for your response! I concern about the memory consumption too. I have no idea if we should trim queries to save memory. For a good memory management, I think we might have to add query string into EntryWeigher.weightOf() calculation (and increase MAX_CACHE_PREPARED_MEMORY ?).
          Hide
          thobbs Tyler Hobbs added a comment -

          This is almost the same as CASSANDRA-7021. When I was looking into CASSANDRA-7021, I didn't think it was a good idea to always store the full query string with the prepared statement due to the extra memory usage. However, I might have been overly cautious. Perhaps a good compromise would be to store up to the first 128 chars of the query string, or something along those lines?

          I'm curious what other C* devs think about this.

          Show
          thobbs Tyler Hobbs added a comment - This is almost the same as CASSANDRA-7021 . When I was looking into CASSANDRA-7021 , I didn't think it was a good idea to always store the full query string with the prepared statement due to the extra memory usage. However, I might have been overly cautious. Perhaps a good compromise would be to store up to the first 128 chars of the query string, or something along those lines? I'm curious what other C* devs think about this.
          Show
          Yasuharu Yasuharu Goto added a comment - My patch is here. https://github.com/apache/cassandra/compare/trunk...matope:11425-trunk

            People

            • Assignee:
              Yasuharu Yasuharu Goto
              Reporter:
              Yasuharu Yasuharu Goto
              Reviewer:
              Robert Stupp
            • Votes:
              0 Vote for this issue
              Watchers:
              6 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development