[ 
https://issues.apache.org/jira/browse/FLINK-8178?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16292513#comment-16292513
 ] 

ASF GitHub Bot commented on FLINK-8178:
---------------------------------------

Github user pnowojski commented on a diff in the pull request:

    https://github.com/apache/flink/pull/5105#discussion_r156928873
  
    --- Diff: 
flink-runtime/src/test/java/org/apache/flink/runtime/io/network/buffer/LocalBufferPoolDestroyTest.java
 ---
    @@ -104,11 +104,10 @@ public void testDestroyWhileBlockingRequest() throws 
Exception {
         * @return Flag indicating whether the Thread is in a blocking buffer
         * request or not
         */
    -   private boolean isInBlockingBufferRequest(StackTraceElement[] 
stackTrace) {
    +   public static boolean isInBlockingBufferRequest(StackTraceElement[] 
stackTrace) {
                if (stackTrace.length >= 3) {
                        return stackTrace[0].getMethodName().equals("wait") &&
    -                                   
stackTrace[1].getMethodName().equals("requestBuffer") &&
    -                                   
stackTrace[2].getMethodName().equals("requestBufferBlocking");
    +                                   
stackTrace[1].getClassName().equals(LocalBufferPool.class.getName());
    --- End diff --
    
    I do not like this test in a first place, but I couldn't come up with a 
better solution. This change make it at least less implementation dependent. 
The same logic as in my "crusade" against mockito. If you put such specific 
condition as it was before (blocking on `requestBuffer`) in one more test, you 
have to manually fix it in one more place during refactoring/adding features 
etc, which drastically increases the cost of maintaining the project and just 
doesn't scale up with the project size :( 
    
    On the other hand you can argue that if after a refactor, we add more 
waiting conditions (if `LocalBufferPool` can block in two different places 
depending on some internal condition), broader check like this is might also be 
the better choice/condition.


> Introduce not threadsafe write only BufferBuilder
> -------------------------------------------------
>
>                 Key: FLINK-8178
>                 URL: https://issues.apache.org/jira/browse/FLINK-8178
>             Project: Flink
>          Issue Type: Improvement
>          Components: Network
>            Reporter: Piotr Nowojski
>            Assignee: Piotr Nowojski
>             Fix For: 1.5.0
>
>
> While Buffer class is used in multithreaded context it requires 
> synchronisation. Now it is miss-leading/unclear and suggesting that 
> RecordSerializer should take into account synchronisation of the Buffer 
> that's holding. With NotThreadSafe BufferBuilder there would be clear 
> separation between single-threaded writing/creating a BufferBuilder and 
> multithreaded Buffer handling/retaining/recycling.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

Reply via email to