pnowojski commented on a change in pull request #6809: [FLINK-10491][network] 
Pass BufferPoolOwner in the constructor of LocalBufferPool
URL: https://github.com/apache/flink/pull/6809#discussion_r223658727
 
 

 ##########
 File path: 
flink-runtime/src/main/java/org/apache/flink/runtime/io/network/buffer/LocalBufferPool.java
 ##########
 @@ -86,7 +84,7 @@
 
        private boolean isDestroyed;
 
-       private BufferPoolOwner owner;
+       private final BufferPoolOwner owner;
 
 Review comment:
   please change it to `private final Optional<BufferPoolOwner> owner`. This 
value is logically optional and should be marked as such (we can not use 
`@Nullable` because we are not enforcing nullable checks during 
compilation/checkstyle). 
   
   Ditto for the constructor and method parameters that are providing this 
value. Either overload them by providing both:
   ```
   BufferPool createBufferPool(int numRequiredBuffers, int maxUsedBuffers, 
BufferPoolOwner owner);
   
   BufferPool createBufferPool(int numRequiredBuffers, int maxUsedBuffers);
   ```
   or if you are lazy provide one method:
   ```
   BufferPool createBufferPool(int numRequiredBuffers, int maxUsedBuffers, 
Optional<BufferPoolOwner> owner)
   ```
   but the first option is more clean.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[email protected]


With regards,
Apache Git Services

Reply via email to