You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by bbende <gi...@git.apache.org> on 2016/10/03 14:54:03 UTC

[GitHub] nifi pull request #1088: NIFI-2841 Refactoring logic in SplitAvro RecordSpli...

GitHub user bbende opened a pull request:

    https://github.com/apache/nifi/pull/1088

    NIFI-2841 Refactoring logic in SplitAvro RecordSplitter to avoid maki\u2026

    \u2026ng two calls in a row to reader.hasNext()

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

    $ git pull https://github.com/bbende/nifi NIFI-2841

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

    https://github.com/apache/nifi/pull/1088.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 #1088
    
----
commit b46e700d17240e7fe1f589ef7ca9b41e1a86a2bb
Author: Bryan Bende <bb...@apache.org>
Date:   2016-09-29T17:49:27Z

    NIFI-2841 Refactoring logic in SplitAvro RecordSplitter to avoid making two calls in a row to reader.hasNext()

----


---
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] nifi pull request #1088: NIFI-2841 Refactoring logic in SplitAvro RecordSpli...

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

    https://github.com/apache/nifi/pull/1088#discussion_r86388099
  
    --- Diff: nifi-nar-bundles/nifi-avro-bundle/nifi-avro-processors/src/main/java/org/apache/nifi/processors/avro/SplitAvro.java ---
    @@ -267,7 +267,8 @@ public void process(InputStream rawIn) throws IOException {
                             }
     
                             // while records are left, start a new split by spawning a FlowFile
    -                        while (reader.hasNext()) {
    +                        final AtomicReference<Boolean> hasNextHolder = new AtomicReference<Boolean>(reader.hasNext());
    --- End diff --
    
    If there are no records, then the call to reader.next() [will throw a NoSuchElementException](https://avro.apache.org/docs/1.7.7/api/java/org/apache/avro/file/DataFileStream.html#next(D)). You could wrap the code in a try/catch, but then it's a little heavier, so the AtomicReference is probably fine?


---
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] nifi issue #1088: NIFI-2841 Refactoring logic in SplitAvro RecordSplitter to...

Posted by olegz <gi...@git.apache.org>.
Github user olegz commented on the issue:

    https://github.com/apache/nifi/pull/1088
  
    Reviewing. . .


---
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] nifi pull request #1088: NIFI-2841 Refactoring logic in SplitAvro RecordSpli...

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

    https://github.com/apache/nifi/pull/1088#discussion_r83463726
  
    --- Diff: nifi-nar-bundles/nifi-avro-bundle/nifi-avro-processors/src/main/java/org/apache/nifi/processors/avro/SplitAvro.java ---
    @@ -267,7 +267,8 @@ public void process(InputStream rawIn) throws IOException {
                             }
     
                             // while records are left, start a new split by spawning a FlowFile
    -                        while (reader.hasNext()) {
    +                        final AtomicReference<Boolean> hasNextHolder = new AtomicReference<Boolean>(reader.hasNext());
    --- End diff --
    
    I think instead of introducing AtomicReference we can make it a bit lighter by simply changing the inner loop to be a _do/while_ loop
    ```
    . . .
    int recordCount = 0;                                      
    do {
        recordHolder.set(reader.next(recordHolder.get()));
        splitWriter.write(recordHolder.get());
        recordCount++;
     } while (reader.hasNext() && recordCount < splitSize);
    . . .
    ```


---
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] nifi pull request #1088: NIFI-2841 Refactoring logic in SplitAvro RecordSpli...

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

    https://github.com/apache/nifi/pull/1088


---
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] nifi pull request #1088: NIFI-2841 Refactoring logic in SplitAvro RecordSpli...

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

    https://github.com/apache/nifi/pull/1088#discussion_r87056058
  
    --- Diff: nifi-nar-bundles/nifi-avro-bundle/nifi-avro-processors/src/main/java/org/apache/nifi/processors/avro/SplitAvro.java ---
    @@ -267,7 +267,8 @@ public void process(InputStream rawIn) throws IOException {
                             }
     
                             // while records are left, start a new split by spawning a FlowFile
    -                        while (reader.hasNext()) {
    +                        final AtomicReference<Boolean> hasNextHolder = new AtomicReference<Boolean>(reader.hasNext());
    --- End diff --
    
    Yeah, you right and in the end it's more of a stylistic comment. So, I am good.


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