github-actions[bot] commented on code in PR #64742:
URL: https://github.com/apache/doris/pull/64742#discussion_r3459404674
##########
fe/fe-core/src/main/java/org/apache/doris/metric/MetricRepo.java:
##########
@@ -438,6 +445,31 @@ public Integer getValue() {
}
};
DORIS_METRIC_REGISTER.addMetrics(connections);
+ GAUGE_ARROW_FLIGHT_CONNECTIONS = new
GaugeMetric<Integer>("arrow_flight_connection_total",
+ MetricUnit.CONNECTIONS, "total arrow flight connections") {
+ @Override
+ public Integer getValue() {
+ return
ExecuteEnv.getInstance().getScheduler().getFlightSqlConnectPoolMgr().getConnectionNum();
+ }
+ };
+ DORIS_METRIC_REGISTER.addMetrics(GAUGE_ARROW_FLIGHT_CONNECTIONS);
+ GAUGE_CONNECTION_MAX = new GaugeMetric<Integer>("connection_max",
+ MetricUnit.CONNECTIONS, "max connections") {
+ @Override
Review Comment:
`connection_total` above is populated from
`ConnectScheduler.getConnectionNum()`, which sums the MySQL and Arrow Flight
pools, but this new max gauge only returns `Config.qe_max_connection`. With the
default config, Arrow Flight can add another 4096 connections, so
`doris_fe_connection_total` can exceed `doris_fe_connection_max` even while
both pools are within their limits. Please either make this max match the total
metric, for example by including `Config.arrow_flight_max_connections`, or
rename/scope it as the MySQL-only limit so the exported gauges are not
internally inconsistent.
##########
fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/Auth.java:
##########
@@ -586,6 +587,8 @@ private void createUserInternal(UserIdentity userIdent,
String roleName, byte[]
}
// other user properties
propertyMgr.addUserResource(userIdent.getQualifiedUser());
Review Comment:
This update should not run for every `Auth` instance. The checkpoint path
constructs a separate checkpoint `Env`, whose constructor creates a separate
`Auth` and calls `initUser()` before the image is loaded; with this new hook
that checkpoint Auth writes the static `USER_GAUGE_CONNECTION_MAX` gauge for
`root`/`admin` using the default value. Since `Auth.readFields()` later loads
the real checkpoint properties without resyncing metrics, the serving FE can
continue exporting the stale/default value. Please guard these metric updates
to the serving env only, or move the per-user refresh into `MetricRepo` so it
always reads from `Env.getServingEnv().getAuth()`.
--
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]
