You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by StephanEwen <gi...@git.apache.org> on 2014/11/10 17:22:25 UTC

[GitHub] incubator-flink pull request: [FLINK-1323] implementation that use...

GitHub user StephanEwen opened a pull request:

    https://github.com/apache/incubator-flink/pull/193

    [FLINK-1323]  implementation that uses callbacks on completed write requests. Refactor I/O Manager Readers and Writers to interfaces, ...

      - add
     - This change also allows for a very simple way of plugging in a synchronous version of the I/O manager.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/StephanEwen/incubator-flink ioMan

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/incubator-flink/pull/193.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #193
    
----
commit 51801de74ed2d024fd5643b7e79c5e0de4b5d513
Author: Stephan Ewen <se...@apache.org>
Date:   2014-11-07T17:45:19Z

    [FLINK-1323] Refactor I/O Manager Readers and Writers to interfaces, add implementation that uses callbacks on completed write requests.
    
     - This change also allows for a very simple way of plugging in a synchronous version of the I/O manager.

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-flink pull request: [FLINK-1323] Add an I/O writer imple...

Posted by uce <gi...@git.apache.org>.
Github user uce commented on a diff in the pull request:

    https://github.com/apache/incubator-flink/pull/193#discussion_r20142356
  
    --- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/io/disk/iomanager/BlockChannelWriter.java ---
    @@ -16,101 +16,40 @@
      * limitations under the License.
      */
     
    -
     package org.apache.flink.runtime.io.disk.iomanager;
     
    -
     import java.io.IOException;
     import java.util.concurrent.LinkedBlockingQueue;
    -import java.util.concurrent.TimeUnit;
     
     import org.apache.flink.core.memory.MemorySegment;
     
    -
     /**
      * A writer that writes data in blocks to a file channel. The writer receives the data blocks in the form of 
      * {@link org.apache.flink.core.memory.MemorySegment}, which it writes entirely to the channel,
    - * regardless of how space in the segment is used. The writing happens in an asynchronous fashion. That is, a write
    - * request is not processed by the thread that issues it, but by an asynchronous writer thread. Once the request
    - * is done, the asynchronous writer adds the MemorySegment to a <i>return queue</i> where it can be popped by the
    - * worker thread, to be reused. The return queue is in this case a
    - * {@link java.util.concurrent.LinkedBlockingQueue}, such that the working thread blocks until the request has been served,
    - * if the request is still pending when the it requires the segment back. 
    - * <p>
    - * Typical write behind is realized, by having a small set of segments in the return queue at all times. When a
    - * memory segment must be written, the request is issued to the writer and a new segment is immediately popped from
    - * the return queue. Once too many requests have been issued and the I/O thread cannot keep up, the working thread
    - * naturally blocks until another segment is available again.
    + * regardless of how space in the segment is used. The writing may be realized synchronously, or asynchronously,
    + * depending on the implementation. To support 
    --- End diff --
    
    trivial: comment ends apruptly


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-flink pull request: [FLINK-1323] Add an I/O writer imple...

Posted by uce <gi...@git.apache.org>.
Github user uce commented on the pull request:

    https://github.com/apache/incubator-flink/pull/193#issuecomment-62529953
  
    Very nice refactoring. Looks good to me.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-flink pull request: [FLINK-1323] Add an I/O writer imple...

Posted by uce <gi...@git.apache.org>.
Github user uce commented on a diff in the pull request:

    https://github.com/apache/incubator-flink/pull/193#discussion_r20142322
  
    --- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/io/disk/iomanager/AbstractFileIOChannel.java ---
    @@ -0,0 +1,107 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.flink.runtime.io.disk.iomanager;
    +
    +import java.io.File;
    +import java.io.IOException;
    +import java.io.RandomAccessFile;
    +import java.nio.channels.FileChannel;
    +
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +import com.google.common.base.Preconditions;
    +
    +public abstract class AbstractFileIOChannel implements FileIOChannel {
    +
    +	/** Logger object for channel and its subclasses */
    +	protected static final Logger LOG = LoggerFactory.getLogger(FileIOChannel.class);
    +	
    +	/** The ID of the underlying channel. */
    +	protected final FileIOChannel.ID id;
    +	
    +	/** A file channel for NIO access to the file. */
    +	protected final FileChannel fileChannel;
    +	
    +	
    +	/**
    +	 * Creates a new channel to the path indicated by the given ID. The channel hands IO requests to
    +	 * the given request queue to be processed.
    +	 * 
    +	 * @param channelID The id describing the path of the file that the channel accessed.
    +	 * @param writeEnabled Flag describing whether the channel should be opened in read/write mode, rather
    +	 *                     than in read-only mode.
    +	 * @throws IOException Thrown, if the channel could no be opened.
    +	 */
    +	protected AbstractFileIOChannel(FileIOChannel.ID channelID, boolean writeEnabled) throws IOException {
    +		Preconditions.checkNotNull(channelID);
    +		this.id = channelID;
    --- End diff --
    
    trivial: we can do `this.id = checkNotNull(channelID);` directly


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-flink pull request: [FLINK-1323] Add an I/O writer imple...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/incubator-flink/pull/193


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-flink pull request: [FLINK-1323] Add an I/O writer imple...

Posted by uce <gi...@git.apache.org>.
Github user uce commented on the pull request:

    https://github.com/apache/incubator-flink/pull/193#issuecomment-62530151
  
    I will merge this.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---