Hi, Ivan, Pavel! Thanks for your responses. > But is there a real need/benefit from using scan queries over primitive SQL queries today
In addition to Pavel's response, I'd like to add about IndexQuery. This query is in experimental status, but it shows great performance and beats SQL for *some* type of queries. For IndexQuery we can leverage on understanding that we use only index and apply some optimizations: skip boundaries check (h2 Select.isConditionMet), apply filtering by inline IO instead of extracting fields from BinaryObjects, also we skip some H2 related stuff. Also it's not implemented yet, but for IndexQuery we can access data pages sequentially instead of randomly (by iterating with IndexRow cursor by batches), and I expect it may make performance better in some highload cases. > it is desirable that successful (but may be strange) scenarios continue to work after fix (and not fail e.g. due to some introduced exception) > Partition reservation should be opt-in if we decide to proceed. With the approach I proposed we can reach it. Without specified query timeout (and cache queries don't have a public API for setting timeout), we can retry our attempts to reserve partitions while we do not succeed. So, users may get some increase in query time execution under unstable topology. But in exchange users will get more stable results, and avoid exceptions like in the ticket I showed [1]. We can implement this logic for IndexQuery only (as it's only experimental since 2.12), and extend it on ScanQuery later after stabilizing with IndexQuery. WDYT? [1] https://issues.apache.org/jira/browse/IGNITE-12591 On Thu, Dec 9, 2021 at 10:39 AM Pavel Tupitsyn <ptupit...@apache.org> wrote: > Agree with Ivan regarding compatibility. > Partition reservation should be opt-in if we decide to proceed. > > > is there a real need/benefit from using > > scan queries over primitive SQL queries today > > SQL requires additional configuration (QueryEntity) and involves memory and > CPU overhead to maintain the indexes. > Especially if the query is used rarely (maybe some maintenance tasks), scan > is a better option. > > On Thu, Dec 9, 2021 at 10:12 AM Ivan Pavlukhin <vololo...@gmail.com> > wrote: > > > Hi Maksim, > > > > Thank you for looking into this. While fixing wrong/surprising > > behavior is very important, I also have some concerns, let's say, from > > different angle of view: > > 1. From a first glance it seems that similar behavior of scan and SQL > > queries is a good idea. But is there a real need/benefit from using > > scan queries over primitive SQL queries today (e.g. SELECT * FROM > > table1)? > > 2. Also we need to carefully think about compatibility. It is > > desirable that successful (but may be strange) scenarios continue to > > work after fix (and not fail e.g. due to some introduced exception). > > > > Regarding partition reservation for long-time open cursors I guess > > some ideas might be found in LAZY mode for SQL queries [1]. > > > > [1] https://issues.apache.org/jira/browse/IGNITE-9171 > > > > 2021-12-08 20:11 GMT+03:00, Maksim Timonin <timoninma...@apache.org>: > > > Hi, Igniters! > > > > > > There is a known issue that ScanQuery on unstable topology returns > > > incorrect results: duplicates data [1] or fails with an exception [2]. > > The > > > reason is ScanQuery doesn't reserve partitions. > > > > > > IndexQuery shares the same query processing as ScanQuery, and then it > is > > > also affected by unstable topology. I want to fix it for IndexQuery. > > > IndexQuery should provide the same stability as SQL queries do - no > > > occasional failures or data duplication. > > > > > > I dived into the SQL query processing and found that we can unify logic > > for > > > SQL queries and cache queries (ScanQuery, IndexQuery, TextQuery): > > > > > > 1. Currently, cache queries use `GridCacheQueryAdapter#nodes` to find > > nodes > > > to run a query. From the other side, SQL processing uses for the same > > goal > > > `ReducePartitionMapper#nodesForPartitions`. It looks like those methods > > > have some slight differences, but in the common, they do the same. So, > I > > > propose to make a single method for both cases. > > > > > > 2. SQL processing uses `PartitionReservationManager` for reserve > > partitions > > > on the map side. I propose to move it to the ignite-core module and > start > > > using it for the cache queries. > > > > > > 3. Implement retries for the cache queries in case we failed to reserve > > > partitions on the map side. > > > > > > Currently, I see a downside of reserving partitions for cache queries: > > > cache queries are lazy. And the time of partition reservation depends > on > > a > > > user's application code (how fast a cursor is iterated and closed). > > AFAIU, > > > it's not very good to have a partition in reserve too long. Please, > > correct > > > me if I'm wrong here. > > > > > > But from the other side, Ignite reserves partitions for ScanQuery when > a > > > partition has been specified as a ScanQuery parameter, and Ignite > > reserves > > > partitions for SQL with the flag lazy=true. Also: > > > - IndexQuery: I expect simple queries that will return a relatively > small > > > amount of data. Then partitions wouldn't be reserved too much time. > > > - the same is for TextQuery - it returns a limited amount of data (due > to > > > the Lucene logic). > > > - full ScanQuery it's not in use much, AFAIK. So, it's by default a > > pretty > > > heavy operation. > > > > > > So, I think it's safe to reserve partitions in any case. But there > could > > be > > > an alternative optimistic approach, smth like that: > > > 1. Check that topology is stable and waiting while it's not stabilized. > > > 2. Run a query on a stable cluster. > > > 3. In cases when a cluster becomes unstable during query execution - > try > > to > > > reserve partitions at runtime (query should be aware of topology > changes) > > > and fail in case of reservation failure (if a user already fetched some > > > data). > > > > > > I don't like the idea of this optimistic approach because in case a > user > > > got some data, we don't have a better solution than to fail a query in > > case > > > of cluster instability and reservation failure. > > > > > > Igniters, WDYT? > > > > > > [1] https://issues.apache.org/jira/browse/IGNITE-12591 > > > [2] https://issues.apache.org/jira/browse/IGNITE-16031 > > > > > > > > > -- > > > > Best regards, > > Ivan Pavlukhin > > >
