deardeng commented on code in PR #67636:
URL: https://github.com/apache/doris/pull/67636#discussion_r4061612339
##########
fe/fe-core/src/main/java/org/apache/doris/cloud/catalog/CloudTabletRebalancer.java:
##########
@@ -1029,24 +1035,62 @@ boolean staleRouteSweepNeeded(Set<Long> currentBes) {
// baseline. Advancing only after a sweep would leave the baseline at
the pre-addition set, and
// dropping that same backend later would compare equal to it and go
unnoticed.
lastSweptBackends = currentBes;
- if (pendingSweepRounds > 0) {
+ // Outside the configured window, leave pendingSweepRounds untouched
rather than draining it: a
+ // backend that goes away outside the window must still get its two
rounds once the window
+ // opens, not lose them to rounds that never actually swept.
+ if (pendingSweepRounds > 0 && isStaleRouteCleanTimeAllowed()) {
pendingSweepRounds--;
Review Comment:
ignore
##########
fe/fe-core/src/main/java/org/apache/doris/cloud/catalog/CloudTabletRebalancer.java:
##########
@@ -1029,24 +1035,62 @@ boolean staleRouteSweepNeeded(Set<Long> currentBes) {
// baseline. Advancing only after a sweep would leave the baseline at
the pre-addition set, and
// dropping that same backend later would compare equal to it and go
unnoticed.
lastSweptBackends = currentBes;
- if (pendingSweepRounds > 0) {
+ // Outside the configured window, leave pendingSweepRounds untouched
rather than draining it: a
+ // backend that goes away outside the window must still get its two
rounds once the window
+ // opens, not lose them to rounds that never actually swept.
+ if (pendingSweepRounds > 0 && isStaleRouteCleanTimeAllowed()) {
pendingSweepRounds--;
return true;
}
return false;
}
+ /**
+ * Whether the configured cleanup window
(cloud_tablet_rebalancer_stale_route_clean_start_time to
+ * ..._end_time) contains the current time. Equal start/end -- including
the "00:00"/"00:00" default --
+ * means unrestricted, matching the pre-existing behavior of sweeping
whenever staleRouteSweepNeeded()
+ * says a sweep is due. An unparseable configuration also falls back to
unrestricted rather than
+ * silently disabling cleanup.
+ */
+ @VisibleForTesting
+ boolean isStaleRouteCleanTimeAllowed() {
+ LocalTime start =
parseCleanTime(Config.cloud_tablet_rebalancer_stale_route_clean_start_time);
+ LocalTime end =
parseCleanTime(Config.cloud_tablet_rebalancer_stale_route_clean_end_time);
+ if (start == null || end == null || start.equals(end)) {
+ return true;
+ }
+ return isWithinWindow(LocalTime.now(TimeUtils.getDorisZoneId()),
start, end);
+ }
+
+ @VisibleForTesting
+ static boolean isWithinWindow(LocalTime now, LocalTime start, LocalTime
end) {
+ if (start.isBefore(end)) {
+ return !now.isBefore(start) && !now.isAfter(end);
+ }
+ // across midnight, e.g. 23:00 - 06:00
+ return !now.isBefore(start) || !now.isAfter(end);
+ }
+
+ private static LocalTime parseCleanTime(String time) {
+ try {
+ return LocalTime.parse(time, STALE_ROUTE_CLEAN_TIME_FORMAT);
+ } catch (DateTimeParseException e) {
+ LOG.warn("invalid cloud_tablet_rebalancer_stale_route_clean time:
{}", time);
+ return null;
+ }
+ }
+
private boolean completeRouteInfo() {
List<UpdateCloudReplicaInfo> updateReplicaInfos = new
ArrayList<UpdateCloudReplicaInfo>();
long[] assignedErrNum = {0L};
- long[] staleRouteNum = {0L};
boolean sweepStaleRoutes = staleRouteSweepNeeded(allBes);
- // loopCloudReplica() has the compute group loop innermost, so it
hands us every replica once per
- // live compute group, while removeInvalidRoutes() scans the whole
route map and does not care
- // which group we are on. Pin the sweep to one arbitrary group id so a
sweeping round still makes a
- // single pass per replica. If clusterToBes is empty the callback
never runs at all, so the serving
- // catalog keeps the entries until it reloads the image -- there is
nothing to route in that state.
- String sweepTicket = sweepStaleRoutes ?
clusterToBes.keySet().stream().findFirst().orElse(null) : null;
+ // Cleanup is independent of compute groups and includes shadow
indices of active tables.
+ // Take the catalog and backend set from the same Env so the sweep can
never mix the two.
+ Env currentEnv = Env.getCurrentEnv();
+ long staleRouteNum = sweepStaleRoutes
+ ? ((CloudInternalCatalog) currentEnv.getInternalCatalog())
+
.removeInvalidCloudReplicaRoutes(currentEnv.getClusterInfo())
Review Comment:
ignore
--
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]
