[ASTERIXDB-2204] Define IIndexCursor interface

• Type: Bug
• Status: Resolved
• Priority: Major
• Resolution: Done
• Component/s: STO - Storage
• Assignee: Abdullah Alamoudi
• Reporter: Abdullah Alamoudi
• Created: 19/Dec/17 10:37
• Updated: 14/Mar/18 23:48
• Resolved: 14/Mar/18 23:48

Description

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.