Additional AbstractAction Information (#1445)

Author: linmagit

src/include/self_driving/pilot/action/abstract_action.h

@@ -3,6 +3,7 @@
 #include <string>
 #include <vector>
 
+#include "catalog/catalog_defs.h"
 #include "common/resource_tracker.h"
 #include "self_driving/pilot/action/action_defs.h"
 
@@ -16,8 +17,10 @@ class AbstractAction {
   /**
    * Constructor for the base AbstractAction.
    * @param family The family that this action belongs to
+   * @param db_oid The ID of the database that this action belongs to
    */
-  explicit AbstractAction(ActionType family) : action_family_(family), id_(action_id_counter++) {}
+  explicit AbstractAction(ActionType family, catalog::db_oid_t db_oid)
+      : action_family_(family), db_oid_(db_oid), id_(action_id_counter++) {}
 
   virtual ~AbstractAction() = default;
 
@@ -30,25 +33,40 @@ class AbstractAction {
   }
 
   /** @return The estimated runtime metrics for this action */
-  const common::ResourceTracker::Metrics &GetActualMetrics() { return estimated_metrics_; }
+  const common::ResourceTracker::Metrics &GetEstimatedMetrics() { return estimated_metrics_; }
 
   /** @return This action's ID */
   action_id_t GetActionID() const { return id_; }
 
   /** @return This action's family */
   ActionType GetActionFamily() const { return action_family_; }
 
+  /** @return This action's database oid */
+  catalog::db_oid_t GetDatabaseOid() const { return db_oid_; }
+
+  /**
+   * Add an invalidated action
+   * @param id Action ID
+   */
+  void AddInvalidatedAction(action_id_t id) { invalidated_action_ids_.emplace_back(id); }
+
   /**
-   * Add an equivalent action
+   * Get the invalidated action ids if this action is applied
+   * @return Action ID vector
+   */
+  const std::vector<action_id_t> &GetInvalidatedActions() const { return invalidated_action_ids_; }
+
+  /**
+   * Add an invalidated action
    * @param id Action ID
    */
-  void AddEquivalentAction(action_id_t id) { equivalent_action_ids_.emplace_back(id); }
+  void AddEnabledAction(action_id_t id) { enabled_action_ids_.emplace_back(id); }
 
   /**
-   * Get the equivalent action ids
+   * Get the enabled action ids if this action is applied
    * @return Action ID vector
    */
-  const std::vector<action_id_t> &GetEquivalentActions() const { return equivalent_action_ids_; }
+  const std::vector<action_id_t> &GetEnabledActions() const { return enabled_action_ids_; }
 
   /**
    * Add a reverse action
@@ -87,10 +105,13 @@ class AbstractAction {
 
   ActionType action_family_;
 
+  catalog::db_oid_t db_oid_;
+
   /** ID is unique for an action among on planning process (one MCTS) */
   action_id_t id_;
 
-  std::vector<action_id_t> equivalent_action_ids_;
+  std::vector<action_id_t> invalidated_action_ids_;
+  std::vector<action_id_t> enabled_action_ids_;
   std::vector<action_id_t> reverse_action_ids_;
 };
 

src/include/self_driving/pilot/action/create_index_action.h

@@ -22,12 +22,14 @@ class CreateIndexAction : public AbstractAction {
  public:
   /**
    * Construct CreateIndexAction
+   * @param db_oid Database id of the index
    * @param index_name Name of the index
    * @param table_name The table to create index on
    * @param columns The columns to build index on
    */
-  CreateIndexAction(std::string index_name, std::string table_name, std::vector<IndexColumn> columns)
-      : AbstractAction(ActionType::CREATE_INDEX),
+  CreateIndexAction(catalog::db_oid_t db_oid, std::string index_name, std::string table_name,
+                    std::vector<IndexColumn> columns)
+      : AbstractAction(ActionType::CREATE_INDEX, db_oid),
         index_name_(std::move(index_name)),
         table_name_(std::move(table_name)),
         columns_(std::move(columns)) {

src/include/self_driving/pilot/action/drop_index_action.h

@@ -16,12 +16,14 @@ class DropIndexAction : public AbstractAction {
  public:
   /**
    * Construct DropIndexAction
+   * @param db_oid Database id of the index
    * @param index_name The name of the index
    * @param table_name The table to create index on
    * @param columns The columns to build index on
    */
-  DropIndexAction(std::string index_name, std::string table_name, std::vector<IndexColumn> columns)
-      : AbstractAction(ActionType::DROP_INDEX),
+  DropIndexAction(catalog::db_oid_t db_oid, std::string index_name, std::string table_name,
+                  std::vector<IndexColumn> columns)
+      : AbstractAction(ActionType::DROP_INDEX, db_oid),
         index_name_(std::move(index_name)),
         table_name_(std::move(table_name)),
         columns_(std::move(columns)) {

src/self_driving/pilot/action/change_knob_action.cpp

@@ -8,7 +8,8 @@ namespace noisepage::selfdriving::pilot {
 template <class T>
 ChangeKnobAction<T>::ChangeKnobAction(settings::Param param, std::string param_name, T change_value,
                                       common::ManagedPointer<settings::SettingsManager> settings_manager)
-    : AbstractAction(ActionType::CHANGE_KNOB),
+    // ChangeKnobAction should not need a database oid so put the invalid oid here
+    : AbstractAction(ActionType::CHANGE_KNOB, catalog::INVALID_DATABASE_OID),
       param_(param),
       param_name_(std::move(param_name)),
       change_value_(change_value),

src/self_driving/pilot/action/generators/change_knob_action_generator.cpp

@@ -61,6 +61,9 @@ void ChangeKnobActionGenerator::GenerateActionForType(
 
       // Populate the reverse actions
       action_map->at(first_action_id)->AddReverseAction(second_action_id);
+
+      // Note: change knob actions do not have any enabled/invalidated actions since they're based on deltas and
+      // always valid (unless exceeds the knob setting limit, which is handled by `ChangeKnobAction::IsValid()`).
     }
   }
 }

src/self_driving/pilot/action/generators/index_action_generator.cpp

@@ -43,6 +43,9 @@ void IndexActionGenerator::FindMissingIndex(const planner::AbstractPlanNode *pla
         reinterpret_cast<const planner::IndexScanPlanNode *>(plan)->GetCoverAllColumns())
       return;
 
+    // Get database oid
+    catalog::db_oid_t db_oid = scan_plan->GetDatabaseOid();
+
     // Get table oid
     catalog::table_oid_t table_oid;
     if (plan_type == planner::PlanNodeType::INDEXSCAN)
@@ -85,20 +88,27 @@ void IndexActionGenerator::FindMissingIndex(const planner::AbstractPlanNode *pla
       // TODO(Lin): Don't insert potentially duplicated actions
       // Generate the create index action
       std::string new_index_name = IndexActionUtil::GenerateIndexName(table_name, index_columns);
-      auto create_index_action = std::make_unique<CreateIndexAction>(new_index_name, table_name, index_columns);
+      auto create_index_action = std::make_unique<CreateIndexAction>(db_oid, new_index_name, table_name, index_columns);
       action_id_t create_index_action_id = create_index_action->GetActionID();
+      // Create index would invalidate itself
+      create_index_action->AddInvalidatedAction(create_index_action_id);
       action_map->emplace(create_index_action_id, std::move(create_index_action));
       // Only the create index action is valid
       candidate_actions->emplace_back(create_index_action_id);
 
       // Generate the drop index action
-      auto drop_index_action = std::make_unique<DropIndexAction>(new_index_name, table_name, index_columns);
+      auto drop_index_action = std::make_unique<DropIndexAction>(db_oid, new_index_name, table_name, index_columns);
       action_id_t drop_index_action_id = drop_index_action->GetActionID();
+      // Drop index would invalidate itself
+      drop_index_action->AddInvalidatedAction(drop_index_action_id);
       action_map->emplace(drop_index_action_id, std::move(drop_index_action));
 
       // Populate the reverse actions
       action_map->at(create_index_action_id)->AddReverseAction(drop_index_action_id);
       action_map->at(drop_index_action_id)->AddReverseAction(create_index_action_id);
+      // Populate the enabled actions
+      action_map->at(create_index_action_id)->AddEnabledAction(drop_index_action_id);
+      action_map->at(drop_index_action_id)->AddEnabledAction(create_index_action_id);
     }
   }
 }

test/self_driving/generate_index_action_test.cpp

@@ -82,6 +82,10 @@ TEST_F(GenerateIndexAction, GenerateSingleColumnIndexAction) {
   action_id_t second_action_id = action_ids[1];
   EXPECT_EQ(action_map[first_action_id]->GetReverseActions()[0], second_action_id);
   EXPECT_EQ(action_map[second_action_id]->GetReverseActions()[0], first_action_id);
+  EXPECT_EQ(action_map[first_action_id]->GetEnabledActions()[0], second_action_id);
+  EXPECT_EQ(action_map[second_action_id]->GetEnabledActions()[0], first_action_id);
+  EXPECT_EQ(action_map[first_action_id]->GetInvalidatedActions()[0], first_action_id);
+  EXPECT_EQ(action_map[second_action_id]->GetInvalidatedActions()[0], second_action_id);
 }
 
 // NOLINTNEXTLINE