[
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 <jenk...@fulliautomatix.ics.uci.edu>
Integration-Tests: Jenkins <jenk...@fulliautomatix.ics.uci.edu>
Contrib: Jenkins <jenk...@fulliautomatix.ics.uci.edu>
Reviewed-by: abdullah alamoudi <bamou...@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)
