|
20 | 20 | import java.lang.reflect.Constructor; |
21 | 21 | import java.util.Collection; |
22 | 22 | import java.util.Collections; |
23 | | -import java.util.HashSet; |
24 | 23 | import java.util.List; |
25 | 24 | import java.util.Objects; |
26 | 25 | import java.util.Optional; |
@@ -105,24 +104,9 @@ boolean shouldSkipSloppyServerEvaluation() { |
105 | 104 | return isConditionalBalancingEnabled(); |
106 | 105 | } |
107 | 106 |
|
108 | | - Set<Integer> getServersWithTablesToIsolate() { |
109 | | - Optional<MetaTableIsolationConditional> metaTableIsolationConditional = |
110 | | - conditionals.stream().filter(MetaTableIsolationConditional.class::isInstance) |
111 | | - .map(MetaTableIsolationConditional.class::cast).findAny(); |
112 | | - Set<Integer> serversWithTablesToIsolate = new HashSet<>(); |
113 | | - if (metaTableIsolationConditional.isPresent()) { |
114 | | - serversWithTablesToIsolate |
115 | | - .addAll(metaTableIsolationConditional.get().getServersHostingMeta()); |
116 | | - } |
117 | | - |
118 | | - Optional<SystemTableIsolationConditional> systemTableIsolationConditional = |
119 | | - conditionals.stream().filter(SystemTableIsolationConditional.class::isInstance) |
120 | | - .map(SystemTableIsolationConditional.class::cast).findAny(); |
121 | | - if (systemTableIsolationConditional.isPresent()) { |
122 | | - serversWithTablesToIsolate |
123 | | - .addAll(systemTableIsolationConditional.get().getServersHostingSystemTables()); |
124 | | - } |
125 | | - return serversWithTablesToIsolate; |
| 107 | + void clearConditionalWeightCaches() { |
| 108 | + conditionals.stream().map(RegionPlanConditional::getCandidateGenerator) |
| 109 | + .flatMap(Optional::stream).forEach(RegionPlanConditionalCandidateGenerator::clearWeightCache); |
126 | 110 | } |
127 | 111 |
|
128 | 112 | void loadConf(Configuration conf) { |
@@ -165,65 +149,49 @@ void loadClusterState(BalancerClusterState cluster) { |
165 | 149 | } |
166 | 150 |
|
167 | 151 | /** |
168 | | - * Check if the proposed action violates conditionals. Must be called before applying the action. |
| 152 | + * Indicates whether the action is good for our conditional compliance. |
169 | 153 | * @param cluster The cluster state |
170 | 154 | * @param action The proposed action |
171 | | - * @return -1 if the action is an improvement, 0 if it's neutral, and >=1 if it is in violation |
| 155 | + * @return -1 if conditionals improve, 0 if neutral, 1 if conditionals degrade |
172 | 156 | */ |
173 | | - int isViolating(BalancerClusterState cluster, BalanceAction action) { |
174 | | - if (conditionals.isEmpty()) { |
| 157 | + int getViolationCountChange(BalancerClusterState cluster, BalanceAction action) { |
| 158 | + boolean isViolatingPre = isViolating(cluster, action.undoAction()); |
| 159 | + boolean isViolatingPost = isViolating(cluster, action); |
| 160 | + if (isViolatingPre && isViolatingPost) { |
175 | 161 | return 0; |
| 162 | + } else if (!isViolatingPre && isViolatingPost) { |
| 163 | + return 1; |
| 164 | + } else { |
| 165 | + return -1; |
176 | 166 | } |
| 167 | + } |
177 | 168 |
|
178 | | - // loadClusterState(cluster); todo rmattingly don't think this is necessary |
179 | | - List<RegionPlan> regionPlans = doActionAndRefreshConditionals(cluster, action); |
180 | | - |
181 | | - // Now we're in the proposed finished state |
182 | | - // We can get the original violation count by running inverse plans |
183 | | - List<RegionPlan> inversePlans = getInversePlans(regionPlans); |
184 | | - int originalViolationCount = 0; |
185 | | - for (RegionPlan inversePlan : inversePlans) { |
186 | | - originalViolationCount += getConditionalViolationCount(conditionals, inversePlan); |
| 169 | + /** |
| 170 | + * Check if the proposed action violates conditionals |
| 171 | + * @param cluster The cluster state |
| 172 | + * @param action The proposed action |
| 173 | + */ |
| 174 | + boolean isViolating(BalancerClusterState cluster, BalanceAction action) { |
| 175 | + conditionals.forEach(conditional -> conditional.refreshClusterState(cluster)); |
| 176 | + if (conditionals.isEmpty()) { |
| 177 | + return false; |
187 | 178 | } |
188 | | - |
189 | | - // Now go back to the original state, and measure |
190 | | - // the proposed violation count |
191 | | - doActionAndRefreshConditionals(cluster, action.undoAction()); |
192 | | - int proposedViolationCount = 0; |
| 179 | + List<RegionPlan> regionPlans = cluster.convertActionToPlans(action); |
193 | 180 | for (RegionPlan regionPlan : regionPlans) { |
194 | | - proposedViolationCount += getConditionalViolationCount(conditionals, regionPlan); |
195 | | - } |
196 | | - |
197 | | - if (proposedViolationCount - originalViolationCount < 0 && proposedViolationCount == 0) { |
198 | | - // Only take a random improvement if it eliminates violations, or exists in a neutral state |
199 | | - // Otherwise we are probably just fighting our conditional generators |
200 | | - return -1; |
201 | | - } else { |
202 | | - return proposedViolationCount; |
| 181 | + if (isViolating(regionPlan)) { |
| 182 | + return true; |
| 183 | + } |
203 | 184 | } |
| 185 | + return false; |
204 | 186 | } |
205 | 187 |
|
206 | | - private List<RegionPlan> doActionAndRefreshConditionals(BalancerClusterState cluster, |
207 | | - BalanceAction action) { |
208 | | - List<RegionPlan> regionPlans = cluster.doAction(action); |
209 | | - // loadClusterState(cluster); todo rmattingly don't think this is necessary |
210 | | - return regionPlans; |
211 | | - } |
212 | | - |
213 | | - private static List<RegionPlan> getInversePlans(List<RegionPlan> regionPlans) { |
214 | | - return regionPlans.stream().map(regionPlan -> new RegionPlan(regionPlan.getRegionInfo(), |
215 | | - regionPlan.getDestination(), regionPlan.getSource())).toList(); |
216 | | - } |
217 | | - |
218 | | - private static int getConditionalViolationCount(Set<RegionPlanConditional> conditionals, |
219 | | - RegionPlan regionPlan) { |
220 | | - int regionPlanConditionalViolationCount = 0; |
221 | | - for (RegionPlanConditional regionPlanConditional : conditionals) { |
222 | | - if (regionPlanConditional.isViolating(regionPlan)) { |
223 | | - regionPlanConditionalViolationCount++; |
| 188 | + boolean isViolating(RegionPlan regionPlan) { |
| 189 | + for (RegionPlanConditional conditional : conditionals) { |
| 190 | + if (conditional.isViolating(regionPlan)) { |
| 191 | + return true; |
224 | 192 | } |
225 | 193 | } |
226 | | - return regionPlanConditionalViolationCount; |
| 194 | + return false; |
227 | 195 | } |
228 | 196 |
|
229 | 197 | private RegionPlanConditional createConditional(Class<? extends RegionPlanConditional> clazz, |
|
0 commit comments