Actually I brought a point about using SQL queries instead of scan queries because I worry about inconsistency between different implementations. AFAIR Scan and SQL queries implementations are totally different. Could you tell me how Index queries fit there?
My general ideas are as follows: 1. In an ideal world it would be great to have only one API (to rule them all) and implementation to cover all cases (i.e. scan, index, SQL). Also, I wonder how other vendors tackle this? 2. New IndexQuery might not mimic ScanQuery behavior but instead have a correct/expected one (including partition reservation). Perhaps IndexQuery should also cover regular entry iteration (scan) cases and become a new ground for generalized scan/index queries. 2021-12-09 17:08 GMT+03:00, Pavel Tupitsyn <ptupit...@apache.org>: > IndexQuery is experimental, so we can indeed make it reserve partitions by > default. No objections here. > > But with ScanQuery, I think it's better to add a "reservePartitions" > property so that default behavior is not affected. > Existing users may already have some logic in place to handle > inconsistencies. So if we enable reservation by default, > they'll get an unnecessary performance degradation. > > On Thu, Dec 9, 2021 at 4:42 PM Maksim Timonin <timoninma...@apache.org> > wrote: > >> 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 >> > > >> > >> > -- Best regards, Ivan Pavlukhin
