Copilot commented on code in PR #66219:
URL: https://github.com/apache/doris/pull/66219#discussion_r3671833275
##########
fe/fe-core/src/test/java/org/apache/doris/cloud/rpc/MetaServiceProxyTest.java:
##########
@@ -315,11 +315,16 @@ public void testGetInstanceRateLimitedBeforeRpc() throws
RpcException {
@Test
public void testGetVisibleVersionAsyncRateLimitedBeforeRpc() throws
RpcException {
- enableRateLimit(1, "", 1, 0);
+ // Consume the whole quota in one weighted request and use a long
refresh
+ // period so scheduler stalls cannot replenish permits before the
assertion.
+ int burstSeconds = 60;
+ enableRateLimit(1, "", burstSeconds, 0);
Review Comment:
`burstSeconds` is described as a “long refresh period”, but the name
suggests “burst capacity” rather than a refresh/replenish window. Rename it to
something like `refreshSeconds`/`replenishPeriodSeconds` to match the intent
and reduce confusion about what the third `enableRateLimit(...)` argument
represents.
##########
fe/fe-core/src/test/java/org/apache/doris/cloud/rpc/MetaServiceProxyTest.java:
##########
@@ -315,11 +315,16 @@ public void testGetInstanceRateLimitedBeforeRpc() throws
RpcException {
@Test
public void testGetVisibleVersionAsyncRateLimitedBeforeRpc() throws
RpcException {
- enableRateLimit(1, "", 1, 0);
+ // Consume the whole quota in one weighted request and use a long
refresh
+ // period so scheduler stalls cannot replenish permits before the
assertion.
+ int burstSeconds = 60;
+ enableRateLimit(1, "", burstSeconds, 0);
MetaServiceProxy proxy = new MetaServiceProxy();
MetaServiceClient client = mockNormalClient();
putClient(proxy, client);
- consumeRateLimitPermits(proxy, "getPartitionVersion");
+ SettableFuture<Cloud.GetVersionResponse> future =
SettableFuture.create();
+
Mockito.when(client.getVisibleVersionAsync(Mockito.any())).thenReturn(future);
+
proxy.getVisibleVersionAsync(buildBatchPartitionVersionRequest(CPU_CORES *
burstSeconds));
Review Comment:
The test’s quota exhaustion depends on `CPU_CORES * burstSeconds`, which can
make the test behavior machine-dependent (different core counts can change the
request “weight” and whether it fully exhausts the limiter). Prefer deriving
the required weight from the limiter configuration (or using a fixed,
deterministic permit/weight value) so the test is stable across environments.
Also, since the stubbed `SettableFuture` is never completed/cancelled, consider
completing it immediately (or returning an already-completed future) to keep
the test hermetic and avoid lingering async work in the proxy implementation.
--
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]
