Feature #10325
Optimize AdaptiveQuery for single-result scenarios
0%
History
#1 Updated by Andrei Plugaru 12 months ago
The scope of this task is to optimize the AdaptiveQuery into RandomAccessQuery when we know that the where clause matches a single record. Technically, this should happen when the fields in the where clause match an unique index and also the fields from the index are mandatory.
Optimizing into RandomAccessQuery gives us the possibility of using the FF cache, and also avoids unnecessary SQLs being executed after the first record has been retrieved.
Before moving into implementation I have done some analysis to determine the impact. More exactly, I have tried to understand the number of AdaptiveQueries that return a single result. The exact logic added for determining if an AdaptiveQuery returns a single result is in AdaptiveQuery.initialize. Then, I have used a JMX counter to count the exact number of such instances. The exact code is in 10325a/16065.
During the performance tests of a large customer application, I have found out there are more than 3k such instances, so converting them into RandomAccessQuery should make an important difference.
#2 Updated by Andrei Plugaru 12 months ago
An initial approach I have tried is to make the AdaptiveQuery run into dynamic mode when we know it matches at most 1 record. This would have implicitly used a RAQ. However, this is not the best approach. Take this query for example:
for each table1 where table1.id = 1:
message table1.id.
end.
Also note that the where clause would match at most 1 record. With the current trunk, this would have used ProgressiveResults, so 2 SQL queries would have been executed. However, switching it into dynamic mode executes 3 SQLs. However, ideally this should execute only 1 SQL as we know beforehand that only 1 record will be found in the DB.
#3 Updated by Andrei Plugaru 12 months ago
I have further investigated this. Currently, switching the AdaptiveQuery into dynamic mode will lead to the query from the last note into executing more SQLs queries because of the way the FQL bundle is built.
However, had some conversations with Alex regarding some of his changes in 9724b which could help in this scenario. Basically, there he improved the revalidation of the AdaptiveQuery back to the preselect mode by skipping some sort criteria if we already have them in the original where clause. Unfortunately, this doesn't really work out of the box for this scenario because those changes are only hit for NEXT and PREV. I would need some similar logic to be done also for the FIRST case.
- The AdaptiveQuery is switched to dynamic mode
- The record is fetched either from FFcache or DB
- The query is switched back to preselect mode, however the new FQL should only have
the original where clause + recid > ?
#4 Updated by Andrei Plugaru 12 months ago
I have continued with the integration of Alex's idea from 9724b regarding moving the break position in the index based on the elements in the WHERE clause when the adaptive query is revalidated into preselect mode. I have extended that to skip all the elements in the WHERE when it matches an unique index. This means that the breakPosition will be set on the position of the recId field. All my changes are currently in 10325a/16067. It seems to be working relatively fine on my testcases.
The current problem that I am encountering is that after the query is revalidated into preselect mode, this SQL query(select table1__im0_.recid as col0_0_ from table1 table1__im0_ where table1__im0_.id = 1 and table1__im0_.recid > ? order by table1__im0_.id asc, table1__im0_.recid asc limit ? offset ?) is executed twice.
This is the stack trace of the 1st execution: Show
This is the stack trace of the 2nd execution: Show
I don't think this is directly caused by my changes, but rather a lenient bug that is exposed in my scenario. Nevertheless, the fact that more SQL queries are run than required still bothers me. During the execution, ResultsAdapter.first returns false as there are no more records in the DB, however, then the same query is being executed again because of the call to ProgressiveResults.next. IMHO, I don't see any reason to execute the whole logic in ProgressiveResults.next given that the previous call to ResultsAdapter.first didn't return anything. I am thinking to short circuit the execution somehow in order to skip executing the 2nd query.
#5 Updated by Andrei Plugaru 12 months ago
- Assignee set to Andrei Plugaru
- Status changed from New to WIP
I have continued implementation for this task.
For the issue spotted in the last part of #10325-4, I have stopped from calling adapter.first() when the AdaptiveQuery returns a single result. The reason for that call was to move the move the cursor forward in the case that record was also visited in dynamic mode. However, as for my current implementation for the single result scenario, I don't think we can get into a situation when that record would be visited in dynamic mode.
Apart from this, my last commit 10325a/16075 contains some other changes to control when exactly the transition(dynamic -> preselect or preselect -> dynamic) is done. However, the most important change is about when we can move the break position to recId. Previously, I was just using the same implementation as the one from 9724b which walked on the sorting index and moved to the last position which matched a property from the where clause. However, during the tests of a large customer application I understood that I actually don't need the sorting index to match the where clause properties. That is because I already know the we are having at most 1 result, so the sorting is actually irrelevant and I can move the break position directly to recId.
Testing¶
In order to get a brief idea of the correctness of the changes I have run the fwd tests of a large customer application and got no regressions. However, in order to measure the performance impact of the changes, I have for now, used the number of SQL select queries and the total execution time of the queries as metrics. For a warm server, the number of SQL queries executed dropped by 1777 (13%). However the total execution time of the SQLs dropped by only 167ms (5%). The reason why the reduction of execution time is not relative to the SQLs executed might be that the queries that are no longer executed were fast or that some queries that replaced the old ones are slower. However, an important note is that in my testing, the DB server is on the same machine as the FWD server, so the network latency is minimal. In real world scenario where we can have latency of a couple of ms for each packet, given a reduction of SQLs executed of more than 1.7k, I think that the difference of the execution time of the SQls might also substantially increase.
#6 Updated by Andrei Plugaru 12 months ago
- Status changed from WIP to Review
- reviewer Alexandru Lungu added
I have committed 10325a/16076. This revision contains mostly cosmetic changes.
I have run the performance test suite of a large customer application and obtained about 1.5% performance improvement, however as I stated in the previous note, the change can have a bigger impact in a scenario where the latency of network packets increases.
Currently, I don't really like that a new FQLPreprocessor object needs to be created in AdaptiveQuery.initialize. I tried to use the FQLPreprocessor from the components, however the UniqueIndexLookup from the fql is null in that case because of the problem reported in #10302. I tried some easy fixes for that, but got severe regressions.
Apart from that, I think this is ready for a review. Alex, please take a look!
#8 Updated by Alexandru Lungu 3 months ago
- Assignee changed from Andrei Plugaru to Alexandru Lungu