flink-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "ASF GitHub Bot (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (FLINK-8178) Introduce not threadsafe write only BufferBuilder
Date Fri, 15 Dec 2017 13:16:00 GMT

    [ 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:

    --- 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

View raw message