cmu-db / peloton

The Self-Driving Database Management System
http://pelotondb.io
Apache License 2.0
2.04k stars 622 forks source link

Remove IsolationLevelType::READ_ONLY #1313

Closed apavlo closed 6 years ago

apavlo commented 6 years ago

The way that we are marking that a txn is read-only is by setting it's isolation level to READ_ONLY.

https://github.com/cmu-db/peloton/blob/master/src/include/common/internal_types.h#L425

We currently don't support setting a txn to be read-only from the SQL interface, but the TransactionManager is checking for this IsolationLevel all over the place:

https://github.com/cmu-db/peloton/blob/master/src/concurrency/transaction_manager.cpp#L37

This is incorrect logic. READ_ONLY is not a valid isolation level. A txn can be different isolation levels and also read-only.

We need to remove IsolationLevelType::READ_ONLY and then add a readonly_ flag to the TransactionContext class. The TransactionManager should check this flag not the isolation type.

Nov11 commented 6 years ago

I find this piece of code in TransactionContext. Should I keep it or re-implement it using a new member variable readonly?

/**
   * @brief      Determines if read only.
   *
   * @return     True if read only, False otherwise.
   */
  inline bool IsReadOnly() const {
    return is_written_ == false && insert_count_ == 0;
  }

'iswritten ' is updated when RecordDelete/RecordUpdate are called. I think the function 'IsReadOnly' is not an equivalent to a readonly flag.