Fault tolerant execution for Mongo connector #15062

mwd410 · 2022-11-16T19:59:26Z

Description

This provides support for FTE with the Mongo connector. It uses the same paradigm as my previous PR. We only use this new paradigm with retries enabled, otherwise we use the old paradigm. This is important because this also increases our minimum mongo version from 4.0 to 4.2, but only when FTE is enabled - this allows clients using Mongo on 4.0 to continue upgrading their trino version without having to upgrade their mongo, so long as they can live without FTE.

Non-technical explanation

This provides support for fault tolerant execution on MongoDB. If you want to enable FTE, we require a minimum of Mongo version 4.2, otherwise we still support 4.0.

Release notes

( ) This is not user-visible or docs only and no release notes are required.
( ) Release notes are required, please propose a release note for me.
(x) Release notes are required, with the following suggested text:

# MongoDB
* Upgrade minimum required MongoDB version to 4.2. ({issue}`15062`)
* Add support for writes when [fault-tolerant
  execution](/admin/fault-tolerant-execution) is enabled. ({issue}`15062`)

plugin/trino-mongodb/src/main/java/io/trino/plugin/mongodb/MongoMetadata.java

plugin/trino-mongodb/src/main/java/io/trino/plugin/mongodb/MongoSession.java

plugin/trino-mongodb/src/test/java/io/trino/plugin/mongodb/MongoServer.java

mosabua · 2022-11-17T17:02:48Z

docs/src/main/sphinx/connector/mongodb.rst

@@ -14,7 +14,7 @@ Requirements

 To connect to MongoDB, you need:

-* MongoDB 4.0 or higher.
+* MongoDB 4.2 or higher.


This should go into release notes @colebow

Is there something I need to do for that?

Nope, you're good! You've ticked the box in the PR template, so I've got it from here. 😄

losipiuk · 2022-11-22T15:13:12Z

plugin/trino-mongodb/src/test/java/io/trino/plugin/mongodb/MongoQueryRunner.java

-            Map<String, String> properties = ImmutableMap.of(
-                    "mongodb.case-insensitive-name-matching", "true",
-                    "mongodb.connection-url", server.getConnectionString().toString());
+            connectorProperties = new HashMap(ImmutableMap.copyOf(connectorProperties));


Why HashMap? I would stick to the pattern and use ImmutableMap.Builder here,

PostgreSqlQueryRunner uses a HashMap, but OK.

Using HashMap makes it possible to override the default connectorProperties. If i used an ImmutableMap.builder(), the builder doesn't have a putIfAbsent, and our style checks require i use buildOrThrow() rather than build(), so it'd be very awkward to allow that.

losipiuk · 2022-11-22T15:17:15Z

plugin/trino-mongodb/src/main/java/io/trino/plugin/mongodb/MongoOutputTableHandle.java

+        this.temporaryTableName = requireNonNull(temporaryTableName, "temporaryTableName is null");
+        this.pageSinkIdColumnName = requireNonNull(pageSinkIdColumnName, "pageSinkIdColumnName is null");


verify either both or none set. Those always come in pairs, right?

gah - i added that to MongoInsertTableHandle and forgot to here.

losipiuk · 2022-11-22T15:18:55Z

plugin/trino-mongodb/src/main/java/io/trino/plugin/mongodb/MongoMetadata.java

+                .addAll(columns)
+                .add(pageSinkIdColumn)
+                .build();
+        SchemaTableName tempTable = SchemaTableName.schemaTableName(tableMetadata.getTable().getSchemaName(), generateTemporaryTableName());


nit: s/temp/temporary/

losipiuk · 2022-11-22T15:19:26Z

plugin/trino-mongodb/src/main/java/io/trino/plugin/mongodb/MongoMetadata.java


-        MongoTableHandle table = (MongoTableHandle) tableHandle;
-        List<MongoColumnHandle> columns = mongoSession.getTable(table.getSchemaTableName()).getColumns();
+        SchemaTableName tempTable = SchemaTableName.schemaTableName(handle.getSchemaTableName().getSchemaName(), generateTemporaryTableName());


losipiuk · 2022-11-22T15:22:59Z

plugin/trino-mongodb/src/main/java/io/trino/plugin/mongodb/MongoMetadata.java

+
+        try {
+            // Create the temporary page sink ID table
+            SchemaTableName pageSinkTable = SchemaTableName.schemaTableName(tempTable.getSchemaName(), generateTemporaryTableName());


pageSinkIdsTable?

losipiuk · 2022-11-22T15:24:29Z

plugin/trino-mongodb/src/main/java/io/trino/plugin/mongodb/MongoMetadata.java

        return Optional.empty();
    }

