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


Reply via email to