xintongsong commented on a change in pull request #11916: URL: https://github.com/apache/flink/pull/11916#discussion_r415522836
########## File path: flink-yarn/src/main/java/org/apache/flink/yarn/WorkerSpecContainerResourceAdapter.java ########## @@ -71,52 +72,65 @@ } Optional<Resource> tryComputeContainerResource(final WorkerResourceSpec workerResourceSpec) { - return Optional.ofNullable(workerSpecToContainerResource.computeIfAbsent( + final InternalContainerResource internalContainerResource = workerSpecToContainerResource.computeIfAbsent( Preconditions.checkNotNull(workerResourceSpec), - this::createAndMapContainerResource)); + this::createAndMapContainerResource); + if (internalContainerResource != null) { + return Optional.of(internalContainerResource.toResource()); + } else { + return Optional.empty(); + } } Set<WorkerResourceSpec> getWorkerSpecs(final Resource containerResource, final MatchingStrategy matchingStrategy) { - return getEquivalentContainerResource(containerResource, matchingStrategy).stream() + final InternalContainerResource internalContainerResource = new InternalContainerResource(containerResource); + return getEquivalentInternalContainerResource(internalContainerResource, matchingStrategy).stream() .flatMap(resource -> containerResourceToWorkerSpecs.getOrDefault(resource, Collections.emptySet()).stream()) .collect(Collectors.toSet()); } Set<Resource> getEquivalentContainerResource(final Resource containerResource, final MatchingStrategy matchingStrategy) { + final InternalContainerResource internalContainerResource = new InternalContainerResource(containerResource); + return getEquivalentInternalContainerResource(internalContainerResource, matchingStrategy).stream() + .map(InternalContainerResource::toResource) + .collect(Collectors.toSet()); + } + + private Set<InternalContainerResource> getEquivalentInternalContainerResource(final InternalContainerResource internalContainerResource, final MatchingStrategy matchingStrategy) { Review comment: I see your point. But I think the benefit for the current approach is that, we make it explicit that for all methods visible from outside the first thing we do is to convert `Resource` into `InternalContainerResource`, and the last thing we do before returning is to covert it back. ---------------------------------------------------------------- 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