Lazily build MongoClient in MongoClientModule

[ebyhr]

Description

The existing MongoClientModule. defaultConfigurator looks up DNS record when SRV is used.
Connectors shouldn't try accessing remote datasources during the initialization.

Release notes

(x) This is not user-visible or is docs only, and no release notes are required.

[sourcery-ai]

Reviewer's Guide

This PR introduces a MongoClientFactory to defer DNS lookups and replaces the singleton MongoClient in MongoSession with per-operation clients created and closed via the factory, updating dependent classes and module bindings accordingly.

Class diagram for MongoClientFactory and MongoSession changes

classDiagram
    class MongoClientFactory {
        +OpenTelemetry openTelemetry
        +Set<MongoClientSettingConfigurator> configurators
        +MongoClient create()
    }
    class MongoSession {
        -MongoClientFactory clientFactory
        +MongoClient createClient()
        +List<HostAddress> getAddresses()
        +List<String> getAllSchemas()
        +void createSchema(String schemaName)
        +void dropSchema(String schemaName, boolean cascade)
        +void createTable(RemoteTableName name, List<MongoColumnHandle> columns, Optional<String> comment)
        +void dropTable(RemoteTableName remoteTableName)
        +void setTableComment(MongoTableHandle table, Optional<String> comment)
        +void setColumnComment(MongoTableHandle table, String columnName, Optional<String> comment)
        +void renameTable(MongoTableHandle table, SchemaTableName newName)
        +void addColumn(MongoTableHandle table, ColumnMetadata columnMetadata)
        +void renameColumn(MongoTableHandle table, String source, String target)
        +void dropColumn(MongoTableHandle table, String columnName)
        +void setColumnType(MongoTableHandle table, String columnName, Type type)
        +void insertMany(RemoteTableName table, List<Document> documents, InsertManyOptions options)
        +List<MongoIndex> getIndexes(String schemaName, String tableName)
        +long deleteDocuments(RemoteTableName remoteTableName, TupleDomain<ColumnHandle> constraint)
        +MongoCursor<Document> execute(MongoClient client, MongoTableHandle tableHandle, List<MongoColumnHandle> columns)
    }
    MongoSession --> MongoClientFactory : uses

Loading

Class diagram for MongoPageSource and MongoPageSink changes

classDiagram
    class MongoPageSource {
        -MongoClient client
        -MongoCursor<Document> cursor
        +MongoPageSource(MongoSession mongoSession, MongoTableHandle tableHandle, List<MongoColumnHandle> columns, String implicitPrefix)
        +void close()
    }
    class MongoPageSink {
        +CompletableFuture<?> appendPage(Page page)
    }
    MongoPageSource --> MongoSession : uses
    MongoPageSink --> MongoSession : uses

Loading

File-Level Changes

Change	Details	Files
Add MongoClientFactory and bind it in the module	Create MongoClientFactory that injects OpenTelemetry and configurators to lazily build MongoClientSettings Bind MongoClientFactory as a singleton in MongoClientModule instead of a direct MongoClient Update createMongoSession provider to accept MongoClientFactory and remove direct MongoClient creation Remove closing binder for MongoSession	plugin/trino-mongodb/src/main/java/io/trino/plugin/mongodb/MongoClientFactory.java plugin/trino-mongodb/src/main/java/io/trino/plugin/mongodb/MongoClientModule.java
Refactor MongoSession to use per-operation clients	Replace the persistent MongoClient field with MongoClientFactory Wrap every database operation in try-with-resources using factory.create() Adjust method signatures to accept or internally create a MongoClient Remove Closeable implementation and close() method Add createClient() and insertMany() helpers	plugin/trino-mongodb/src/main/java/io/trino/plugin/mongodb/MongoSession.java
Update MongoMetadata, MongoPageSource, and MongoPageSink to use the new client approach	In MongoMetadata.finishInsert, use session.createClient() to obtain a client Change execute() in MongoPageSource to accept a client and close it on source close Introduce a client field in MongoPageSource and close it in close() Modify MongoPageSink to call session.insertMany instead of direct getCollection	plugin/trino-mongodb/src/main/java/io/trino/plugin/mongodb/MongoMetadata.java plugin/trino-mongodb/src/main/java/io/trino/plugin/mongodb/MongoPageSource.java plugin/trino-mongodb/src/main/java/io/trino/plugin/mongodb/MongoPageSink.java
Add SRV connection URL test	Introduce testMongoDbSrv in TestMongoPlugin to verify mongodb+srv URL handling	plugin/trino-mongodb/src/test/java/io/trino/plugin/mongodb/TestMongoPlugin.java

---

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 there - I've reviewed your changes - here's some feedback:

• MongoSession now opens and closes a MongoClient on every operation, which can severely impact performance—consider reusing a single client instance per session to leverage connection pooling and reduce overhead.
• Dropping Closeable from MongoSession removes any clear shutdown path for the clients created by MongoClientFactory—ensure there’s a defined lifecycle to close those clients and avoid resource leaks.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- MongoSession now opens and closes a MongoClient on every operation, which can severely impact performance—consider reusing a single client instance per session to leverage connection pooling and reduce overhead.
- Dropping Closeable from MongoSession removes any clear shutdown path for the clients created by MongoClientFactory—ensure there’s a defined lifecycle to close those clients and avoid resource leaks.

## Individual Comments

### Comment 1
<location> `plugin/trino-mongodb/src/test/java/io/trino/plugin/mongodb/TestMongoPlugin.java:48-49` </location>
<code_context>
         connector.shutdown();
     }
+
+    @Test
+    void testMongoDbSrv()
+    {
+        ConnectorFactory factory = getOnlyElement(new MongoPlugin().getConnectorFactories());
</code_context>

<issue_to_address>
**suggestion (testing):** Consider adding assertions to verify connector initialization and error handling for SRV URLs.

Please add assertions to confirm successful initialization and proper error handling, including cases with invalid SRV URLs and connection failures.
</issue_to_address>

---

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.}

[Copilot]

Pull Request Overview

This pull request introduces a lazy MongoClientFactory to defer DNS lookups during connector initialization when using MongoDB SRV connection strings, addressing the issue where connectors were accessing remote datasources during initialization.

Key changes:

• Replaces eager MongoClient creation with a factory pattern that creates clients on-demand
• Refactors all MongoDB operations to use try-with-resources for client lifecycle management
• Adds integration test to verify SRV connection string handling

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
MongoClientFactory.java	New factory class that creates MongoClient instances lazily to avoid DNS lookups during initialization
MongoClientModule.java	Updated to bind MongoClientFactory instead of creating MongoClient eagerly
MongoSession.java	Refactored to use factory and wrap all operations in try-with-resources blocks
MongoPageSource.java	Updated to create and manage client lifecycle through the session
MongoPageSink.java	Simplified to use session's insertMany method instead of direct collection access
MongoMetadata.java	Updated to use session's createClient method for client access
TestMongoPlugin.java	Added integration test for MongoDB SRV connection handling

---

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

[findepi]

The existing MongoClientModule.createMongoSession looks up DNS record when SRV is used.
Connectors shouldn't try accessing remote datasources during the initialization.

❤️
cc @homar

[findepi]

This requires a comment that this is lazy-loaded shared client instance.
Otherwise it look like we're creating a new client just to close it.

Alternatively, we could inject supplier without memoization and explicitly cache the client on a field in here.

[ebyhr]

Let me leave a comment instead of injecting a supplier.