[jira] [Commented] (IGNITE-6937) SQL TX: Support SELECT FOR UPDATE

["Vladimir Ozerov (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/IGNITE-6937?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16454330#comment-16454330 ] 

Vladimir Ozerov commented on IGNITE-6937:
-----------------------------------------

[~al.psc], my comments:
1) {{GridCacheMapEntry}} - my preference would be to avoid duplicated logic in lock method and callback. Looks like common method could be implemented for both scenarios - callback should simply delegate to lock method
2) {{GridDhtTransactionalCacheAdapter}} - why did we remove exception handling? I understand that {{IgniteCheckedException}} is never throws, but shouldn't we catch all exceptions instead of ignoring all of them?
3) {{GridCacheOperation}} - new member should be in the end of the list
4) {{GridH2QueryRequest}} - embedding enlist request with almost all the same fields is definitely not a good idea. Please suggest a design without duplicated data being sent over the wire.
5) {{GridReduceQueryExecutor}} - lines 604 and 607 - why do we call it twice?
6) {{GridMapQueryExecutor.txLockMsgLog}} - dead code?
7) {{GridMapQueryExecutor:751}} - can we perform rewrite only once on reducer node instead of doing it multiple times on map nodes?
8) {{QueryPageEnlistFutureSupplier}} - please avoid C1 and anonymous classes
9) {{GridMapQueryExecutor#sendNextPage}} - is it ok that we call {{fut.init()}} on every call this method? Looks wrong to me. Do we have tests for multi-page query?
10) Looking at amount of code changes on reducer side I doubt that assembling everything on reducer side was correct decision. It seems that we'd better to process everything on mapper side and return the very first page when everything is already locked. In fact, we already work this way - in non-lazy mode H2 moves everything to local heap result set first, and in current implementation lazy flag is prohibited explicitly for SFU. Can we simply do the following:
- Execute the request
- Extract result from {{ResultSet}} which always would be heap-based {{org.h2.result.LocalResult}}
- Lock these entries
- Send results in pages cutting {{_KEY}} column
It seems that with this approach significant amount of complexity would go away.
11) {{CacheMvccSelectForUpdateQueryTest}} - tests do not work (hang)

> SQL TX: Support SELECT FOR UPDATE
> ---------------------------------
>
>                 Key: IGNITE-6937
>                 URL: https://issues.apache.org/jira/browse/IGNITE-6937
>             Project: Ignite
>          Issue Type: Task
>          Components: cache, sql
>            Reporter: Vladimir Ozerov
>            Assignee: Alexander Paschenko
>            Priority: Major
>              Labels: iep-3
>             Fix For: 2.6
>
>
> Normally in SQL world readers do not block writers. This is how our SELECT operations should work by default. But we need to add a support for {{SELECT ... FOR UPDATE}} read mode, when reader obtains exclusive lock on read. 
> In this mode we lock entries as usual, but then send data back to the caller. First page can be returned directly in our {{LockResponse}}. Next pages should be requested in separate requests. With this approach {{SELECT ,,, FOR UPDATE}} will require only single round-trip to both lock and read data in case of small updates.
> Update {{SELECT}} statement syntax once this feature is supported:
> https://apacheignite-sql.readme.io/docs/select



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)