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

Allow "intercepting" query by user provided custom classes

    Details

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

      Description

      The idea for this ticket is to abstract the main execution methods of QueryProcessor into an interface, something like:

      public interface QueryHandler
      {
          public ResultSet process(String query, QueryState state, QueryOptions options);
          public ResultMessage.Prepared prepare(String query, QueryState state);
          public ResultSet processPrepared(CQLStatement statement, QueryState state, QueryOptions options);
          public ResultSet processBatch(BatchStatement statement, QueryState state, BatchQueryOptions options);
      }
      

      and to allow users to provide a specific class of their own (implementing said interface) to which the native protocol would handoff queries to (so by default queries would go to QueryProcessor, but you would have a way to use a custom class instead).

      A typical use case for that could be to allow some form of custom logging of incoming queries and/or of their results. But this could probably also have some application for testing as one could have a handler that completely bypass QueryProcessor if you want, say, do perf regression tests for a given driver (and don't want to actually execute the query as you're perf testing the driver, not C*) without needing to patch the sources. Those being just examples, the mechanism is generic enough to allow for other ideas.

      Most importantly, it requires very little code in C*. As for how users would register their "handler", it can be as simple as a startup flag indicating the class to use, or a yaml setting, or both.

      1. 6659.txt
        24 kB
        Sylvain Lebresne

        Activity

        Hide
        slebresne Sylvain Lebresne added a comment -

        Alright, last version lgtm, committed.

        Show
        slebresne Sylvain Lebresne added a comment - Alright, last version lgtm, committed.
        Hide
        beobal Sam Tunnicliffe added a comment -

        Ok fair enough, I've pushed another commit to my branch that reverts the QP.prepare() changes & renames the thrift version of getPrepared()

        Show
        beobal Sam Tunnicliffe added a comment - Ok fair enough, I've pushed another commit to my branch that reverts the QP.prepare() changes & renames the thrift version of getPrepared()
        Hide
        slebresne Sylvain Lebresne added a comment -

        I'm not sure the "refactoring of QP.prepare" is useful, nor that we need 2 prepare method in the interface. The QueryState that the existing prepare methods takes has a getClientState(), and if that's an instance of ThriftClientState, then it's from thrift (and you can call the existing QP.prepare() with the proper flag). Also, that's a nit, but I'd be keen to rewrite the thrift getPrepared() to getPreparedForThrift() or something so it's a bit more explicit now that it's in a somewhat public interface.

        The rest of the changes lgtm.

        Show
        slebresne Sylvain Lebresne added a comment - I'm not sure the "refactoring of QP.prepare" is useful, nor that we need 2 prepare method in the interface. The QueryState that the existing prepare methods takes has a getClientState(), and if that's an instance of ThriftClientState, then it's from thrift (and you can call the existing QP.prepare() with the proper flag). Also, that's a nit, but I'd be keen to rewrite the thrift getPrepared() to getPreparedForThrift() or something so it's a bit more explicit now that it's in a somewhat public interface. The rest of the changes lgtm.
        Hide
        beobal Sam Tunnicliffe added a comment -

        Sylvain Lebresne I've made a couple of extensions to your original patch & pushed the results to https://github.com/beobal/cassandra/commits/6659

        The first commit there is your original patch, there are 2 relevant additions in the second commit:

        • Made QueryProcessor.processStatement public, so that QueryHandler implementations can call it from within their process implementations. Otherwise all the QH implementation has to work with is the original CQL String. If it wants access to the parsed CQLStatement, it can call QP.getStatement, but it has to delegate to QP.process to actually execute it, meaning double the parsing overhead.
        • Extended the QH interface so that it is also usable for CQL3 queries received over thrift (this also involved a slight refactoring of QP.prepare)

        While I was at it I also cleaned up a couple of things in QP (removed the hooks imports, dropped some unused arguments etc) and removed the now unused classes

        Show
        beobal Sam Tunnicliffe added a comment - Sylvain Lebresne I've made a couple of extensions to your original patch & pushed the results to https://github.com/beobal/cassandra/commits/6659 The first commit there is your original patch, there are 2 relevant additions in the second commit: Made QueryProcessor.processStatement public, so that QueryHandler implementations can call it from within their process implementations. Otherwise all the QH implementation has to work with is the original CQL String. If it wants access to the parsed CQLStatement, it can call QP.getStatement, but it has to delegate to QP.process to actually execute it, meaning double the parsing overhead. Extended the QH interface so that it is also usable for CQL3 queries received over thrift (this also involved a slight refactoring of QP.prepare) While I was at it I also cleaned up a couple of things in QP (removed the hooks imports, dropped some unused arguments etc) and removed the now unused classes
        Hide
        slebresne Sylvain Lebresne added a comment -

        Seems I had forgotten to "patch available that one"

        Show
        slebresne Sylvain Lebresne added a comment - Seems I had forgotten to "patch available that one"
        Hide
        jbellis Jonathan Ellis added a comment -

        Tagging Benjamin Coverston for review.

        Show
        jbellis Jonathan Ellis added a comment - Tagging Benjamin Coverston for review.
        Hide
        slebresne Sylvain Lebresne added a comment -

        Attaching fairly trivial patch to implement this (the patch is against 2.0 because that has virtually no chance to break anything existing so why not). Note that the patch remove the pre and post execution hooks from QueryProcessor as those were only here for external tool and, unless I'm missing something obvious, the mechanism here provides a stricly more general mechanism.

        Show
        slebresne Sylvain Lebresne added a comment - Attaching fairly trivial patch to implement this (the patch is against 2.0 because that has virtually no chance to break anything existing so why not). Note that the patch remove the pre and post execution hooks from QueryProcessor as those were only here for external tool and, unless I'm missing something obvious, the mechanism here provides a stricly more general mechanism.

          People

          • Assignee:
            slebresne Sylvain Lebresne
            Reporter:
            slebresne Sylvain Lebresne
            Reviewer:
            Sam Tunnicliffe
          • Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development