• Our product teams collect and evaluate feedback from a number of different sources. To learn more about how we use customer feedback in the planning process, check out our new feature policy.

      NOTE: This suggestion is for JIRA Cloud. Using JIRA Server? See the corresponding suggestion.

      Summary
      IssueNavigator calls every instance of a JQL function's getValues() three times

      1. to validate the query
      2. to get the query's issues
      3. to see if the query fits in the simple filter form [getQueryContext()]
        If the JQL function has to inspect a lot of data or returns a lot data, this leads to severe performance issues unexpectedly multiply.

      Detailed Explanation
      As some Atlassians know, we wedged Mike Cannon-Brookes prototype of advanced search into our Jira 3.13 and made enhancements to search change history and all the comment fields.
      I've been working to adapt what we did to work in Jira 4.

      In Jira 3, this required a heavy-handed modification of the LuceneQueryCreator so that a Lucene Filter on the comment index was properly or'ed or and'ed or not'ed against the issue index. It was messy but it worked. The performance was OK as long as the user didn't have too many queries against the comment index as sub-queries.

      For Jira 4, I figured that using a JQL function to search on the comment index and then return JiraDataType.ISSUE was the cleaner way to do what we had done by hacking LuceneQueryCreator in Jira 3. The JQL function searches the comment index and returns a list of issue id's. The rest would be handled by in Jira 4's great new re-worked search system and I could encapsulate everything in a plugin.

      What I did works, but it has horrible performance.
      IssueNavigator calls every instance of the function's getValues() three times

      1. to validate the query
      2. to get the query's issues
      3. to see if the query fits in the simple filter form [getQueryContext()]

      This means that my comment JQL function has to search the comment index 3 times, loop 3 times through each hit on the comment index and convert it to an issue id, and then the system re-queries the issue index using those issue ids in a new search 3 times.
      In our Jira 3 implementation the comment index was searched once, converted to a Lucene Filter and then filtered once against the issue index using Lucene. Here are averages of a couple comparisons that I did on the same machine running both Jira 3 and Jira 4

      issues Jira 3 Jira 4
      500 5 seconds 5 seconds
      30000 23 seconds 190 seconds
      100000 53 seconds 10 minutes

      (I also noticed that if an issue in the results is for some reason invalid – for example if it fell victim to Bulk Edit woes – then after the user has waited minutes to get their slow running results, instead of getting all their issues with a warning note about the invalid issues, they get zero results and an error message which doesn't indicate what went wrong. See invalid-issue.png. I've filed a separate bug on this, JRA-22277.)

      I believe JQL needs several an improvements

      1. getValues() should only be called once not 3 times per function. Perhaps wrap the call to the function in an object that holds onto the values while it gets passed from validation to query to context anaylysis. Another idea, add an isTooComplexForSimpleSearch() method to the JqlFunction interface that would negate the need to run the query for most functions
      2. The returning of issue id's from JqlFunction.getValues() is almost always going to lead to a poor-performance situation because whatever the function does to discover issue ids is going to be expensive. Perhaps it should be deprecated. Update I also have concern for the User functions. For example, I wrote another one that returns all the users that are no longer active so that project leads can re-assign issues that are assigned to employees who no longer work at our company. This function has to loop through all the users and find which ones no longer belong to any groups. The JQL framework also runs this function three times per function call.
      3. Perhaps the better alternative would be to have a JiraDataType.FILTER so that getValues would return a Lucene Filter. after analyzing the issue further myself, I realize that this isn't a valid suggestion

      Update, September 15
      After studying this for a couple days, I was able to speed up our system with 4 changes:

      1. To stop it from looping through thousands of issues and constructing an issue for each before the query executes, I commented out the validation code in IssueIdValidator
      2. I modified IssueIdQueryFactory to construct an IssueIdFilter wrapped by a ConstantScoreQuery rather than a BooleanQuery with a sub-query for each issue id. This avoids the Too Many Clauses Error if the JQL function hits more than 32000 issues. This is similar to the solution that I posed in JRA-22453. See code below.
      3. I modified MultiClauseDecoratorContextFactory so that it only loops through the first 100 results during getQueryContext().
      4. To work around but not solve the problem of IssueNavigator calling each instance of a JQL function 3 times, I put a cache in my JQL issue-type functions that holds onto each instance's results for a few seconds. This consumes unnecessary memory but it's the best that I can think of short of re-writing the entire JQL call framework to hold onto the results and then pass them through validate(), executeQuery(), and getQueryContext() rather than having validate(), executeQuery(), and getQueryContext() each separately calling jqlIssueFunction.getValues().
      5. To avoid unneccessarily calling isCurrentQueryTooComplex(), I hacked our IssueNavigator.isAdvanced() to have a regular expression that recognizes if there's a JQL string that matches one of our nasty custom JQL issue-type functions.

      With these improvements, our Jira 4 now runs faster than Jira 3! Yay!
      New results

      issues Jira 3 Jira 4
      500 5 seconds 4 seconds
      30000 23 seconds 8 seconds
      100000 53 seconds 23 seconds

      (Digression – another improvement request that I contemplate – get rid of the issue index and index everything, including change history, inside the comment index)

      IssueIdQueryFactory.java
          private Query createPositiveEqualsQuery(final List<QueryLiteral> rawValues)
          {
              if (rawValues.size() == 1)
              {
                  return createQuery(rawValues.get(0));
              }
              else
              {
                  return new ConstantScoreQuery(new IssueIdFilter(rawValues));
              }
          }
      

        1. invalid-issue.png
          84 kB
          Jeff Kirby

            [JRACLOUD-22256] Performance Issue with JQL functions

            @James
            Any timeline for the 4.4.5 release?

            David Corley added a comment - @James Any timeline for the 4.4.5 release?

            diegor added a comment -

            @James - thanks a lot for your effort to fix this "bug". My concern is that I can't upgrade my installation due to internal dependencies. At the moment we have 4.4.0 and I'd like to have a guide or tutorial that explain how to implement it.
            Thank you again.

            diegor added a comment - @James - thanks a lot for your effort to fix this "bug". My concern is that I can't upgrade my installation due to internal dependencies. At the moment we have 4.4.0 and I'd like to have a guide or tutorial that explain how to implement it. Thank you again.

            The results of getValues are now cached in the request query cache, keyed on user clause and operand. getValues is now only ever called once per clause

            tier-0 grump added a comment - The results of getValues are now cached in the request query cache, keyed on user clause and operand. getValues is now only ever called once per clause

            diegor added a comment -

            @James - Thank you very much for your help. I (and the Jira community) appreciate it!

            diegor added a comment - @James - Thank you very much for your help. I (and the Jira community) appreciate it!

            @Diego - that's exactly what I mean - I'll try to write up a tutorial on developer.atlassian.com this weekend.

            tier-0 grump added a comment - @Diego - that's exactly what I mean - I'll try to write up a tutorial on developer.atlassian.com this weekend.

            MattS added a comment -

            James,

            Thanks, that's useful guidance on how to write a custom searcher. And that's hard to come by!

            ~Matt

            MattS added a comment - James, Thanks, that's useful guidance on how to write a custom searcher. And that's hard to come by! ~Matt

            Jeff Kirby added a comment -

            Thank you, James, for taking a look at this. It will benefit the core Jira product.

            Jeff Kirby added a comment - Thank you, James, for taking a look at this. It will benefit the core Jira product.

            diegor added a comment -

            @James, thank you very much for you response. So you're suggesting to have a look at you CommentSearcher to replicate it for my custom field, aren't you?
            Can you point me to a tutorial or documentation that can help me somehow?

            Thanks a lot!

            diegor added a comment - @James, thank you very much for you response. So you're suggesting to have a look at you CommentSearcher to replicate it for my custom field, aren't you? Can you point me to a tutorial or documentation that can help me somehow? Thanks a lot!

            @Diego - the problem here is in how we convert the QueryLiterals you return into a LuceneQuery - the default mechanism is to simply create a single clause for each QueryLiteral, so if you had 6 cases you would end up with a query like case:case1 case:case2 case:case3 case:case4 case:case5 case:case6 - you can imagine that if the function returns 60000 elements the query is pretty unoptimised (especially with all the Lucene scoring going on). You probably need to implement a custom searcher that builds a more efficient query than our generic query creation mechanism does - you're also very close to falling victim to the TooManyClauses exception, which is triggered at 65000 clauses. The usual way to do this is to use HitCollectors, then wrap these in a Filter and search with this (under the covers this is how we changed the CommentSearcher) - the improvements in search time are dramatic. We don't do this with CFSelectTypes, because firstly I suspect that we never thought customers would end up with 60000 options in them, and secondly it's much harder to create the BitSet filter in a generic fashion.

            tier-0 grump added a comment - @Diego - the problem here is in how we convert the QueryLiterals you return into a LuceneQuery - the default mechanism is to simply create a single clause for each QueryLiteral, so if you had 6 cases you would end up with a query like case:case1 case:case2 case:case3 case:case4 case:case5 case:case6 - you can imagine that if the function returns 60000 elements the query is pretty unoptimised (especially with all the Lucene scoring going on). You probably need to implement a custom searcher that builds a more efficient query than our generic query creation mechanism does - you're also very close to falling victim to the TooManyClauses exception, which is triggered at 65000 clauses. The usual way to do this is to use HitCollectors, then wrap these in a Filter and search with this (under the covers this is how we changed the CommentSearcher) - the improvements in search time are dramatic. We don't do this with CFSelectTypes, because firstly I suspect that we never thought customers would end up with 60000 options in them, and secondly it's much harder to create the BitSet filter in a generic fashion.

            The siren call to fix this is proving too strong to resist, I will look into a fix for the multiple getValues() calls, as we shouldn't be leaving it to the plugin developer to code around.

            tier-0 grump added a comment - The siren call to fix this is proving too strong to resist, I will look into a fix for the multiple getValues() calls, as we shouldn't be leaving it to the plugin developer to code around.

              jwinters tier-0 grump
              adc6ee404f6d Jeff Kirby
              Votes:
              24 Vote for this issue
              Watchers:
              36 Start watching this issue

                Created:
                Updated:
                Resolved: