Uploaded image for project: 'Tajo'
  1. Tajo
  2. TAJO-1494

Add SeekableScanner support to DelimitedTextFileScanner

    Details

    • Type: New Feature
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.11.0
    • Component/s: Storage
    • Labels:
      None

      Description

      CSVFile will be deprecated. So DelimitedTextFileScanner should support SeekableScanner interface for BSTIndex

      1. TAJO-1494_2.patch
        29 kB
        Jinho Kim
      2. TAJO-1494_3.patch
        35 kB
        Jinho Kim
      3. TAJO-1494.patch
        27 kB
        Jinho Kim

        Issue Links

          Activity

          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user jinossy opened a pull request:

          https://github.com/apache/tajo/pull/489

          TAJO-1494: Add SeekableScanner support to DelimitedTextFileScanner

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

          $ git pull https://github.com/jinossy/tajo TAJO-1494

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

          https://github.com/apache/tajo/pull/489.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 #489


          commit e2884b226e87b26481577bc33db7de71153b8417
          Author: Jinho Kim <jhkim@apache.org>
          Date: 2015-04-01T04:34:13Z

          TAJO-1494: Add SeekableScanner support to DelimitedTextFileScanner


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user jinossy opened a pull request: https://github.com/apache/tajo/pull/489 TAJO-1494 : Add SeekableScanner support to DelimitedTextFileScanner You can merge this pull request into a Git repository by running: $ git pull https://github.com/jinossy/tajo TAJO-1494 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tajo/pull/489.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 #489 commit e2884b226e87b26481577bc33db7de71153b8417 Author: Jinho Kim <jhkim@apache.org> Date: 2015-04-01T04:34:13Z TAJO-1494 : Add SeekableScanner support to DelimitedTextFileScanner
          Hide
          tajoqa Tajo QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12708604/TAJO-1494.patch
          against master revision release-0.9.0-rc0-227-g487a0e5.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 4 new or modified test files.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 javadoc. The applied patch does not increase the total number of javadoc warnings.

          +1 checkstyle. The patch generated 0 code style errors.

          -1 findbugs. The patch appears to introduce 10 new Findbugs (version 2.0.3) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed unit tests in tajo-core tajo-storage/tajo-storage-common tajo-storage/tajo-storage-hdfs.

          Test results: https://builds.apache.org/job/PreCommit-TAJO-Build/687//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/687//artifact/incubator-tajo/patchprocess/newPatchFindbugsWarningstajo-storage-common.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/687//artifact/incubator-tajo/patchprocess/newPatchFindbugsWarningstajo-storage-hdfs.html
          Console output: https://builds.apache.org/job/PreCommit-TAJO-Build/687//console

          This message is automatically generated.

          Show
          tajoqa Tajo QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12708604/TAJO-1494.patch against master revision release-0.9.0-rc0-227-g487a0e5. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 4 new or modified test files. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 javadoc. The applied patch does not increase the total number of javadoc warnings. +1 checkstyle. The patch generated 0 code style errors. -1 findbugs. The patch appears to introduce 10 new Findbugs (version 2.0.3) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed unit tests in tajo-core tajo-storage/tajo-storage-common tajo-storage/tajo-storage-hdfs. Test results: https://builds.apache.org/job/PreCommit-TAJO-Build/687//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/687//artifact/incubator-tajo/patchprocess/newPatchFindbugsWarningstajo-storage-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/687//artifact/incubator-tajo/patchprocess/newPatchFindbugsWarningstajo-storage-hdfs.html Console output: https://builds.apache.org/job/PreCommit-TAJO-Build/687//console This message is automatically generated.
          Hide
          jhkim Jinho Kim added a comment -

          I've fix findbug warnings

          Show
          jhkim Jinho Kim added a comment - I've fix findbug warnings
          Hide
          tajoqa Tajo QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12708611/TAJO-1494_2.patch
          against master revision release-0.9.0-rc0-227-g487a0e5.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 4 new or modified test files.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 javadoc. The applied patch does not increase the total number of javadoc warnings.

          +1 checkstyle. The patch generated 0 code style errors.

          -1 findbugs. The patch appears to introduce 8 new Findbugs (version 2.0.3) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed unit tests in tajo-core tajo-storage/tajo-storage-common tajo-storage/tajo-storage-hdfs.

          Test results: https://builds.apache.org/job/PreCommit-TAJO-Build/688//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/688//artifact/incubator-tajo/patchprocess/newPatchFindbugsWarningstajo-storage-common.html
          Console output: https://builds.apache.org/job/PreCommit-TAJO-Build/688//console

          This message is automatically generated.

          Show
          tajoqa Tajo QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12708611/TAJO-1494_2.patch against master revision release-0.9.0-rc0-227-g487a0e5. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 4 new or modified test files. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 javadoc. The applied patch does not increase the total number of javadoc warnings. +1 checkstyle. The patch generated 0 code style errors. -1 findbugs. The patch appears to introduce 8 new Findbugs (version 2.0.3) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed unit tests in tajo-core tajo-storage/tajo-storage-common tajo-storage/tajo-storage-hdfs. Test results: https://builds.apache.org/job/PreCommit-TAJO-Build/688//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/688//artifact/incubator-tajo/patchprocess/newPatchFindbugsWarningstajo-storage-common.html Console output: https://builds.apache.org/job/PreCommit-TAJO-Build/688//console This message is automatically generated.
          Hide
          githubbot ASF GitHub Bot added a comment -

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

          https://github.com/apache/tajo/pull/489#discussion_r27720677

          — Diff: tajo-storage/tajo-storage-common/src/main/java/org/apache/tajo/storage/BufferPool.java —
          @@ -44,7 +44,7 @@ public static long maxDirectMemory() {
          }

          • public synchronized static ByteBuf directBuffer(int size) {
            + public static ByteBuf directBuffer(int size) {
              • End diff –

          Could you explain why you deleted synchronized keyword?

          Show
          githubbot ASF GitHub Bot added a comment - Github user blrunner commented on a diff in the pull request: https://github.com/apache/tajo/pull/489#discussion_r27720677 — Diff: tajo-storage/tajo-storage-common/src/main/java/org/apache/tajo/storage/BufferPool.java — @@ -44,7 +44,7 @@ public static long maxDirectMemory() { } public synchronized static ByteBuf directBuffer(int size) { + public static ByteBuf directBuffer(int size) { End diff – Could you explain why you deleted synchronized keyword?
          Hide
          githubbot ASF GitHub Bot added a comment -

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

          https://github.com/apache/tajo/pull/489#discussion_r27721152

          — Diff: tajo-storage/tajo-storage-common/src/main/java/org/apache/tajo/storage/FSDataInputChannel.java —
          @@ -0,0 +1,67 @@
          +/**
          + * 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.tajo.storage;
          +
          +import org.apache.hadoop.fs.ByteBufferReadable;
          +import org.apache.hadoop.fs.FSDataInputStream;
          +import org.apache.hadoop.io.IOUtils;
          +
          +import java.io.IOException;
          +import java.nio.ByteBuffer;
          +import java.nio.channels.Channels;
          +import java.nio.channels.ReadableByteChannel;
          +
          +/**
          + * FSDataInputChannel is a NIO channel implementation of direct read ability to read from HDFS
          + */
          +public final class FSDataInputChannel extends InputChannel implements SeekableChannel {
          +
          + private ReadableByteChannel channel;
          + private FSDataInputStream inputStream;
          + private boolean isDirectRead;
          +
          + public FSDataInputChannel(FSDataInputStream inputStream) {
          + if (inputStream.getWrappedStream() instanceof ByteBufferReadable) {
          — End diff –

          Is there no necessary for checking DFSInputStream?

          Show
          githubbot ASF GitHub Bot added a comment - Github user blrunner commented on a diff in the pull request: https://github.com/apache/tajo/pull/489#discussion_r27721152 — Diff: tajo-storage/tajo-storage-common/src/main/java/org/apache/tajo/storage/FSDataInputChannel.java — @@ -0,0 +1,67 @@ +/** + * 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.tajo.storage; + +import org.apache.hadoop.fs.ByteBufferReadable; +import org.apache.hadoop.fs.FSDataInputStream; +import org.apache.hadoop.io.IOUtils; + +import java.io.IOException; +import java.nio.ByteBuffer; +import java.nio.channels.Channels; +import java.nio.channels.ReadableByteChannel; + +/** + * FSDataInputChannel is a NIO channel implementation of direct read ability to read from HDFS + */ +public final class FSDataInputChannel extends InputChannel implements SeekableChannel { + + private ReadableByteChannel channel; + private FSDataInputStream inputStream; + private boolean isDirectRead; + + public FSDataInputChannel(FSDataInputStream inputStream) { + if (inputStream.getWrappedStream() instanceof ByteBufferReadable) { — End diff – Is there no necessary for checking DFSInputStream?
          Hide
          githubbot ASF GitHub Bot added a comment -

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

          https://github.com/apache/tajo/pull/489#discussion_r27721416

          — Diff: tajo-storage/tajo-storage-hdfs/src/main/java/org/apache/tajo/storage/FileAppender.java —
          @@ -53,6 +53,10 @@ public FileAppender(Configuration conf, TaskAttemptId taskAttemptId, Schema sche

          try {
          if (taskAttemptId != null) {
          + if (!(conf instanceof TajoConf)) {
          + throw new IllegalArgumentException("Configuration must be a TajoConf instance");
          — End diff –

          How about change the exception message as follows.
          ```
          Configuration must be an instance of TajoConf.
          (or Configuration is not an instance of TajoConf.)
          ```

          Show
          githubbot ASF GitHub Bot added a comment - Github user blrunner commented on a diff in the pull request: https://github.com/apache/tajo/pull/489#discussion_r27721416 — Diff: tajo-storage/tajo-storage-hdfs/src/main/java/org/apache/tajo/storage/FileAppender.java — @@ -53,6 +53,10 @@ public FileAppender(Configuration conf, TaskAttemptId taskAttemptId, Schema sche try { if (taskAttemptId != null) { + if (!(conf instanceof TajoConf)) { + throw new IllegalArgumentException("Configuration must be a TajoConf instance"); — End diff – How about change the exception message as follows. ``` Configuration must be an instance of TajoConf. (or Configuration is not an instance of TajoConf.) ```
          Hide
          githubbot ASF GitHub Bot added a comment -

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

          https://github.com/apache/tajo/pull/489#discussion_r27728204

          — Diff: tajo-storage/tajo-storage-common/src/main/java/org/apache/tajo/storage/BufferPool.java —
          @@ -44,7 +44,7 @@ public static long maxDirectMemory() {
          }

          • public synchronized static ByteBuf directBuffer(int size) {
            + public static ByteBuf directBuffer(int size) {
              • End diff –

          Because bufferPool does not have a critical section.

          Show
          githubbot ASF GitHub Bot added a comment - Github user jinossy commented on a diff in the pull request: https://github.com/apache/tajo/pull/489#discussion_r27728204 — Diff: tajo-storage/tajo-storage-common/src/main/java/org/apache/tajo/storage/BufferPool.java — @@ -44,7 +44,7 @@ public static long maxDirectMemory() { } public synchronized static ByteBuf directBuffer(int size) { + public static ByteBuf directBuffer(int size) { End diff – Because bufferPool does not have a critical section.
          Hide
          githubbot ASF GitHub Bot added a comment -

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

          https://github.com/apache/tajo/pull/489#discussion_r27728205

          — Diff: tajo-storage/tajo-storage-common/src/main/java/org/apache/tajo/storage/FSDataInputChannel.java —
          @@ -0,0 +1,67 @@
          +/**
          + * 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.tajo.storage;
          +
          +import org.apache.hadoop.fs.ByteBufferReadable;
          +import org.apache.hadoop.fs.FSDataInputStream;
          +import org.apache.hadoop.io.IOUtils;
          +
          +import java.io.IOException;
          +import java.nio.ByteBuffer;
          +import java.nio.channels.Channels;
          +import java.nio.channels.ReadableByteChannel;
          +
          +/**
          + * FSDataInputChannel is a NIO channel implementation of direct read ability to read from HDFS
          + */
          +public final class FSDataInputChannel extends InputChannel implements SeekableChannel {
          +
          + private ReadableByteChannel channel;
          + private FSDataInputStream inputStream;
          + private boolean isDirectRead;
          +
          + public FSDataInputChannel(FSDataInputStream inputStream) {
          + if (inputStream.getWrappedStream() instanceof ByteBufferReadable) {
          — End diff –

          Yes, you can find ByteBufferReadable inteface in getWrappedStream()

          Show
          githubbot ASF GitHub Bot added a comment - Github user jinossy commented on a diff in the pull request: https://github.com/apache/tajo/pull/489#discussion_r27728205 — Diff: tajo-storage/tajo-storage-common/src/main/java/org/apache/tajo/storage/FSDataInputChannel.java — @@ -0,0 +1,67 @@ +/** + * 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.tajo.storage; + +import org.apache.hadoop.fs.ByteBufferReadable; +import org.apache.hadoop.fs.FSDataInputStream; +import org.apache.hadoop.io.IOUtils; + +import java.io.IOException; +import java.nio.ByteBuffer; +import java.nio.channels.Channels; +import java.nio.channels.ReadableByteChannel; + +/** + * FSDataInputChannel is a NIO channel implementation of direct read ability to read from HDFS + */ +public final class FSDataInputChannel extends InputChannel implements SeekableChannel { + + private ReadableByteChannel channel; + private FSDataInputStream inputStream; + private boolean isDirectRead; + + public FSDataInputChannel(FSDataInputStream inputStream) { + if (inputStream.getWrappedStream() instanceof ByteBufferReadable) { — End diff – Yes, you can find ByteBufferReadable inteface in getWrappedStream()
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user jinossy commented on the pull request:

          https://github.com/apache/tajo/pull/489#issuecomment-89284561

          Thank you for the review.
          I've update the exception message

          Show
          githubbot ASF GitHub Bot added a comment - Github user jinossy commented on the pull request: https://github.com/apache/tajo/pull/489#issuecomment-89284561 Thank you for the review. I've update the exception message
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user jihoonson commented on the pull request:

          https://github.com/apache/tajo/pull/489#issuecomment-89289249

          Accelerant work!
          ```DelimitedTextFileScanner``` greatly worked with the BST index.

          Show
          githubbot ASF GitHub Bot added a comment - Github user jihoonson commented on the pull request: https://github.com/apache/tajo/pull/489#issuecomment-89289249 Accelerant work! ```DelimitedTextFileScanner``` greatly worked with the BST index.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user jinossy commented on the pull request:

          https://github.com/apache/tajo/pull/489#issuecomment-89306130

          In this patch, test case was lacking in content. I will add more unit test
          Thanks.

          Show
          githubbot ASF GitHub Bot added a comment - Github user jinossy commented on the pull request: https://github.com/apache/tajo/pull/489#issuecomment-89306130 In this patch, test case was lacking in content. I will add more unit test Thanks.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user jinossy commented on the pull request:

          https://github.com/apache/tajo/pull/489#issuecomment-89960257

          I've add more unit test

          Show
          githubbot ASF GitHub Bot added a comment - Github user jinossy commented on the pull request: https://github.com/apache/tajo/pull/489#issuecomment-89960257 I've add more unit test
          Hide
          tajoqa Tajo QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12723322/TAJO-1494_3.patch
          against master revision release-0.9.0-rc0-242-g72948b6.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 5 new or modified test files.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 javadoc. The applied patch does not increase the total number of javadoc warnings.

          +1 checkstyle. The patch generated 0 code style errors.

          -1 findbugs. The patch appears to introduce 25 new Findbugs (version 2.0.3) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed unit tests in tajo-core tajo-storage/tajo-storage-common tajo-storage/tajo-storage-hdfs.

          Test results: https://builds.apache.org/job/PreCommit-TAJO-Build/713//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/713//artifact/incubator-tajo/patchprocess/newPatchFindbugsWarningstajo-core.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/713//artifact/incubator-tajo/patchprocess/newPatchFindbugsWarningstajo-storage-common.html
          Console output: https://builds.apache.org/job/PreCommit-TAJO-Build/713//console

          This message is automatically generated.

          Show
          tajoqa Tajo QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12723322/TAJO-1494_3.patch against master revision release-0.9.0-rc0-242-g72948b6. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 5 new or modified test files. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 javadoc. The applied patch does not increase the total number of javadoc warnings. +1 checkstyle. The patch generated 0 code style errors. -1 findbugs. The patch appears to introduce 25 new Findbugs (version 2.0.3) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed unit tests in tajo-core tajo-storage/tajo-storage-common tajo-storage/tajo-storage-hdfs. Test results: https://builds.apache.org/job/PreCommit-TAJO-Build/713//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/713//artifact/incubator-tajo/patchprocess/newPatchFindbugsWarningstajo-core.html Findbugs warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/713//artifact/incubator-tajo/patchprocess/newPatchFindbugsWarningstajo-storage-common.html Console output: https://builds.apache.org/job/PreCommit-TAJO-Build/713//console This message is automatically generated.
          Hide
          githubbot ASF GitHub Bot added a comment -

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

          https://github.com/apache/tajo/pull/489#discussion_r27795739

          — Diff: tajo-storage/tajo-storage-common/src/main/java/org/apache/tajo/storage/BufferPool.java —
          @@ -44,7 +44,7 @@ public static long maxDirectMemory() {
          }

          • public synchronized static ByteBuf directBuffer(int size) {
            + public static ByteBuf directBuffer(int size) {
              • End diff –

          Oh! I see.

          Show
          githubbot ASF GitHub Bot added a comment - Github user blrunner commented on a diff in the pull request: https://github.com/apache/tajo/pull/489#discussion_r27795739 — Diff: tajo-storage/tajo-storage-common/src/main/java/org/apache/tajo/storage/BufferPool.java — @@ -44,7 +44,7 @@ public static long maxDirectMemory() { } public synchronized static ByteBuf directBuffer(int size) { + public static ByteBuf directBuffer(int size) { End diff – Oh! I see.
          Hide
          githubbot ASF GitHub Bot added a comment -

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

          https://github.com/apache/tajo/pull/489#discussion_r27795762

          — Diff: tajo-storage/tajo-storage-common/src/main/java/org/apache/tajo/storage/FSDataInputChannel.java —
          @@ -0,0 +1,67 @@
          +/**
          + * 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.tajo.storage;
          +
          +import org.apache.hadoop.fs.ByteBufferReadable;
          +import org.apache.hadoop.fs.FSDataInputStream;
          +import org.apache.hadoop.io.IOUtils;
          +
          +import java.io.IOException;
          +import java.nio.ByteBuffer;
          +import java.nio.channels.Channels;
          +import java.nio.channels.ReadableByteChannel;
          +
          +/**
          + * FSDataInputChannel is a NIO channel implementation of direct read ability to read from HDFS
          + */
          +public final class FSDataInputChannel extends InputChannel implements SeekableChannel {
          +
          + private ReadableByteChannel channel;
          + private FSDataInputStream inputStream;
          + private boolean isDirectRead;
          +
          + public FSDataInputChannel(FSDataInputStream inputStream) {
          + if (inputStream.getWrappedStream() instanceof ByteBufferReadable) {
          — End diff –

          O.K. Thanks for your feedback.

          Show
          githubbot ASF GitHub Bot added a comment - Github user blrunner commented on a diff in the pull request: https://github.com/apache/tajo/pull/489#discussion_r27795762 — Diff: tajo-storage/tajo-storage-common/src/main/java/org/apache/tajo/storage/FSDataInputChannel.java — @@ -0,0 +1,67 @@ +/** + * 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.tajo.storage; + +import org.apache.hadoop.fs.ByteBufferReadable; +import org.apache.hadoop.fs.FSDataInputStream; +import org.apache.hadoop.io.IOUtils; + +import java.io.IOException; +import java.nio.ByteBuffer; +import java.nio.channels.Channels; +import java.nio.channels.ReadableByteChannel; + +/** + * FSDataInputChannel is a NIO channel implementation of direct read ability to read from HDFS + */ +public final class FSDataInputChannel extends InputChannel implements SeekableChannel { + + private ReadableByteChannel channel; + private FSDataInputStream inputStream; + private boolean isDirectRead; + + public FSDataInputChannel(FSDataInputStream inputStream) { + if (inputStream.getWrappedStream() instanceof ByteBufferReadable) { — End diff – O.K. Thanks for your feedback.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user blrunner commented on the pull request:

          https://github.com/apache/tajo/pull/489#issuecomment-90296550

          +1

          Ship it.

          Show
          githubbot ASF GitHub Bot added a comment - Github user blrunner commented on the pull request: https://github.com/apache/tajo/pull/489#issuecomment-90296550 +1 Ship it.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

          https://github.com/apache/tajo/pull/489

          Show
          githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/tajo/pull/489
          Hide
          jhkim Jinho Kim added a comment -

          Committed it
          Thank you for the review!

          Show
          jhkim Jinho Kim added a comment - Committed it Thank you for the review!
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Tajo-master-CODEGEN-build #294 (See https://builds.apache.org/job/Tajo-master-CODEGEN-build/294/)
          TAJO-1494: Add SeekableScanner support to DelimitedTextFileScanner. (jhkim: rev 633109ac75bc8036e49eba8ea48c025fc0f342da)

          • CHANGES
          • tajo-storage/tajo-storage-hdfs/src/main/java/org/apache/tajo/storage/text/DelimitedTextFile.java
          • tajo-storage/tajo-storage-hdfs/src/main/java/org/apache/tajo/storage/FileAppender.java
          • tajo-storage/tajo-storage-hdfs/src/main/java/org/apache/tajo/storage/text/ByteBufLineReader.java
          • tajo-storage/tajo-storage-common/src/main/java/org/apache/tajo/storage/FSDataInputChannel.java
          • tajo-storage/tajo-storage-hdfs/src/test/java/org/apache/tajo/storage/TestLineReader.java
          • tajo-storage/tajo-storage-common/src/main/java/org/apache/tajo/storage/LocalFileInputChannel.java
          • tajo-storage/tajo-storage-hdfs/src/test/java/org/apache/tajo/storage/index/TestBSTIndex.java
          • tajo-storage/tajo-storage-hdfs/src/test/java/org/apache/tajo/storage/TestByteBufLineReader.java
          • tajo-storage/tajo-storage-common/src/main/java/org/apache/tajo/storage/BufferPool.java
          • tajo-storage/tajo-storage-common/src/main/java/org/apache/tajo/storage/SeekableChannel.java
          • tajo-storage/tajo-storage-hdfs/src/main/java/org/apache/tajo/storage/text/DelimitedLineReader.java
          • tajo-storage/tajo-storage-hdfs/src/main/java/org/apache/tajo/storage/thirdparty/parquet/CodecFactory.java
          • tajo-core/src/test/java/org/apache/tajo/LocalTajoTestingUtility.java
          • tajo-storage/tajo-storage-common/src/main/java/org/apache/tajo/storage/ByteBufInputChannel.java
          • tajo-storage/tajo-storage-hdfs/src/test/java/org/apache/tajo/storage/TestStorages.java
          • tajo-storage/tajo-storage-hdfs/src/main/java/org/apache/tajo/storage/text/LineSplitProcessor.java
          • tajo-storage/tajo-storage-common/src/main/java/org/apache/tajo/storage/InputChannel.java
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Tajo-master-CODEGEN-build #294 (See https://builds.apache.org/job/Tajo-master-CODEGEN-build/294/ ) TAJO-1494 : Add SeekableScanner support to DelimitedTextFileScanner. (jhkim: rev 633109ac75bc8036e49eba8ea48c025fc0f342da) CHANGES tajo-storage/tajo-storage-hdfs/src/main/java/org/apache/tajo/storage/text/DelimitedTextFile.java tajo-storage/tajo-storage-hdfs/src/main/java/org/apache/tajo/storage/FileAppender.java tajo-storage/tajo-storage-hdfs/src/main/java/org/apache/tajo/storage/text/ByteBufLineReader.java tajo-storage/tajo-storage-common/src/main/java/org/apache/tajo/storage/FSDataInputChannel.java tajo-storage/tajo-storage-hdfs/src/test/java/org/apache/tajo/storage/TestLineReader.java tajo-storage/tajo-storage-common/src/main/java/org/apache/tajo/storage/LocalFileInputChannel.java tajo-storage/tajo-storage-hdfs/src/test/java/org/apache/tajo/storage/index/TestBSTIndex.java tajo-storage/tajo-storage-hdfs/src/test/java/org/apache/tajo/storage/TestByteBufLineReader.java tajo-storage/tajo-storage-common/src/main/java/org/apache/tajo/storage/BufferPool.java tajo-storage/tajo-storage-common/src/main/java/org/apache/tajo/storage/SeekableChannel.java tajo-storage/tajo-storage-hdfs/src/main/java/org/apache/tajo/storage/text/DelimitedLineReader.java tajo-storage/tajo-storage-hdfs/src/main/java/org/apache/tajo/storage/thirdparty/parquet/CodecFactory.java tajo-core/src/test/java/org/apache/tajo/LocalTajoTestingUtility.java tajo-storage/tajo-storage-common/src/main/java/org/apache/tajo/storage/ByteBufInputChannel.java tajo-storage/tajo-storage-hdfs/src/test/java/org/apache/tajo/storage/TestStorages.java tajo-storage/tajo-storage-hdfs/src/main/java/org/apache/tajo/storage/text/LineSplitProcessor.java tajo-storage/tajo-storage-common/src/main/java/org/apache/tajo/storage/InputChannel.java
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Tajo-master-build #656 (See https://builds.apache.org/job/Tajo-master-build/656/)
          TAJO-1494: Add SeekableScanner support to DelimitedTextFileScanner. (jhkim: rev 633109ac75bc8036e49eba8ea48c025fc0f342da)

          • tajo-storage/tajo-storage-common/src/main/java/org/apache/tajo/storage/FSDataInputChannel.java
          • tajo-storage/tajo-storage-hdfs/src/main/java/org/apache/tajo/storage/text/DelimitedLineReader.java
          • tajo-storage/tajo-storage-hdfs/src/main/java/org/apache/tajo/storage/text/LineSplitProcessor.java
          • tajo-storage/tajo-storage-hdfs/src/test/java/org/apache/tajo/storage/TestLineReader.java
          • tajo-storage/tajo-storage-common/src/main/java/org/apache/tajo/storage/ByteBufInputChannel.java
          • tajo-storage/tajo-storage-hdfs/src/test/java/org/apache/tajo/storage/TestStorages.java
          • tajo-storage/tajo-storage-hdfs/src/main/java/org/apache/tajo/storage/text/DelimitedTextFile.java
          • tajo-storage/tajo-storage-hdfs/src/test/java/org/apache/tajo/storage/index/TestBSTIndex.java
          • tajo-storage/tajo-storage-hdfs/src/main/java/org/apache/tajo/storage/thirdparty/parquet/CodecFactory.java
          • tajo-storage/tajo-storage-common/src/main/java/org/apache/tajo/storage/BufferPool.java
          • tajo-storage/tajo-storage-hdfs/src/main/java/org/apache/tajo/storage/FileAppender.java
          • tajo-storage/tajo-storage-common/src/main/java/org/apache/tajo/storage/SeekableChannel.java
          • tajo-storage/tajo-storage-hdfs/src/test/java/org/apache/tajo/storage/TestByteBufLineReader.java
          • tajo-storage/tajo-storage-common/src/main/java/org/apache/tajo/storage/LocalFileInputChannel.java
          • tajo-storage/tajo-storage-hdfs/src/main/java/org/apache/tajo/storage/text/ByteBufLineReader.java
          • tajo-core/src/test/java/org/apache/tajo/LocalTajoTestingUtility.java
          • tajo-storage/tajo-storage-common/src/main/java/org/apache/tajo/storage/InputChannel.java
          • CHANGES
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Tajo-master-build #656 (See https://builds.apache.org/job/Tajo-master-build/656/ ) TAJO-1494 : Add SeekableScanner support to DelimitedTextFileScanner. (jhkim: rev 633109ac75bc8036e49eba8ea48c025fc0f342da) tajo-storage/tajo-storage-common/src/main/java/org/apache/tajo/storage/FSDataInputChannel.java tajo-storage/tajo-storage-hdfs/src/main/java/org/apache/tajo/storage/text/DelimitedLineReader.java tajo-storage/tajo-storage-hdfs/src/main/java/org/apache/tajo/storage/text/LineSplitProcessor.java tajo-storage/tajo-storage-hdfs/src/test/java/org/apache/tajo/storage/TestLineReader.java tajo-storage/tajo-storage-common/src/main/java/org/apache/tajo/storage/ByteBufInputChannel.java tajo-storage/tajo-storage-hdfs/src/test/java/org/apache/tajo/storage/TestStorages.java tajo-storage/tajo-storage-hdfs/src/main/java/org/apache/tajo/storage/text/DelimitedTextFile.java tajo-storage/tajo-storage-hdfs/src/test/java/org/apache/tajo/storage/index/TestBSTIndex.java tajo-storage/tajo-storage-hdfs/src/main/java/org/apache/tajo/storage/thirdparty/parquet/CodecFactory.java tajo-storage/tajo-storage-common/src/main/java/org/apache/tajo/storage/BufferPool.java tajo-storage/tajo-storage-hdfs/src/main/java/org/apache/tajo/storage/FileAppender.java tajo-storage/tajo-storage-common/src/main/java/org/apache/tajo/storage/SeekableChannel.java tajo-storage/tajo-storage-hdfs/src/test/java/org/apache/tajo/storage/TestByteBufLineReader.java tajo-storage/tajo-storage-common/src/main/java/org/apache/tajo/storage/LocalFileInputChannel.java tajo-storage/tajo-storage-hdfs/src/main/java/org/apache/tajo/storage/text/ByteBufLineReader.java tajo-core/src/test/java/org/apache/tajo/LocalTajoTestingUtility.java tajo-storage/tajo-storage-common/src/main/java/org/apache/tajo/storage/InputChannel.java CHANGES

            People

            • Assignee:
              jhkim Jinho Kim
              Reporter:
              jhkim Jinho Kim
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development