reswqa commented on code in PR #22176:
URL: https://github.com/apache/flink/pull/22176#discussion_r1136525101
##########
flink-runtime/src/test/java/org/apache/flink/runtime/resourcemanager/slotmanager/DefaultResourceAllocationStrategyTest.java:
##########
@@ -39,7 +40,15 @@ class DefaultResourceAllocationStrategyTest {
private static final int NUM_OF_SLOTS = 5;
private static final DefaultResourceAllocationStrategy STRATEGY =
Review Comment:
```suggestion
private static final DefaultResourceAllocationStrategy
ANY_MATCHING_STRATEGY =
```
##########
flink-runtime/src/main/java/org/apache/flink/runtime/resourcemanager/slotmanager/SlotMatchingStrategy.java:
##########
@@ -41,4 +42,18 @@ <T extends TaskManagerSlotInformation> Optional<T>
findMatchingSlot(
ResourceProfile requestedProfile,
Collection<T> freeSlots,
Function<InstanceID, Integer> numberRegisteredSlotsLookup);
+
+ /**
+ * Finds a matching resource instance for the request {@link
ResourceProfile} given the
+ * collection of available instances.
+ *
+ * @param resourceMatchingPredicate Predicate of whether instance resource
is matching
+ * @param availableInstances collection of available instances
+ * @param numberUtilizationLookup lookup for the weight of instance, lower
is better
+ * @return Returns a matching instance or {@link Optional#empty()} if
there is none
+ */
+ Optional<InstanceID> findMatchingResource(
+ Predicate<InstanceID> resourceMatchingPredicate,
+ Collection<InstanceID> availableInstances,
+ Function<InstanceID, Double> numberUtilizationLookup);
Review Comment:
Why `numberUtilizationLookup` is required for the interface. IIUC, only
`LeastUtilizationSlotMatchingStrategy` need this lookup function. It's strange
to put it in the interface protocol.
##########
flink-runtime/src/test/java/org/apache/flink/runtime/resourcemanager/slotmanager/AnyMatchingSlotMatchingStrategyTest.java:
##########
@@ -88,6 +92,33 @@ public void
findMatchingSlot_withUnfulfillableRequest_returnsEmptyResult() {
assertFalse(optionalMatchingSlot.isPresent());
}
+ @Test
+ public void findMatchingSlotWithFulfillableRequestReturnsFulfillingSlot() {
+ final Optional<InstanceID> optionalInstanceID =
+ AnyMatchingSlotMatchingStrategy.INSTANCE.findMatchingResource(
+ instanceID -> true,
+ availableInstances,
+ instanceID -> {
+ if (instanceID == instanceId) {
+ return 100.0;
+ } else {
+ return 1.0;
+ }
+ });
+
+ assertTrue(optionalInstanceID.isPresent());
Review Comment:
Please migrate this test class to Junit5&AssertJ first.
--
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]
