[
https://issues.apache.org/jira/browse/IGNITE-6937?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16454330#comment-16454330
]
Vladimir Ozerov edited comment on IGNITE-6937 at 4/26/18 2:45 PM:
------------------------------------------------------------------
[~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)
was (Author: vozerov):
[~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)
