Copilot commented on code in PR #12547:
URL: https://github.com/apache/gravitino/pull/12547#discussion_r3827749553
##########
trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/system/GravitinoSystemConnector.java:
##########
@@ -183,14 +192,30 @@ public SchemaTableName getTableName() {
return tableName;
}
+ // The system table data lives on the coordinator only: the catalog load
loop runs there, and
+ // the registration state it records is never replicated to workers.
Splits are built in
+ // SplitManager.getSplits(), which also runs on the coordinator, so this
is always set by the
+ // time it is read. It stays null on worker JVMs, where the behaviour is
unchanged.
+ private static volatile HostAddress coordinatorAddress;
+
+ /**
+ * Sets the coordinator address that system table splits are pinned to.
+ *
+ * @param address the host and port of the Trino coordinator
+ */
+ public static void setCoordinatorAddress(HostAddress address) {
+ coordinatorAddress = address;
+ }
+
@Override
public boolean isRemotelyAccessible() {
- return true;
+ return coordinatorAddress == null;
}
Review Comment:
`Split.isRemotelyAccessible()` / `getAddresses()` rely on the static
`coordinatorAddress`, but `Split` instances are serialized/deserialized across
nodes via JSON (`@JsonCreator` only includes `tableName`). On worker JVMs the
static field remains `null`, so splits become remotely accessible and lose the
coordinator pinning, which can schedule system table reads on workers where
`GravitinoSystemTableFactory.SYSTEM_TABLES` is never registered (and queries
will fail with "table does not exist").
To make split pinning reliable, the coordinator address needs to be part of
the split payload (an instance field serialized with the split), or otherwise
deterministically derivable on every node.
--
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]
