github-code-scanning[bot] commented on code in PR #13197: URL: https://github.com/apache/druid/pull/13197#discussion_r1233538197
########## server/src/main/java/org/apache/druid/server/coordinator/loading/SegmentReplicationStatus.java: ########## @@ -0,0 +1,84 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.druid.server.coordinator.loading; + +import com.google.common.collect.ImmutableMap; +import it.unimi.dsi.fastutil.objects.Object2LongMap; +import it.unimi.dsi.fastutil.objects.Object2LongOpenHashMap; +import org.apache.druid.timeline.DataSegment; +import org.apache.druid.timeline.SegmentId; + +import java.util.HashMap; +import java.util.Map; + +/** + * An immutable object that contains information about the under-replicated + * or unavailable status of all used segments. This state is recomputed by + * the {@link StrategicSegmentAssigner} in every run. + */ +public class SegmentReplicationStatus +{ + private final Map<SegmentId, SegmentReplicaCount> totalReplicaCounts; + private final Map<SegmentId, Map<String, SegmentReplicaCount>> replicaCountsInTier; + + public SegmentReplicationStatus(Map<SegmentId, Map<String, SegmentReplicaCount>> replicaCountsInTier) + { + this.replicaCountsInTier = ImmutableMap.copyOf(replicaCountsInTier); + + final Map<SegmentId, SegmentReplicaCount> totalReplicaCounts = new HashMap<>(); Review Comment: ## Possible confusion of local and field Potentially confusing name: [SegmentReplicationStatus](1) also refers to field [totalReplicaCounts](2) (as this.totalReplicaCounts). [Show more details](https://github.com/apache/druid/security/code-scanning/5119) ########## server/src/main/java/org/apache/druid/server/coordinator/loading/SegmentLoadingConfig.java: ########## @@ -0,0 +1,153 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.druid.server.coordinator.loading; + +import org.apache.druid.java.util.common.logger.Logger; +import org.apache.druid.server.coordinator.CoordinatorDynamicConfig; + +/** + * Contains recomputed configs from {@link CoordinatorDynamicConfig} based on + * whether {@link CoordinatorDynamicConfig#isSmartSegmentLoading} is enabled or not. + */ +public class SegmentLoadingConfig +{ + private static final Logger log = new Logger(SegmentLoadingConfig.class); + + private final int maxSegmentsInLoadQueue; + private final int replicationThrottleLimit; + private final int maxReplicaAssignmentsInRun; + private final int maxLifetimeInLoadQueue; + + private final int maxSegmentsToMove; + private final int percentDecommSegmentsToMove; + + private final boolean useRoundRobinSegmentAssignment; + private final boolean emitBalancingStats; + + /** + * Creates a new SegmentLoadingConfig with recomputed coordinator config values from + * based on whether {@link CoordinatorDynamicConfig#isSmartSegmentLoading()} + * is enabled or not. + */ + public static SegmentLoadingConfig create(CoordinatorDynamicConfig dynamicConfig, int numUsedSegments) + { + if (dynamicConfig.isSmartSegmentLoading()) { + // Compute recommended values + // Impose a lower bound on both replicationThrottleLimit and maxSegmentsToMove + final int throttlePercentage = 2; + final int replicationThrottleLimit = Math.max(100, numUsedSegments * throttlePercentage / 100); + + // Impose an upper bound on maxSegmentsToMove to ensure that coordinator + // run times are bounded. This limit can be relaxed as performance of + // the CostBalancerStrategy.computeCost() is improved. + final int maxSegmentsToMove = Math.min(1000, replicationThrottleLimit); + + log.info( + "Smart segment loading is enabled. Recomputed replicationThrottleLimit" + + " [%,d] (%d%% of used segments [%,d]) and maxSegmentsToMove [%d].", + replicationThrottleLimit, throttlePercentage, numUsedSegments, maxSegmentsToMove + ); + + return new SegmentLoadingConfig( + 0, + replicationThrottleLimit, + Integer.MAX_VALUE, + dynamicConfig.getReplicantLifetime(), + maxSegmentsToMove, + 100, + true, + false + ); + } else { + // Use the configured values + return new SegmentLoadingConfig( + dynamicConfig.getMaxSegmentsInNodeLoadingQueue(), + dynamicConfig.getReplicationThrottleLimit(), + dynamicConfig.getMaxNonPrimaryReplicantsToLoad(), Review Comment: ## Deprecated method or constructor invocation Invoking [CoordinatorDynamicConfig.getMaxNonPrimaryReplicantsToLoad](1) should be avoided because it has been deprecated. [Show more details](https://github.com/apache/druid/security/code-scanning/5120) ########## server/src/main/java/org/apache/druid/server/coordinator/loading/SegmentLoadingConfig.java: ########## @@ -0,0 +1,153 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.druid.server.coordinator.loading; + +import org.apache.druid.java.util.common.logger.Logger; +import org.apache.druid.server.coordinator.CoordinatorDynamicConfig; + +/** + * Contains recomputed configs from {@link CoordinatorDynamicConfig} based on + * whether {@link CoordinatorDynamicConfig#isSmartSegmentLoading} is enabled or not. + */ +public class SegmentLoadingConfig +{ + private static final Logger log = new Logger(SegmentLoadingConfig.class); + + private final int maxSegmentsInLoadQueue; + private final int replicationThrottleLimit; + private final int maxReplicaAssignmentsInRun; + private final int maxLifetimeInLoadQueue; + + private final int maxSegmentsToMove; + private final int percentDecommSegmentsToMove; + + private final boolean useRoundRobinSegmentAssignment; + private final boolean emitBalancingStats; + + /** + * Creates a new SegmentLoadingConfig with recomputed coordinator config values from + * based on whether {@link CoordinatorDynamicConfig#isSmartSegmentLoading()} + * is enabled or not. + */ + public static SegmentLoadingConfig create(CoordinatorDynamicConfig dynamicConfig, int numUsedSegments) + { + if (dynamicConfig.isSmartSegmentLoading()) { + // Compute recommended values + // Impose a lower bound on both replicationThrottleLimit and maxSegmentsToMove + final int throttlePercentage = 2; + final int replicationThrottleLimit = Math.max(100, numUsedSegments * throttlePercentage / 100); + + // Impose an upper bound on maxSegmentsToMove to ensure that coordinator + // run times are bounded. This limit can be relaxed as performance of + // the CostBalancerStrategy.computeCost() is improved. + final int maxSegmentsToMove = Math.min(1000, replicationThrottleLimit); + + log.info( + "Smart segment loading is enabled. Recomputed replicationThrottleLimit" + + " [%,d] (%d%% of used segments [%,d]) and maxSegmentsToMove [%d].", + replicationThrottleLimit, throttlePercentage, numUsedSegments, maxSegmentsToMove + ); + + return new SegmentLoadingConfig( + 0, + replicationThrottleLimit, + Integer.MAX_VALUE, + dynamicConfig.getReplicantLifetime(), + maxSegmentsToMove, + 100, + true, + false + ); + } else { + // Use the configured values + return new SegmentLoadingConfig( + dynamicConfig.getMaxSegmentsInNodeLoadingQueue(), + dynamicConfig.getReplicationThrottleLimit(), + dynamicConfig.getMaxNonPrimaryReplicantsToLoad(), + dynamicConfig.getReplicantLifetime(), + dynamicConfig.getMaxSegmentsToMove(), + dynamicConfig.getDecommissioningMaxPercentOfMaxSegmentsToMove(), Review Comment: ## Deprecated method or constructor invocation Invoking [CoordinatorDynamicConfig.getDecommissioningMaxPercentOfMaxSegmentsToMove](1) should be avoided because it has been deprecated. [Show more details](https://github.com/apache/druid/security/code-scanning/5121) -- 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]
