Use a deep copy of query stats whose values won't change during sorting.

[ChristopherSchultz]

Fixes https://bz.apache.org/bugzilla/show_bug.cgi?id=58489

[aooohan]

Another unrelated thing, it might be better to unify the coding style?
eg:

for() {
   // code
}

not:

for()
 // code

not

[FSchumacher]

What happens, when all (or enough) of the stats have been changed in between? Wouldn't we get a IndexOutOfBoundException? May be, we should add a guard to the while expression and log a message, that we could net free enough stats.

[ChristopherSchultz]

Good question. If 100% of the stats have been updated, then we will remove nothing, no space will be gained, and we will end up running off the end of the list, throwing an error.

I see a few options:

1. YOLO! Just let the loop run and hope for the best! j/k
2. Ensure removeIndex doesn't grow too high, then just stop
3. Do (2), and repeat the sort+loop until we remove enough entries to meet cache-size expectations

I think probably (2) is sufficient. Thoughts?

[FSchumacher]

I had number two in mind, with a warn message at the end, when we could not free enough stats.

[aooohan]

Maybe use PriorityQueue instead of ArrayList is a better way? The latter will not have the problem of removeIndex being too high. See below:

protected void removeOldest(ConcurrentHashMap<String,QueryStats> queries) {
        int removeCount = queries.size() - maxQueries;
        if (removeCount <= 0) {
            return;
        }
        // Make a defensive deep-copy of the query stats list to prevent
        // concurrent changes to the lastModified member during list-sort
        PriorityQueue<MiniQueryStats> queue = new PriorityQueue<>(queries.size(), miniQueryStatsComparator);
        for(QueryStats stats : queries.values()) {
            queue.offer(new MiniQueryStats(stats));
        }
        while (removeCount > 0 && !queue.isEmpty()) {
            MiniQueryStats mqs = queue.poll();
            // Check to see if the lastInvocation has been updated since we
            // took our snapshot. If the timestamps disagree, it means
            // that this item is no longer the oldest (and it likely now
            // one of the newest).
            if(mqs.lastInvocation == mqs.queryStats.lastInvocation) {
                String sql = mqs.queryStats.query;
                queries.remove(sql);
                removeCount--;
                if (log.isDebugEnabled()) log.debug("Removing slow query, capacity reached:"+sql);
            }
            
        }
    }