PR feedback changes.

Author: mbutrovich

src/concurrency/timestamp_ordering_transaction_manager.cpp

@@ -653,9 +653,6 @@ ResultType TimestampOrderingTransactionManager::CommitTransaction(
   // 3. install a new tuple for insert operations.
   // Iterate through each item pointer in the read write set
 
-  // TODO (Pooja): This might be inefficient since we will have to get the
-  // tile_group_header for each entry. Check if this needs to be consolidated
-
   oid_t last_tile_group_id = INVALID_OID;
   storage::TileGroupHeader *tile_group_header = nullptr;
 
@@ -813,8 +810,6 @@ ResultType TimestampOrderingTransactionManager::AbortTransaction(
   }
 
   // Iterate through each item pointer in the read write set
-  // TODO (Pooja): This might be inefficient since we will have to get the
-  // tile_group_header for each entry. Check if this needs to be consolidated
 
   oid_t last_tile_group_id = INVALID_OID;
   storage::TileGroupHeader *tile_group_header = nullptr;

src/concurrency/transaction_context.cpp

@@ -126,15 +126,17 @@ void TransactionContext::RecordInsert(const ItemPointer &location) {
   rw_set_[location] = RWType::INSERT;
 }
 
-void TransactionContext::RecordDelete(const ItemPointer &location) {
+bool TransactionContext::RecordDelete(const ItemPointer &location) {
   PELOTON_ASSERT(rw_set_.count(location) == 0 ||
                  (rw_set_[location] != RWType::DELETE &&
                   rw_set_[location] != RWType::INS_DEL));
   auto rw_set_it = rw_set_.find(location);
   if (rw_set_it != rw_set_.end() && rw_set_it->second == RWType::INSERT) {
     rw_set_it->second = RWType::INS_DEL;
+    return true;
   } else {
     rw_set_[location] = RWType::DELETE;
+    return false;
   }
 }
 

src/include/concurrency/transaction_context.h

@@ -171,8 +171,9 @@ class TransactionContext : public Printable {
    * @brief      Delete the record.
    *
    * @param[in]  <unnamed>  The logical physical location of the record
+   * @return    true if INS_DEL, false if DELETE
    */
-  void RecordDelete(const ItemPointer &);
+  bool RecordDelete(const ItemPointer &);
 
   RWType GetRWType(const ItemPointer &);
 

test/common/lock_free_array_test.cpp

@@ -101,23 +101,21 @@ TEST_F(LockFreeArrayTests, FindValidAndEraseTest) {
   {
     LockFreeArray<value_type> array;
 
-    value_type invalid_value = 6288;
-
     size_t const element_count = 3;
     for (size_t element = 0; element < element_count; ++element) {
       array.Append(element);
     }
 
     // in range, valid
-    EXPECT_EQ(2, array.FindValid(2, invalid_value));
+    EXPECT_EQ(2, array.FindValid(2, INVALID_OID));
 
     // out of range
-    EXPECT_EQ(invalid_value, array.FindValid(6, invalid_value));
+    EXPECT_EQ(INVALID_OID, array.FindValid(6, INVALID_OID));
 
-    array.Erase(2, invalid_value);
+    array.Erase(2, INVALID_OID);
 
     // in range, erased
-    EXPECT_EQ(invalid_value, array.FindValid(2, invalid_value));
+    EXPECT_EQ(INVALID_OID, array.FindValid(2, INVALID_OID));
   }
 }