+    private void finishInsert(
+            SchemaTableName targetTable,
+            SchemaTableName tempTable,


losipiuk · 2022-11-22T15:24:54Z

plugin/trino-mongodb/src/main/java/io/trino/plugin/mongodb/MongoMetadata.java

+            closer.register(() -> mongoSession.dropTable(pageSinkTable));
+
+            // Insert all the page sink IDs into the page sink ID table
+            MongoCollection<Document> pageSinkCollection = mongoSession.getCollection(pageSinkTable);


pageSinkIds?

losipiuk · 2022-11-22T15:26:25Z

plugin/trino-mongodb/src/main/java/io/trino/plugin/mongodb/MongoMetadata.java

+                    .collect(toImmutableList());
+            pageSinkCollection.insertMany(pageSinkIds);
+
+            MongoCollection<Document> tempCollection = mongoSession.getCollection(tempTable);


unfilteredInsertedRows?

stuck with temporaryCollection

losipiuk

LGTM - some naming suggestions.
@ebyhr do you have some more actual comments or is it good to go?

ebyhr

Could you rebase on upstream and resolve conflicts?

plugin/trino-mongodb/src/main/java/io/trino/plugin/mongodb/MongoTable.java

...olerant-tests/src/test/java/io/trino/faulttolerant/mongodb/BaseMongoFailureRecoveryTest.java

losipiuk · 2022-11-24T15:54:59Z

🎉

cla-bot bot added the cla-signed label Nov 16, 2022

mwd410 requested review from ebyhr and losipiuk November 16, 2022 20:00

mwd410 force-pushed the mdeady-mongo-fte-3877 branch from 87e28de to 772653b Compare November 16, 2022 20:02

github-actions bot added the docs label Nov 16, 2022

mwd410 force-pushed the mdeady-mongo-fte-3877 branch from 772653b to 65c4fce Compare November 16, 2022 21:03

ebyhr reviewed Nov 17, 2022

View reviewed changes

mwd410 force-pushed the mdeady-mongo-fte-3877 branch from 65c4fce to 9f263b2 Compare November 17, 2022 16:26

mosabua reviewed Nov 17, 2022

View reviewed changes

mwd410 force-pushed the mdeady-mongo-fte-3877 branch 2 times, most recently from 0360f33 to 78aa2b7 Compare November 21, 2022 18:40

ebyhr mentioned this pull request Nov 21, 2022

Introduce RemoteTableName class to MongoDB #15114

Merged

losipiuk reviewed Nov 22, 2022

View reviewed changes

losipiuk approved these changes Nov 22, 2022

View reviewed changes

mwd410 force-pushed the mdeady-mongo-fte-3877 branch from 78aa2b7 to 0208bd9 Compare November 22, 2022 15:40

ebyhr approved these changes Nov 23, 2022

View reviewed changes

plugin/trino-mongodb/src/main/java/io/trino/plugin/mongodb/MongoTable.java Outdated Show resolved Hide resolved

mwd410 added 2 commits November 23, 2022 14:04

Add more config options for MongoQueryRunner

b8947c4

Upgrade minimum supported version to 4.2 in MongoDB

aed7b63

mwd410 force-pushed the mdeady-mongo-fte-3877 branch from 0208bd9 to e85eb93 Compare November 23, 2022 20:24

ebyhr reviewed Nov 23, 2022

View reviewed changes

...olerant-tests/src/test/java/io/trino/faulttolerant/mongodb/BaseMongoFailureRecoveryTest.java Show resolved Hide resolved

Fault tolerant execution for MongoDB

8af798e

mwd410 force-pushed the mdeady-mongo-fte-3877 branch from e85eb93 to 8af798e Compare November 24, 2022 03:28

losipiuk merged commit 6547e26 into trinodb:master Nov 24, 2022

github-actions bot added this to the 404 milestone Nov 24, 2022

ebyhr mentioned this pull request Nov 24, 2022

Release notes for 405 #15058

Closed

findepi mentioned this pull request Nov 25, 2022

Flaky test TestMongoTaskFailureRecoveryTest #15195

Closed

mwd410 deleted the mdeady-mongo-fte-3877 branch November 28, 2022 14:54

colebow mentioned this pull request Nov 28, 2022

Add Trino 405 release notes #15139

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fault tolerant execution for Mongo connector #15062

Fault tolerant execution for Mongo connector #15062

mwd410 commented Nov 16, 2022 •

edited by ebyhr

Loading

mosabua Nov 17, 2022

mwd410 Nov 17, 2022

colebow Nov 17, 2022

losipiuk Nov 22, 2022

mwd410 Nov 22, 2022

mwd410 Nov 22, 2022

losipiuk Nov 22, 2022

mwd410 Nov 22, 2022

losipiuk Nov 22, 2022

losipiuk Nov 22, 2022

losipiuk Nov 22, 2022

losipiuk Nov 22, 2022

losipiuk Nov 22, 2022

losipiuk Nov 22, 2022

mwd410 Nov 22, 2022

losipiuk left a comment

ebyhr left a comment

losipiuk commented Nov 24, 2022

		this.temporaryTableName = requireNonNull(temporaryTableName, "temporaryTableName is null");
		this.pageSinkIdColumnName = requireNonNull(pageSinkIdColumnName, "pageSinkIdColumnName is null");

Fault tolerant execution for Mongo connector #15062

Fault tolerant execution for Mongo connector #15062

Conversation

mwd410 commented Nov 16, 2022 • edited by ebyhr Loading

Description

Non-technical explanation

Release notes

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

losipiuk left a comment

Choose a reason for hiding this comment

ebyhr left a comment

Choose a reason for hiding this comment

losipiuk commented Nov 24, 2022

mwd410 commented Nov 16, 2022 •

edited by ebyhr

Loading