mynameborat commented on a change in pull request #1446: URL: https://github.com/apache/samza/pull/1446#discussion_r546929602
########## File path: samza-core/src/main/java/org/apache/samza/clustermanager/StandbyContainerManager.java ########## @@ -361,8 +407,41 @@ private FailoverMetadata registerActiveContainerFailure(String activeContainerID } /** - * Check if matching this SamzaResourceRequest to the given resource, meets all standby-container container constraints. + * This method checks from the config if standby allocation is fault domain aware or not, and requests resources accordingly. + * + * @param containerAllocator ContainerAllocator object that requests for resources from the resource manager + * @param containerID Samza container ID that will be run when a resource is allocated for this request + * @param preferredHost name of the host that you prefer to run the processor on + */ + void checkFaultDomainAwarenessEnabledAndRequestResource(ContainerAllocator containerAllocator, String containerID, String preferredHost) { Review comment: nit: I'd prefer to rename the method to `requestResource` since the intent of the method that way is clear. i.e. only request resource and potentially return the `SamzaResourceRequest`. What and how it does to request resource is kept within and can be inferred by reading the method implementation. The name seems too long and the fact that this returns void makes it unusable in some places which has the exact boiler plate code. ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org