Merge branch 'master' into lambda_unused_error

poojanilangekar · web-flow · commit 37b448b241b2 · 2018-04-05T18:37:01.000-04:00
diff --git a/src/codegen/updater.cpp b/src/codegen/updater.cpp
@@ -37,6 +37,8 @@ void Updater::Init(storage::DataTable *table,
   // Target list is kept since it is required at a new version update
   target_list_ =
       new TargetList(target_vector, target_vector + target_vector_size);
+
+  statement_write_set_ = new WriteSet();
 }
 
 char *Updater::GetDataPtr(uint32_t tile_group_id, uint32_t tuple_offset) {
@@ -51,6 +53,11 @@ char *Updater::GetDataPtr(uint32_t tile_group_id, uint32_t tuple_offset) {
 
 char *Updater::Prepare(uint32_t tile_group_id, uint32_t tuple_offset) {
   PELOTON_ASSERT(table_ != nullptr && executor_context_ != nullptr);
+
+  if (IsInStatementWriteSet(ItemPointer(tile_group_id, tuple_offset))) {
+    return nullptr;
+  }
+  
   auto *txn = executor_context_->GetTransaction();
   auto tile_group = table_->GetTileGroupById(tile_group_id).get();
   auto *tile_group_header = tile_group->GetHeader();
@@ -74,7 +81,13 @@ char *Updater::Prepare(uint32_t tile_group_id, uint32_t tuple_offset) {
 }
 
 char *Updater::PreparePK(uint32_t tile_group_id, uint32_t tuple_offset) {
+
   PELOTON_ASSERT(table_ != nullptr && executor_context_ != nullptr);
+
+  if (IsInStatementWriteSet(ItemPointer(tile_group_id, tuple_offset))) {
+    return nullptr;
+  }
+
   auto *txn = executor_context_->GetTransaction();
   auto tile_group = table_->GetTileGroupById(tile_group_id).get();
   auto *tile_group_header = tile_group->GetHeader();
@@ -122,7 +135,6 @@ void Updater::Update() {
   auto tile_group = table_->GetTileGroupById(old_location_.block).get();
   auto *tile_group_header = tile_group->GetHeader();
   auto &txn_manager = concurrency::TransactionManagerFactory::GetInstance();
-
   // Either update in-place
   if (is_owner_ == true) {
     txn_manager.PerformUpdate(txn, old_location_);
@@ -143,6 +155,7 @@ void Updater::Update() {
     return;
   }
   txn_manager.PerformUpdate(txn, old_location_, new_location_);
+  AddToStatementWriteSet(new_location_);
   executor_context_->num_processed++;
 }
 
@@ -166,13 +179,15 @@ void Updater::UpdatePK() {
     return;
   }
   txn_manager.PerformInsert(txn, new_location_, index_entry_ptr);
+  AddToStatementWriteSet(new_location_);
   executor_context_->num_processed++;
 }
 
 void Updater::TearDown() {
   // Updater object does not destruct its own data structures
   tile_.reset();
   delete target_list_;
+  delete statement_write_set_;
 }
 
 }  // namespace codegen
diff --git a/src/executor/update_executor.cpp b/src/executor/update_executor.cpp
@@ -53,13 +53,16 @@ bool UpdateExecutor::DInit() {
   PELOTON_ASSERT(target_table_);
   PELOTON_ASSERT(project_info_);
 
+  statement_write_set_.clear();
+
   return true;
 }
 
 bool UpdateExecutor::PerformUpdatePrimaryKey(
     bool is_owner, storage::TileGroup *tile_group,
     storage::TileGroupHeader *tile_group_header, oid_t physical_tuple_id,
     ItemPointer &old_location) {
+
   auto &transaction_manager =
       concurrency::TransactionManagerFactory::GetInstance();
 
@@ -136,7 +139,7 @@ bool UpdateExecutor::PerformUpdatePrimaryKey(
   }
 
   transaction_manager.PerformInsert(current_txn, location, index_entry_ptr);
-
+  statement_write_set_.insert(location);
   return true;
 }
 
@@ -188,6 +191,9 @@ bool UpdateExecutor::DExecute() {
     oid_t physical_tuple_id = pos_lists[0][visible_tuple_id];
 
     ItemPointer old_location(tile_group->GetTileGroupId(), physical_tuple_id);
+    if (IsInStatementWriteSet(old_location)) {
+      continue;
+    }
 
     LOG_TRACE("Visible Tuple id : %u, Physical Tuple id : %u ",
               visible_tuple_id, physical_tuple_id);
@@ -262,6 +268,7 @@ bool UpdateExecutor::DExecute() {
                                 executor_context_);
 
         transaction_manager.PerformUpdate(current_txn, old_location);
+        statement_write_set_.insert(old_location);
       }
     }
     // if we have already obtained the ownership
@@ -359,6 +366,7 @@ bool UpdateExecutor::DExecute() {
                     new_location.offset);
           transaction_manager.PerformUpdate(current_txn, old_location,
                                             new_location);
+          statement_write_set_.insert(new_location);
 
           // TODO: Why don't we also do this in the if branch above?
           executor_context_->num_processed += 1;  // updated one
diff --git a/src/include/codegen/updater.h b/src/include/codegen/updater.h
@@ -67,6 +67,17 @@ class Updater {
 
   char *GetDataPtr(uint32_t tile_group_id, uint32_t tuple_offset);
 
+  // Check if the tuple is in the statement write set
+  inline bool IsInStatementWriteSet(ItemPointer location) {
+    return statement_write_set_->find(location) !=
+           statement_write_set_->end();
+  }
+
+  // Add the updated location to the statement write set
+  inline void AddToStatementWriteSet(ItemPointer& location) {
+    statement_write_set_->insert(location);
+  }
+
  private:
   // Table and executor context from the update translator
   storage::DataTable *table_;
@@ -75,6 +86,15 @@ class Updater {
   // Target list and direct map list pointer from the update translator
   TargetList *target_list_;
 
+  // Write set for tracking newly created tuples inserted by the same statement
+  // This statement-level write set is essential for avoiding the Halloween Problem,
+  // which refers to the phenomenon that an update operation causes a change to
+  // a tuple, potentially allowing this tuple to be visited more than once during
+  // the same operation.
+  // By maintaining the statement-level write set, an update operation will check 
+  // whether the to-be-updated tuple is created by the same operation.
+  WriteSet *statement_write_set_;
+
   // Ownership information
   bool is_owner_;
   bool acquired_ownership_;
diff --git a/src/include/common/internal_types.h b/src/include/common/internal_types.h
@@ -24,6 +24,7 @@
 #include <unistd.h>
 
 #include "tbb/concurrent_vector.h"
+#include "tbb/concurrent_unordered_set.h"
 
 #include "parser/pg_trigger.h"
 #include "type/type_id.h"
@@ -1209,6 +1210,8 @@ std::ostream &operator<<(std::ostream &os, const RWType &type);
 typedef CuckooMap<ItemPointer, RWType, ItemPointerHasher, ItemPointerComparator>
     ReadWriteSet;
 
+typedef tbb::concurrent_unordered_set<ItemPointer, ItemPointerHasher, ItemPointerComparator> WriteSet;
+
 // this enum is to identify why the version should be GC'd.
 enum class GCVersionType {
   INVALID,
diff --git a/src/include/common/item_pointer.h b/src/include/common/item_pointer.h
@@ -44,6 +44,10 @@ class ItemPointer {
     }
   }
 
+  bool operator==(const ItemPointer &rhs) const {
+    return (block == rhs.block && offset == rhs.offset);
+  }
+
 } __attribute__((__aligned__(8))) __attribute__((__packed__));
 
 extern ItemPointer INVALID_ITEMPOINTER;
diff --git a/src/include/executor/update_executor.h b/src/include/executor/update_executor.h
@@ -49,9 +49,22 @@ class UpdateExecutor : public AbstractExecutor {
 
   bool DExecute();
 
+  inline bool IsInStatementWriteSet(ItemPointer &location) {
+    return (statement_write_set_.find(location) != statement_write_set_.end());
+  }
+
  private:
   storage::DataTable *target_table_ = nullptr;
   const planner::ProjectInfo *project_info_ = nullptr;
+
+  // Write set for tracking newly created tuples inserted by the same statement
+  // This statement-level write set is essential for avoiding the Halloween Problem,
+  // which refers to the phenomenon that an update operation causes a change to
+  // a tuple, potentially allowing this tuple to be visited more than once during
+  // the same operation.
+  // By maintaining the statement-level write set, an update operation will check 
+  // whether the to-be-updated tuple is created by the same operation.
+  WriteSet statement_write_set_;
 };
 
 }  // namespace executor
diff --git a/src/include/storage/data_table.h b/src/include/storage/data_table.h
@@ -286,10 +286,18 @@ class DataTable : public AbstractTable {
                        concurrency::TransactionContext *transaction,
                        ItemPointer **index_entry_ptr);
 
+  inline static size_t GetActiveTileGroupCount() {
+    return default_active_tilegroup_count_;
+  }
+
   static void SetActiveTileGroupCount(const size_t active_tile_group_count) {
     default_active_tilegroup_count_ = active_tile_group_count;
   }
 
+  inline static size_t GetActiveIndirectionArrayCount() {
+    return default_active_indirection_array_count_;
+  }
+
   static void SetActiveIndirectionArrayCount(
       const size_t active_indirection_array_count) {
     default_active_indirection_array_count_ = active_indirection_array_count;
@@ -345,9 +353,8 @@ class DataTable : public AbstractTable {
   bool CheckForeignKeyConstraints(const AbstractTuple *tuple,
                                   concurrency::TransactionContext *transaction);
 
- public:
+ private:
   static size_t default_active_tilegroup_count_;
-
   static size_t default_active_indirection_array_count_;
 
  private:
diff --git a/test/executor/loader_test.cpp b/test/executor/loader_test.cpp
@@ -132,22 +132,32 @@ TEST_F(LoaderTests, LoadingTest) {
   auto expected_tile_group_count = 0;
 
   int total_tuple_count = loader_threads_count * tilegroup_count_per_loader * TEST_TUPLES_PER_TILEGROUP;
-  int max_cached_tuple_count = TEST_TUPLES_PER_TILEGROUP * storage::DataTable::default_active_tilegroup_count_;
-  int max_unfill_cached_tuple_count = (TEST_TUPLES_PER_TILEGROUP - 1) * storage::DataTable::default_active_tilegroup_count_;
+  int max_cached_tuple_count =
+      TEST_TUPLES_PER_TILEGROUP * storage::DataTable::GetActiveTileGroupCount();
+  int max_unfill_cached_tuple_count =
+      (TEST_TUPLES_PER_TILEGROUP - 1) *
+      storage::DataTable::GetActiveTileGroupCount();
 
   if (total_tuple_count - max_cached_tuple_count <= 0) {
     if (total_tuple_count <= max_unfill_cached_tuple_count) {
-      expected_tile_group_count = storage::DataTable::default_active_tilegroup_count_;
+      expected_tile_group_count = storage::DataTable::GetActiveTileGroupCount();
     } else {
-      expected_tile_group_count = storage::DataTable::default_active_tilegroup_count_ + total_tuple_count - max_unfill_cached_tuple_count; 
+      expected_tile_group_count =
+          storage::DataTable::GetActiveTileGroupCount() + total_tuple_count -
+          max_unfill_cached_tuple_count;
     }
   } else {
-    int filled_tile_group_count = total_tuple_count / max_cached_tuple_count * storage::DataTable::default_active_tilegroup_count_;
+    int filled_tile_group_count = total_tuple_count / max_cached_tuple_count * storage::DataTable::GetActiveTileGroupCount();
     
     if (total_tuple_count - filled_tile_group_count * TEST_TUPLES_PER_TILEGROUP - max_unfill_cached_tuple_count <= 0) {
-      expected_tile_group_count = filled_tile_group_count + storage::DataTable::default_active_tilegroup_count_;
+      expected_tile_group_count = filled_tile_group_count +
+                                  storage::DataTable::GetActiveTileGroupCount();
     } else {
-      expected_tile_group_count = filled_tile_group_count + storage::DataTable::default_active_tilegroup_count_ + (total_tuple_count - filled_tile_group_count - max_unfill_cached_tuple_count); 
+      expected_tile_group_count =
+          filled_tile_group_count +
+          storage::DataTable::GetActiveTileGroupCount() +
+          (total_tuple_count - filled_tile_group_count -
+           max_unfill_cached_tuple_count);
     }
   }
 
diff --git a/test/performance/insert_performance_test.cpp b/test/performance/insert_performance_test.cpp
@@ -119,22 +119,33 @@ TEST_F(InsertPerformanceTests, LoadingTest) {
   auto expected_tile_group_count = 0;
 
   int total_tuple_count = loader_threads_count * tilegroup_count_per_loader * TEST_TUPLES_PER_TILEGROUP;
-  int max_cached_tuple_count = TEST_TUPLES_PER_TILEGROUP * storage::DataTable::default_active_tilegroup_count_;
-  int max_unfill_cached_tuple_count = (TEST_TUPLES_PER_TILEGROUP - 1) * storage::DataTable::default_active_tilegroup_count_;
+  int max_cached_tuple_count =
+      TEST_TUPLES_PER_TILEGROUP * storage::DataTable::GetActiveTileGroupCount();
+  int max_unfill_cached_tuple_count =
+      (TEST_TUPLES_PER_TILEGROUP - 1) *
+      storage::DataTable::GetActiveTileGroupCount();
 
   if (total_tuple_count - max_cached_tuple_count <= 0) {
     if (total_tuple_count <= max_unfill_cached_tuple_count) {
-      expected_tile_group_count = storage::DataTable::default_active_tilegroup_count_;
+      expected_tile_group_count = storage::DataTable::GetActiveTileGroupCount();
     } else {
-      expected_tile_group_count = storage::DataTable::default_active_tilegroup_count_ + total_tuple_count - max_unfill_cached_tuple_count; 
+      expected_tile_group_count =
+          storage::DataTable::GetActiveTileGroupCount() + total_tuple_count -
+          max_unfill_cached_tuple_count;
     }
   } else {
-    int filled_tile_group_count = total_tuple_count / max_cached_tuple_count * storage::DataTable::default_active_tilegroup_count_;
-    
+    int filled_tile_group_count = total_tuple_count / max_cached_tuple_count *
+                                  storage::DataTable::GetActiveTileGroupCount();
+
     if (total_tuple_count - filled_tile_group_count * TEST_TUPLES_PER_TILEGROUP - max_unfill_cached_tuple_count <= 0) {
-      expected_tile_group_count = filled_tile_group_count + storage::DataTable::default_active_tilegroup_count_;
+      expected_tile_group_count = filled_tile_group_count +
+                                  storage::DataTable::GetActiveTileGroupCount();
     } else {
-      expected_tile_group_count = filled_tile_group_count + storage::DataTable::default_active_tilegroup_count_ + (total_tuple_count - filled_tile_group_count - max_unfill_cached_tuple_count); 
+      expected_tile_group_count =
+          filled_tile_group_count +
+          storage::DataTable::GetActiveTileGroupCount() +
+          (total_tuple_count - filled_tile_group_count -
+           max_unfill_cached_tuple_count);
     }
   }
 
diff --git a/test/sql/update_sql_test.cpp b/test/sql/update_sql_test.cpp

Original file line number	Diff line number	Diff line change
`@@ -44,6 +44,10 @@ class ItemPointer {`
`44`	`44`	`}`
`45`	`45`	`}`
`46`	`46`
	`47`	`+ bool operator==(const ItemPointer &rhs) const {`
	`48`	`+ return (block == rhs.block && offset == rhs.offset);`
	`49`	`+ }`
	`50`	`+`
`47`	`51`	`} __attribute__((__aligned__(8))) __attribute__((__packed__));`
`48`	`52`
`49`	`53`	`extern ItemPointer INVALID_ITEMPOINTER;`