refactor: Replace getAllConnectors with ConnectorRegistry APIs

[mbasmanova]

Summary:
Replace getAllConnectors() with ConnectorRegistry::unregisterAll() in
PrestoServer::unregisterConnectors(). Change PeriodicTaskManager to
accept only the HiveConnectors it needs instead of the full connector
map, using ConnectorRegistry::findAll().

Differential Revision: D98911125

== RELEASE NOTES ==

General Changes

• None. Internal refactoring only.

[sourcery-ai]

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Refactors connector handling to use ConnectorRegistry APIs directly, narrowing PeriodicTaskManager’s dependency to only Hive connectors and simplifying connector unregistration in PrestoServer.

Sequence diagram for PrestoServer lifecycle connector handling

sequenceDiagram
    participant PrestoServer
    participant ConnectorRegistry
    participant PeriodicTaskManager

    PrestoServer->>ConnectorRegistry: findAllHiveConnector()
    ConnectorRegistry-->>PrestoServer: hiveConnectors

    PrestoServer->>PeriodicTaskManager: constructor(hiveConnectors, server)
    PrestoServer->>PeriodicTaskManager: start()

    loop periodic_connector_stats
        PeriodicTaskManager->>PeriodicTaskManager: addConnectorStatsTask()
        Note over PeriodicTaskManager: Iterate hiveConnectors_ and create HiveConnectorStatsReporter instances
    end

    PrestoServer->>PrestoServer: shutdown_sequence()
    PrestoServer->>ConnectorRegistry: unregisterAll()
    ConnectorRegistry-->>PrestoServer: all_connectors_unregistered

Loading

Class diagram for refactored connector handling

classDiagram
    class PrestoServer {
      +void run()
      +void unregisterConnectors()
    }

    class PeriodicTaskManager {
      +PeriodicTaskManager(std::shared_ptr<folly::CPUThreadPoolExecutor> driverCPUExecutor, std::shared_ptr<folly::IOThreadPoolExecutor> spillerExecutor, TaskManager* taskManager, const velox::memory::MemoryAllocator* memoryAllocator, const velox::cache::AsyncDataCache* asyncDataCache, std::vector<std::shared_ptr<velox::connector::hive::HiveConnector>> hiveConnectors, PrestoServer* server)
      +void start()
      +void addConnectorStatsTask()
      -std::shared_ptr<folly::CPUThreadPoolExecutor> driverCPUExecutor_
      -std::shared_ptr<folly::IOThreadPoolExecutor> spillerExecutor_
      -TaskManager* taskManager_
      -const velox::memory::MemoryAllocator* memoryAllocator_
      -const velox::cache::AsyncDataCache* asyncDataCache_
      -const velox::memory::MemoryArbitrator* arbitrator_
      -std::vector<std::shared_ptr<velox::connector::hive::HiveConnector>> hiveConnectors_
      -PrestoServer* server_
    }

    class ConnectorRegistry {
      +static std::vector<std::shared_ptr<velox::connector::hive::HiveConnector>> findAllHiveConnector()
      +static void unregisterAll()
    }

    class HiveConnector {
    }

    PrestoServer --> PeriodicTaskManager : creates
    PrestoServer ..> ConnectorRegistry : uses
    PeriodicTaskManager o--> HiveConnector : reports_stats_for

Loading

File-Level Changes

Change	Details	Files
Use ConnectorRegistry-based APIs in PrestoServer for connector lookup and unregistration.	Include ConnectorRegistry header in PrestoServer.cpp. Replace getAllConnectors() usage in PrestoServer::run() with ConnectorRegistry::findAll(). Simplify PrestoServer::unregisterConnectors() to call ConnectorRegistry::unregisterAll() and adjust logging accordingly.	presto-native-execution/presto_cpp/main/PrestoServer.cpp
Narrow PeriodicTaskManager dependencies to a list of HiveConnector instances instead of the full connector map.	Change PeriodicTaskManager constructor signature to accept a vector of shared_ptr rather than a map of generic connectors. Store the provided Hive connectors in a hiveConnectors_ member instead of connectors_. Update addConnectorStatsTask() to iterate directly over hiveConnectors_ without dynamic_pointer_cast. Adjust forward declarations to reference velox::connector::hive::HiveConnector instead of the generic Connector.	presto-native-execution/presto_cpp/main/PeriodicTaskManager.cpp presto-native-execution/presto_cpp/main/PeriodicTaskManager.h
Wire in the new dependency on the connector registry library for the Presto main binary.	Link velox_connector_registry in the main Presto C++ target so ConnectorRegistry symbols are available.	presto-native-execution/presto_cpp/main/CMakeLists.txt

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[sourcery-ai]

Hey - I've left some high level feedback:

• In PrestoServer::unregisterConnectors, consider preserving some of the previous logging (e.g., number and names of connectors being unregistered) so that shutdown behavior remains as observable and debuggable as before.
• In PeriodicTaskManager, you now copy the full std::vector<std::shared_ptr<hive::HiveConnector>> into the member hiveConnectors_; if the manager does not need ownership or mutation, consider taking it as a const& or making the member const to avoid unnecessary copying and clarify lifetime/intent.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- In `PrestoServer::unregisterConnectors`, consider preserving some of the previous logging (e.g., number and names of connectors being unregistered) so that shutdown behavior remains as observable and debuggable as before.
- In `PeriodicTaskManager`, you now copy the full `std::vector<std::shared_ptr<hive::HiveConnector>>` into the member `hiveConnectors_`; if the manager does not need ownership or mutation, consider taking it as a `const&` or making the member `const` to avoid unnecessary copying and clarify lifetime/intent.

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[aditi-pandit]

Thanks @mbasmanova

[mbasmanova]

@aditi-pandit Aditi, any chance you could stamp again?

[aditi-pandit]

Sorry about the delay.