github-actions[bot] commented on code in PR #66296:
URL: https://github.com/apache/doris/pull/66296#discussion_r3682259049
##########
fe/fe-common/src/main/java/org/apache/doris/common/Config.java:
##########
@@ -3469,7 +3469,10 @@ public static int metaServiceRpcRetryTimes() {
public static int cloud_sync_version_task_threads_num = 4;
@ConfField(mutable = true, description = {"Maximum table or partition
batch size for get version tasks."})
- public static int cloud_get_version_task_batch_size = 2000;
+ public static int cloud_get_version_task_batch_size = 100;
Review Comment:
[P2] Keep the foreground force-sync batch size separate
This setting is also consumed by `PartitionsProcDir.getPartitionVersions()`
when `cloud_force_sync_version` handles `SHOW PARTITIONS`. That path sends
batches serially and keeps the full foreground retry budget, so changing 2,000
to 100 turns a 2,000-partition request from one RPC into twenty. The PR
describes reducing background-sync pressure; please use a daemon-specific batch
limit (or explicitly justify and test this foreground latency/RPC-count
regression).
##########
fe/fe-core/src/main/java/org/apache/doris/cloud/catalog/CloudSyncVersionDaemon.java:
##########
@@ -58,6 +59,13 @@ protected void runAfterCatalogReady() {
if (!Config.cloud_enable_version_syncer) {
return;
}
+ // This daemon has no ConnectContext, so use the global/default TTLs
to decide whether
+ // the shared version caches need proactive refresh. Finite TTLs
refresh lazily on reads,
+ // while Long.MAX_VALUE never expires and requires this daemon to keep
the cache current.
+ if
(VariableMgr.getDefaultSessionVariable().cloudPartitionVersionCacheTtlMs !=
Long.MAX_VALUE
Review Comment:
[P1] Keep repair active for live infinite-TTL sessions
This guard only describes future/default sessions, while the shared caches
are consumed according to each live session's TTL.
`VariableMgr.newSessionVariable()` clones the defaults once, and `SET GLOBAL`
updates the default plus the setter's session, so another live session can
retain (or later set) both TTLs to `Long.MAX_VALUE`. On a follower, if that
session has cached v1, both defaults become finite, and the async v2 push fails
(the push path only logs the failure), this return also removes the repair
daemon; `CloudPartition.getSnapshotVisibleVersion()` then keeps accepting v1
for that session forever. Please do not stop proactive repair solely from the
defaults while infinite per-session TTLs remain possible, or make the cache
policy genuinely global and transition every live consumer.
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/OlapTable.java:
##########
@@ -3681,7 +3685,7 @@ public static List<Long>
getVisibleVersionFromMeta(List<Long> dbIds, List<Long>
.build();
try {
- Cloud.GetVersionResponse resp =
VersionHelper.getVersionFromMeta(request);
+ Cloud.GetVersionResponse resp =
VersionHelper.getVersionFromMeta(request, maxAttempts);
Review Comment:
[P1] Update the existing OlapTable static mock
`testGetVisibleVersionInBatchCached` uses `mockStatic(VersionHelper.class,
CALLS_REAL_METHODS)` and only stubs the one-argument
`getVersionFromMeta(request)`. This delegation now reaches the unstubbed
two-argument call here, so Mockito invokes the real Meta Service path and the
fixture `callCount` never advances (the test can fail or spend the real retry
budget). `CloudPartitionTest` was updated for the same overload change; please
also stub this overload in `OlapTableTest` and run that test.
--
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]
