diff --git a/src/include/self_driving/pilot/action/abstract_action.h b/src/include/self_driving/pilot/action/abstract_action.h index 5e896dd34b..44f83fc120 100644 --- a/src/include/self_driving/pilot/action/abstract_action.h +++ b/src/include/self_driving/pilot/action/abstract_action.h @@ -3,6 +3,7 @@ #include #include +#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,7 +33,7 @@ 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_; } @@ -38,17 +41,32 @@ class AbstractAction { /** @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 &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 &GetEquivalentActions() const { return equivalent_action_ids_; } + const std::vector &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 equivalent_action_ids_; + std::vector invalidated_action_ids_; + std::vector enabled_action_ids_; std::vector reverse_action_ids_; }; diff --git a/src/include/self_driving/pilot/action/create_index_action.h b/src/include/self_driving/pilot/action/create_index_action.h index b41dbe2426..21f7279648 100644 --- a/src/include/self_driving/pilot/action/create_index_action.h +++ b/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 columns) - : AbstractAction(ActionType::CREATE_INDEX), + CreateIndexAction(catalog::db_oid_t db_oid, std::string index_name, std::string table_name, + std::vector columns) + : AbstractAction(ActionType::CREATE_INDEX, db_oid), index_name_(std::move(index_name)), table_name_(std::move(table_name)), columns_(std::move(columns)) { diff --git a/src/include/self_driving/pilot/action/drop_index_action.h b/src/include/self_driving/pilot/action/drop_index_action.h index 8bf1855564..6b670e0a2c 100644 --- a/src/include/self_driving/pilot/action/drop_index_action.h +++ b/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 columns) - : AbstractAction(ActionType::DROP_INDEX), + DropIndexAction(catalog::db_oid_t db_oid, std::string index_name, std::string table_name, + std::vector columns) + : AbstractAction(ActionType::DROP_INDEX, db_oid), index_name_(std::move(index_name)), table_name_(std::move(table_name)), columns_(std::move(columns)) { diff --git a/src/self_driving/pilot/action/change_knob_action.cpp b/src/self_driving/pilot/action/change_knob_action.cpp index b203465893..291ed628d2 100644 --- a/src/self_driving/pilot/action/change_knob_action.cpp +++ b/src/self_driving/pilot/action/change_knob_action.cpp @@ -8,7 +8,8 @@ namespace noisepage::selfdriving::pilot { template ChangeKnobAction::ChangeKnobAction(settings::Param param, std::string param_name, T change_value, common::ManagedPointer 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), diff --git a/src/self_driving/pilot/action/generators/change_knob_action_generator.cpp b/src/self_driving/pilot/action/generators/change_knob_action_generator.cpp index 8a7fda4aa9..7e7c1a832b 100644 --- a/src/self_driving/pilot/action/generators/change_knob_action_generator.cpp +++ b/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()`). } } } diff --git a/src/self_driving/pilot/action/generators/index_action_generator.cpp b/src/self_driving/pilot/action/generators/index_action_generator.cpp index b4740edd3d..5e1c360104 100644 --- a/src/self_driving/pilot/action/generators/index_action_generator.cpp +++ b/src/self_driving/pilot/action/generators/index_action_generator.cpp @@ -43,6 +43,9 @@ void IndexActionGenerator::FindMissingIndex(const planner::AbstractPlanNode *pla reinterpret_cast(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(new_index_name, table_name, index_columns); + auto create_index_action = std::make_unique(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(new_index_name, table_name, index_columns); + auto drop_index_action = std::make_unique(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); } } } diff --git a/test/self_driving/generate_index_action_test.cpp b/test/self_driving/generate_index_action_test.cpp index 99e741c733..085c9325c2 100644 --- a/test/self_driving/generate_index_action_test.cpp +++ b/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