flink-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] pnowojski commented on a change in pull request #6809: [FLINK-10491][network] Pass BufferPoolOwner in the constructor of LocalBufferPool
Date Tue, 09 Oct 2018 11:30:10 GMT
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:
users@infra.apache.org


With regards,
Apache Git Services

Mime
View raw message