[jira] [Commented] (ASTERIXDB-2204) Define IIndexCursor interface

["ASF subversion and git services (JIRA)" <ji...@apache.org>]

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

ASF subversion and git services commented on ASTERIXDB-2204:
------------------------------------------------------------

Commit 08dc8597e2c9bcbf133250487c71b82d53fd1224 in asterixdb's branch refs/heads/master from [~alamoudi]
[ https://git-wip-us.apache.org/repos/asf?p=asterixdb.git;h=08dc859 ]

[ASTERIXDB-2204][STO] Fix the IIndexCursor interface

- user model changes: no
- storage format changes: no
- interface changes: yes
  - replace IIndexCursor.reset with close
  - replace IIndexCursor.close with destroy

Details:
- This change is the first step towards fixing the behavior
  of implementors/callers of the IIndexCursor interface
- In this change, we simply rename the reset -> close
  and close -> destroy and we write down the javadocs
  explaining the semantics of the interface.
- LSM Index Cursors don't implements ITreeIndexCursor
  anymore.

Change-Id: I64cf8c0a5473268bdfd71fd560ee6b3bff219ce9
Reviewed-on: https://asterix-gerrit.ics.uci.edu/2238
Tested-by: Jenkins <je...@fulliautomatix.ics.uci.edu>
Integration-Tests: Jenkins <je...@fulliautomatix.ics.uci.edu>
Contrib: Jenkins <je...@fulliautomatix.ics.uci.edu>
Reviewed-by: abdullah alamoudi <ba...@gmail.com>


> Define IIndexCursor interface
> -----------------------------
>
>                 Key: ASTERIXDB-2204
>                 URL: https://issues.apache.org/jira/browse/ASTERIXDB-2204
>             Project: Apache AsterixDB
>          Issue Type: Bug
>          Components: STO - Storage
>            Reporter: Abdullah Alamoudi
>            Assignee: Abdullah Alamoudi
>
> The IIndexCursor interface is one of the critical interfaces inside asteridxb. It is used to access tuples inside indexes, we have many implementations for it and it is used differently in a different places. We are trying to specify a contract for the interface that all implementors/users of the a cursor have to follow to ensure consistent state and no leaked resources under any circumstances. The scope of this email focuses on the lifecycle of cursors and on the following existing methods:
> -- void open(ICursorInitialState initialState, ISearchPredicate searchPred) throws HyracksDataException;
> -- boolean hasNext() throws HyracksDataException;
> -- void next() throws HyracksDataException;
> -- void close() throws HyracksDataException;
> -- void reset() throws HyracksDataException;
> Currently, these calls are "mostly" used as follows in our code:
> - If there are multiple search predicates:
> cursor = new cursor();
> while (more predicates){
>   cursor.reset()
>   cursor.open(predicate);
>   while (cursor.hasNext()){
>     cursor.next()
>   }
> }
> cursor.close();
> - If there is a single search predicate:
> cursor = new cursor();
> cursor.open(predicate);
> while (cursor.hasNext()){
>   cursor.next()
> }
> cursor.close();
> There are two problems with this: 
> 1. There is no enforcement of any type of contract. For example, one can open a cursor and reset it and then continue to read tuples from the cursor as follows:
> cursor.open(predicate);
> cursor.hasNext()
> cursor.next()
> cursor.reset()
> cursor.hasNext()
> cursor.next()
> and continue to read tuples. This is bug prone and can cause hidden bugs to linger for a long time.
> 2. Naming and symmetry: open calls don't have corresponding close calls "unless we know the cursor will be used with exactly one search predicate"
> With this, the implementation of the cursor lead to either duplicate code or having close() call reset() or the other way around and handling of special cases.
> Moreover, when there are slight differences, often it is easy to make a change in one and forget about the other.
> ==========================================
> To deal with these issues, we are proposing the following:
> 1. change the methods to:
> -- void open(ICursorInitialState initialState, ISearchPredicate searchPred) throws HyracksDataException;
> -- boolean hasNext() throws HyracksDataException;
> -- void next() throws HyracksDataException;
> -- void close(); // used to be reset()
> -- void destroy(); // used to be close()
> The call cycle becomes:
> - If there are multiple search predicates:
> cursor = new cursor();
> while (more predicates){
>   cursor.open(predicate);
>   while (cursor.hasNext()){
>     cursor.next()
>   }
>   cursor.close(); // used to be reset()
> }
> cursor.destroy(); // used to be close()
> - If there is a single search predicate:
> cursor = new cursor();
> cursor.open(predicate);
> while (cursor.hasNext()){
>   cursor.next()
> }
> cursor.close(); // used to be reset()
> cursor.destroy(); // used to be close()
> This way, we have a symmetry and we know that:
> -- A created cursor will always have cursor.destroy() called.
> -- An open cursor will always have cursor.close() called.
> 2. Enforce the cursor state machine as follows:
> The states are:
> CLOSED
> OPEN
> DESTROYED
> When a cursor object is created, it is in the CLOSED state.
> - CLOSED: The only legal calls are open() --> OPEN, or destroy() --> DESTROYED
> - OPEN: The only legal calls are hasNext(), next(), or close() --> CLOSED.
> - DESTROYED: All calls are illegal.
> We can then add tests to ensure that each of the cursors is enforcing the contract.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)