924060929 commented on code in PR #64096:
URL: https://github.com/apache/doris/pull/64096#discussion_r3548331474
##########
fe/fe-connector/fe-connector-trino/src/main/java/org/apache/doris/connector/trino/TrinoDorisConnector.java:
##########
@@ -73,6 +74,14 @@ public ConnectorScanPlanProvider getScanPlanProvider() {
return new TrinoScanPlanProvider(this);
}
+ @Override
+ public void preCreateValidation(ConnectorValidationContext context) {
Review Comment:
**`preCreateValidation` eagerly loads plugins + constructs the Trino
connector at CREATE CATALOG time** (low, but I3-visible). Legacy validated only
property presence at create and built the connector lazily at first access
(master TrinoConnectorExternalCatalog.checkProperties). Now CREATE CATALOG
fails where it previously succeeded — e.g. a provisioning script that creates
the catalog before dispatching the connector plugins dir, or a config typo the
connector factory validates eagerly. And edit-log replay skips
preCreateValidation, so the identical catalog that can't be created can still
materialize on a restarted FE (create-vs-replay divergence). The comment says
this is intentional (fail-fast on misconfig); if so, it's worth a release note
since it changes the CREATE CATALOG contract for every migrated connector.
##########
fe/fe-connector/fe-connector-trino/src/main/java/org/apache/doris/connector/trino/TrinoConnectorDorisMetadata.java:
##########
@@ -187,17 +203,132 @@ public ConnectorTableSchema getTableSchema(
}
@Override
- public Map<String,
org.apache.doris.connector.api.handle.ConnectorColumnHandle> getColumnHandles(
+ public Map<String, ConnectorColumnHandle> getColumnHandles(
ConnectorSession session, ConnectorTableHandle handle) {
TrinoTableHandle trinoHandle = (TrinoTableHandle) handle;
Map<String, ColumnHandle> trinoHandles =
trinoHandle.getColumnHandleMap();
if (trinoHandles == null || trinoHandles.isEmpty()) {
return Collections.emptyMap();
}
- Map<String,
org.apache.doris.connector.api.handle.ConnectorColumnHandle> result = new
HashMap<>();
+ Map<String, ConnectorColumnHandle> result = new HashMap<>();
for (String colName : trinoHandles.keySet()) {
result.put(colName, new TrinoColumnHandle(colName));
}
return result;
}
+
+ @Override
+ public Optional<FilterApplicationResult<ConnectorTableHandle>> applyFilter(
+ ConnectorSession session,
+ ConnectorTableHandle handle,
+ ConnectorFilterConstraint constraint) {
+ TrinoTableHandle dorisHandle = (TrinoTableHandle) handle;
+ ConnectorExpression expression = constraint.getExpression();
+
+ TrinoPredicateConverter converter = new TrinoPredicateConverter(
+ dorisHandle.getColumnHandleMap(),
+ dorisHandle.getColumnMetadataMap());
+ TupleDomain<ColumnHandle> tupleDomain = converter.convert(expression);
+ if (tupleDomain.isAll()) {
+ return Optional.empty();
+ }
+
+ io.trino.spi.connector.ConnectorSession connSession =
+ trinoSession.toConnectorSession(trinoCatalogHandle);
+ io.trino.spi.connector.ConnectorTransactionHandle txn =
+
trinoConnector.beginTransaction(IsolationLevel.READ_UNCOMMITTED, true, true);
+ io.trino.spi.connector.ConnectorMetadata metadata =
+ trinoConnector.getMetadata(connSession, txn);
+
+
Optional<ConstraintApplicationResult<io.trino.spi.connector.ConnectorTableHandle>>
trinoResult =
Review Comment:
**Trino transactions are opened per metadata call and never closed**
(medium). Every method here (`getTableHandle`, `getTableSchema`, `applyFilter`,
`applyProjection`, ...) does `trinoConnector.beginTransaction(READ_UNCOMMITTED,
...)` + `getMetadata(session, txn)` with no matching commit/rollback anywhere
in the class (grep finds zero — `cleanupQuery` in TrinoScanPlanProvider is
query cleanup, not transaction cleanup). Legacy cached ONE transaction per
schema load in `TrinoSchemaCacheValue` and reused it (master
TrinoConnectorExternalTable.java:147-149). For stateful Trino connectors (e.g.
hive's TransactionManager holds per-txn metadata until commit/rollback), this
leaks 3+ never-closed transactions per query → unbounded FE memory growth
proportional to query count. Since P4/P5/P6 copy this bridge shape, worth
fixing at the source: reuse one transaction per query (or commit/rollback in a
finally).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
