github-actions[bot] commented on code in PR #65694:
URL: https://github.com/apache/doris/pull/65694#discussion_r3593894417
##########
fe/fe-core/src/main/java/org/apache/doris/cloud/rpc/MetaServiceProxy.java:
##########
@@ -331,40 +380,26 @@ public void onSuccess(Cloud.GetVersionResponse result) {
@Override
public void onFailure(Throwable t) {
- if (MetricRepo.isInit && Config.isCloudMode())
{
-
CloudMetrics.META_SERVICE_RPC_ALL_FAILED.increase(1L);
-
CloudMetrics.META_SERVICE_RPC_FAILED.getOrAdd(methodName).increase(1L);
-
CloudMetrics.META_SERVICE_RPC_LATENCY.getOrAdd(methodName)
- .update(System.currentTimeMillis()
- startTime);
- }
+ recordRpcFailed(methodName, startTime);
if (finalClient != null) {
finalClient.shutdown(true);
Review Comment:
[P1] Keep rate-limit rejection out of the version retry loop
Rethrowing the dedicated exception here is not enough for the production
path: `VersionHelper.getVisibleVersionInternal()` catches every `RpcException`
and returns `null`, so `getVisibleVersion()` can run the default 200-attempt
retry loop. Under sustained saturation with
`meta_service_rpc_rate_limit_wait_timeout_ms=0`, every acquisition is rejected
immediately, yet the logical call still sleeps after each attempt, totaling
about 52 to under 120 seconds while query planners and sync workers generate
200 local attempts/logs each. Please propagate `MetaServiceRateLimitException`
through `VersionHelper` (or give it an explicitly bounded limiter-specific
policy) and add an end-to-end helper test that observes one prompt rejection.
##########
fe/fe-core/src/main/java/org/apache/doris/cloud/rpc/MetaServiceProxy.java:
##########
@@ -331,40 +380,26 @@ public void onSuccess(Cloud.GetVersionResponse result) {
@Override
public void onFailure(Throwable t) {
- if (MetricRepo.isInit && Config.isCloudMode())
{
-
CloudMetrics.META_SERVICE_RPC_ALL_FAILED.increase(1L);
-
CloudMetrics.META_SERVICE_RPC_FAILED.getOrAdd(methodName).increase(1L);
-
CloudMetrics.META_SERVICE_RPC_LATENCY.getOrAdd(methodName)
- .update(System.currentTimeMillis()
- startTime);
- }
+ recordRpcFailed(methodName, startTime);
if (finalClient != null) {
finalClient.shutdown(true);
}
}
},
com.google.common.util.concurrent.MoreExecutors.directExecutor());
}
return future;
+ } catch (MetaServiceRateLimitException e) {
+ recordRpcRateLimited(methodName);
+ throw e;
Review Comment:
[P1] Keep rate-limit rejection out of the version retry loop
Rethrowing the dedicated exception here is not enough for the production
path: `VersionHelper.getVisibleVersionInternal()` catches every `RpcException`
and returns `null`, so `getVisibleVersion()` can run the default 200-attempt
retry loop. Under sustained saturation with
`meta_service_rpc_rate_limit_wait_timeout_ms=0`, every acquisition is rejected
immediately, yet the logical call still sleeps after each attempt, totaling
about 52 to under 120 seconds while query planners and sync workers generate
200 local attempts/logs each. Please propagate `MetaServiceRateLimitException`
through `VersionHelper` (or give it an explicitly bounded limiter-specific
policy) and add an end-to-end helper test that observes one prompt rejection.
--
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]
