You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by "ASF GitHub Bot (JIRA)" <ji...@apache.org> on 2017/01/18 08:51:26 UTC

[jira] [Commented] (NIFI-3363) PutKafka throws NullPointerException when User-Defined partition strategy is used

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

ASF GitHub Bot commented on NIFI-3363:
--------------------------------------

GitHub user ijokarumawak opened a pull request:

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

    NIFI-3363: PutKafka NPE with User-Defined partition

    - Marked PutKafka Partition Strategy property as deprecated, as Kafka 0.8 client doesn't use 'partitioner.class' as producer property, we don't have to specify it.
    - Changed Partition Strategy property from a required one to a dynamic property, so that existing processor config can stay in valid state.
    - Fixed partition property to work.
    - Route a flow file if it failed to be published due to invalid partition.
    
    Thank you for submitting a contribution to Apache NiFi.
    
    In order to streamline the review of the contribution we ask you
    to ensure the following steps have been taken:
    
    ### For all changes:
    - [x] Is there a JIRA ticket associated with this PR? Is it referenced 
         in the commit message?
    
    - [x] Does your PR title start with NIFI-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
    
    - [x] Has your PR been rebased against the latest commit within the target branch (typically master)?
    
    - [x] Is your initial contribution a single, squashed commit?
    
    ### For code changes:
    - [x] Have you ensured that the full suite of tests is executed via mvn -Pcontrib-check clean install at the root nifi folder?
    - [x] Have you written or updated unit tests to verify your changes?
    - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? 
    - [ ] If applicable, have you updated the LICENSE file, including the main LICENSE file under nifi-assembly?
    - [ ] If applicable, have you updated the NOTICE file, including the main NOTICE file found under nifi-assembly?
    - [ ] If adding new Properties, have you added .displayName in addition to .name (programmatic access) for each of the new properties?
    
    ### For documentation related changes:
    - [ ] Have you ensured that format looks appropriate for the output in which it is rendered?
    
    ### Note:
    Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible.


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

    $ git pull https://github.com/ijokarumawak/nifi nifi-3363

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

    https://github.com/apache/nifi/pull/1425.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 #1425
    
----
commit 01dc802bafc8fcea8fed670faaded012804aba6f
Author: Koji Kawamura <ij...@apache.org>
Date:   2017-01-18T07:48:09Z

    NIFI-3363: PutKafka NPE with User-Defined partition
    
    - Marked PutKafka Partition Strategy property as deprecated, as Kafka 0.8 client doesn't use 'partitioner.class' as producer property, we don't have to specify it.
    - Changed Partition Strategy property from a required one to a dynamic property, so that existing processor config can stay in valid state.
    - Fixed partition property to work.
    - Route a flow file if it failed to be published due to invalid partition.

----


> PutKafka throws NullPointerException when User-Defined partition strategy is used
> ---------------------------------------------------------------------------------
>
>                 Key: NIFI-3363
>                 URL: https://issues.apache.org/jira/browse/NIFI-3363
>             Project: Apache NiFi
>          Issue Type: Bug
>          Components: Extensions
>    Affects Versions: 1.0.0, 0.4.0, 0.5.0, 0.6.0, 0.4.1, 0.5.1, 0.7.0, 0.6.1, 1.1.0, 0.7.1, 1.1.1, 1.0.1
>         Environment: By looking at the release tags contained in a commit that added User-Defined partition strategy (NIFI-1097 22de23b), it seems all NiFi versions since 0.4.0 is affected.
>            Reporter: Koji Kawamura
>            Assignee: Koji Kawamura
>
> NullPointerException is thrown because PutKafka tries to put null into properties since following if statements doesn't cover USER_DEFINED_PARTITIONING.
> {code: title=PutKafka.java buildKafkaConfigProperties}
> String partitionStrategy = context.getProperty(PARTITION_STRATEGY).getValue();
> String partitionerClass = null;
> if (partitionStrategy.equalsIgnoreCase(ROUND_ROBIN_PARTITIONING.getValue())) {
>     partitionerClass = Partitioners.RoundRobinPartitioner.class.getName();
> } else if (partitionStrategy.equalsIgnoreCase(RANDOM_PARTITIONING.getValue())) {
>     partitionerClass = Partitioners.RandomPartitioner.class.getName();
> }
> properties.setProperty("partitioner.class", partitionerClass); // Happens here
> {code}
> A naive fix for this would be adding one more if statement so that it puts 'partitioner.class' property only if partitionerClass is set.
> However, while I was testing the fix, I found following facts, that revealed this approach wouldn't be the right solution for this issue.
> In short, we don't have to set 'partitioner.class' property with Kafka 0.8.x client in the first place. I assume it's there because PutKafka came through a long history..
> h2. PutKafka history analysis
> - PutKafka used to cover Kafka 0.8 and 0.9
> - Around the time Kafka 0.9 was released, PutKafka added 'partitioner.class' via NIFI-1097: start using new API. There were two client libraries kafka-clients and kafka_2.9.1, both 0.8.2.2.
> - Then PublishKafka is added for Kafka 0.9, at this point, we could add 'partition' property to PublishKafka, but we didn't do that for some reason. PublishKafka doesn't support user defined partition as of this writing. NIFI-1296.
> - The code adding 'partitioner.class' has been left in PutKafka.
> - Further, we separated nar into 0.8, 0.9 and 0.10.
> - Now only PutKafka(0.8) uses 'partitioner.class' property, but 0.8 client doesn't use that property. So we don't need that code at all.
> h2. Then, how should we fix this?
> Since PutKafka in both master and 0.x branch specifically uses Kafka 0.8.x client. We can simply remove the codes adding 'partitioner.class', probably PARTITION_STRATEGY processor property, too.
> h2. Expected result after fix
> - Users can specify Kafka partition with PutKafka 'partition' property, but no need to specify 'partition strategy', NullPointerException won't be thrown
> - A warning log, that used to be logged in nifi-app.log won't be logged any more: {code}2017-01-18 13:53:33,071 WARN [Timer-Driven Process Thread-9] o.a.k.clients.producer.ProducerConfig The configuration partitioner.class = null was supplied but isn't a known config.{code}



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)