RocMarshal commented on PR #27291: URL: https://github.com/apache/flink/pull/27291#issuecomment-3610554701
> I think `assignedTasks` instead of `numberOfTasks` would cover better what this field is intended for. > > I do not see why do we need to use `LoadingWeight` for this. It feels forced to me, and I maybe I miss something, but I also feel it complicates this change for nothing. Thank you @ferenc-csaky for the comments. Pls let me have try on clarifying it: Initially, I did not use `LoadingWeight` when extending the `SlotReport`, and only included the `numberOfTasks`. Why I switched to `LoadingWeight`: - It fully satisfies the current functional requirements (which is necessary). - One of the motivations behind introducing `LoadingWeight` is to provide a way to represent and potentially extend more slot-level load information.What I mean is: if we use `LoadingWeight` as the vehicle for reporting `SlotReport` data, then in the future, if we want to expose additional load-related information, we won’t need to change the load-related parameter passing chain anymore. The `ResourceManager` could simply retrieve the desired load metrics directly from `LoadingWeight`. In short, using an interface-driven approach gives us openness for future extensions. That said, I admit that, relative to the current functionality, this coding approach does violate the principle of minimal changes. Both approaches are acceptable to me. If you believe that, for the current change, adhering to the minimal change principle should take priority, I'd be happy to revise the implementation accordingly. -- 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]
