You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@atlas.apache.org by Tom Beerbower <tb...@hortonworks.com> on 2015/10/09 03:40:45 UTC

Review Request 39157: Configure Consumer Groups for Notifications

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39157/
-----------------------------------------------------------

Review request for atlas, John Speidel and Shwetha GS.


Bugs: ATLAS-215
    https://issues.apache.org/jira/browse/ATLAS-215


Repository: atlas


Description
-------

Currently with the notification framework we set the properties so that all consumers are created in a single group...

        //todo take group id as argument to allow multiple consumers??
        properties.put(ConsumerConfig.GROUP_ID_CONFIG, ATLAS_GROUP);

Having Kafka consumers for both the HOOK and ENTITIES topics in a single group causes this issue ...

    java.lang.IllegalArgumentException: requirement failed: Round-robin assignment is allowed only if all consumers in the group subscribe to the same topics, AND if the stream counts across topics are identical for a given consumer instance.
    Topic ATLAS_ENTITIES has the following available consumer streams: Set(atlas_c6401.ambari.apache.org-1443714948705-e518b690-0)
    Topic ATLAS_HOOK has the following available consumer streams: Set(atlas_c6401.ambari.apache.org-1443714214755-8fd9a5f8-0)

We shouldn't have HOOK consumers and ENTITIES consumers in the same group.


Diffs
-----

  notification/pom.xml 2e12520 
  notification/src/main/java/org/apache/atlas/kafka/KafkaNotification.java 7b3cf89 
  notification/src/main/java/org/apache/atlas/notification/NotificationHookConsumer.java 29194a4 
  notification/src/main/java/org/apache/atlas/notification/NotificationInterface.java d4be07b 
  notification/src/test/java/org/apache/atlas/kafka/KafkaNotificationTest.java 735655c 

Diff: https://reviews.apache.org/r/39157/diff/


Testing
-------

new unit test added

mvn clean test

[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 12.246 s
[INFO] Finished at: 2015-10-08T21:37:55-04:00
[INFO] Final Memory: 40M/605M
[INFO] ------------------------------------------------------------------------


Thanks,

Tom Beerbower

Re: Review Request 39157: Configure Consumer Groups for Notifications

Posted by Tom Beerbower <tb...@hortonworks.com>.

> On Oct. 9, 2015, 4:23 p.m., Suma Shivaprasad wrote:
> > Where is consumer group for entity notifications from ATLAS taken care of?

Thanks for the review!

For entity notification, some other application (i.e. Ranger) is the consumer.  The usage would be something like this ...


    // Obtain provider through injection…
    EntityNotificationConsumerProvider consumerProvider;

    EntityNotificationConsumer consumer = consumerProvider.get();

    …

    while(consumer.hasNext()) {
        EntityNotification notification = consumer.next();


        Entity entity = notification.getEntity();
        …
    }

The consumer group would be specified in the application.properties configuration along with the other kafka specific configs ...

    atlas.kafka.bootstrap.servers=c6401.ambari.apache.org:6667
    atlas.kafka.zookeeper.connect=c6401.ambari.apache.org:2181
    atlas.kafka.group.id=entityConsumer

... and would be passed into the call to createConsumers() when the EntityNotificationConsumer is obtained.

I'll have to merge to read the configuration and pass the consumer group id in the entity notification patch https://reviews.apache.org/r/38341/ (which is still pending) if this change is pushed first.


- Tom


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39157/#review102073
-----------------------------------------------------------


On Oct. 9, 2015, 1:40 a.m., Tom Beerbower wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39157/
> -----------------------------------------------------------
> 
> (Updated Oct. 9, 2015, 1:40 a.m.)
> 
> 
> Review request for atlas, John Speidel and Shwetha GS.
> 
> 
> Bugs: ATLAS-215
>     https://issues.apache.org/jira/browse/ATLAS-215
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> Currently with the notification framework we set the properties so that all consumers are created in a single group...
> 
>         //todo take group id as argument to allow multiple consumers??
>         properties.put(ConsumerConfig.GROUP_ID_CONFIG, ATLAS_GROUP);
> 
> Having Kafka consumers for both the HOOK and ENTITIES topics in a single group causes this issue ...
> 
>     java.lang.IllegalArgumentException: requirement failed: Round-robin assignment is allowed only if all consumers in the group subscribe to the same topics, AND if the stream counts across topics are identical for a given consumer instance.
>     Topic ATLAS_ENTITIES has the following available consumer streams: Set(atlas_c6401.ambari.apache.org-1443714948705-e518b690-0)
>     Topic ATLAS_HOOK has the following available consumer streams: Set(atlas_c6401.ambari.apache.org-1443714214755-8fd9a5f8-0)
> 
> We shouldn't have HOOK consumers and ENTITIES consumers in the same group.
> 
> 
> Diffs
> -----
> 
>   notification/pom.xml 2e12520 
>   notification/src/main/java/org/apache/atlas/kafka/KafkaNotification.java 7b3cf89 
>   notification/src/main/java/org/apache/atlas/notification/NotificationHookConsumer.java 29194a4 
>   notification/src/main/java/org/apache/atlas/notification/NotificationInterface.java d4be07b 
>   notification/src/test/java/org/apache/atlas/kafka/KafkaNotificationTest.java 735655c 
> 
> Diff: https://reviews.apache.org/r/39157/diff/
> 
> 
> Testing
> -------
> 
> new unit test added
> 
> mvn clean test
> 
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 12.246 s
> [INFO] Finished at: 2015-10-08T21:37:55-04:00
> [INFO] Final Memory: 40M/605M
> [INFO] ------------------------------------------------------------------------
> 
> 
> Thanks,
> 
> Tom Beerbower
> 
>

Re: Review Request 39157: Configure Consumer Groups for Notifications

Posted by Suma Shivaprasad <su...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39157/#review102073
-----------------------------------------------------------


Where is consumer group for entity notifications from ATLAS taken care of?

- Suma Shivaprasad


On Oct. 9, 2015, 1:40 a.m., Tom Beerbower wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39157/
> -----------------------------------------------------------
> 
> (Updated Oct. 9, 2015, 1:40 a.m.)
> 
> 
> Review request for atlas, John Speidel and Shwetha GS.
> 
> 
> Bugs: ATLAS-215
>     https://issues.apache.org/jira/browse/ATLAS-215
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> Currently with the notification framework we set the properties so that all consumers are created in a single group...
> 
>         //todo take group id as argument to allow multiple consumers??
>         properties.put(ConsumerConfig.GROUP_ID_CONFIG, ATLAS_GROUP);
> 
> Having Kafka consumers for both the HOOK and ENTITIES topics in a single group causes this issue ...
> 
>     java.lang.IllegalArgumentException: requirement failed: Round-robin assignment is allowed only if all consumers in the group subscribe to the same topics, AND if the stream counts across topics are identical for a given consumer instance.
>     Topic ATLAS_ENTITIES has the following available consumer streams: Set(atlas_c6401.ambari.apache.org-1443714948705-e518b690-0)
>     Topic ATLAS_HOOK has the following available consumer streams: Set(atlas_c6401.ambari.apache.org-1443714214755-8fd9a5f8-0)
> 
> We shouldn't have HOOK consumers and ENTITIES consumers in the same group.
> 
> 
> Diffs
> -----
> 
>   notification/pom.xml 2e12520 
>   notification/src/main/java/org/apache/atlas/kafka/KafkaNotification.java 7b3cf89 
>   notification/src/main/java/org/apache/atlas/notification/NotificationHookConsumer.java 29194a4 
>   notification/src/main/java/org/apache/atlas/notification/NotificationInterface.java d4be07b 
>   notification/src/test/java/org/apache/atlas/kafka/KafkaNotificationTest.java 735655c 
> 
> Diff: https://reviews.apache.org/r/39157/diff/
> 
> 
> Testing
> -------
> 
> new unit test added
> 
> mvn clean test
> 
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 12.246 s
> [INFO] Finished at: 2015-10-08T21:37:55-04:00
> [INFO] Final Memory: 40M/605M
> [INFO] ------------------------------------------------------------------------
> 
> 
> Thanks,
> 
> Tom Beerbower
> 
>

Re: Review Request 39157: Configure Consumer Groups for Notifications

Posted by Shwetha GS <ss...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39157/#review102204
-----------------------------------------------------------



notification/src/main/java/org/apache/atlas/kafka/KafkaNotification.java (line 332)
<https://reviews.apache.org/r/39157/#comment159765>

    Instead of the if condition, add property in application.properties for atlas.kafka.hook.group.id. In general group id should be read from atlas.kafka.<topicname>.group.id


- Shwetha GS


On Oct. 12, 2015, 2:22 a.m., Tom Beerbower wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39157/
> -----------------------------------------------------------
> 
> (Updated Oct. 12, 2015, 2:22 a.m.)
> 
> 
> Review request for atlas, John Speidel and Shwetha GS.
> 
> 
> Bugs: ATLAS-215
>     https://issues.apache.org/jira/browse/ATLAS-215
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> Currently with the notification framework we set the properties so that all consumers are created in a single group...
> 
>         //todo take group id as argument to allow multiple consumers??
>         properties.put(ConsumerConfig.GROUP_ID_CONFIG, ATLAS_GROUP);
> 
> Having Kafka consumers for both the HOOK and ENTITIES topics in a single group causes this issue ...
> 
>     java.lang.IllegalArgumentException: requirement failed: Round-robin assignment is allowed only if all consumers in the group subscribe to the same topics, AND if the stream counts across topics are identical for a given consumer instance.
>     Topic ATLAS_ENTITIES has the following available consumer streams: Set(atlas_c6401.ambari.apache.org-1443714948705-e518b690-0)
>     Topic ATLAS_HOOK has the following available consumer streams: Set(atlas_c6401.ambari.apache.org-1443714214755-8fd9a5f8-0)
> 
> We shouldn't have HOOK consumers and ENTITIES consumers in the same group.
> 
> 
> Diffs
> -----
> 
>   notification/pom.xml 2e12520 
>   notification/src/main/java/org/apache/atlas/kafka/KafkaNotification.java 7b3cf89 
>   notification/src/test/java/org/apache/atlas/kafka/KafkaNotificationTest.java 735655c 
> 
> Diff: https://reviews.apache.org/r/39157/diff/
> 
> 
> Testing
> -------
> 
> new unit test added
> 
> mvn clean test
> 
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 12.246 s
> [INFO] Finished at: 2015-10-08T21:37:55-04:00
> [INFO] Final Memory: 40M/605M
> [INFO] ------------------------------------------------------------------------
> 
> 
> Thanks,
> 
> Tom Beerbower
> 
>

Re: Review Request 39157: Configure Consumer Groups for Notifications

Posted by Tom Beerbower <tb...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39157/
-----------------------------------------------------------

(Updated Oct. 12, 2015, 2:22 a.m.)


Review request for atlas, John Speidel and Shwetha GS.


Changes
-------

Update diff based on Shwetha's comments.


Bugs: ATLAS-215
    https://issues.apache.org/jira/browse/ATLAS-215


Repository: atlas


Description
-------

Currently with the notification framework we set the properties so that all consumers are created in a single group...

        //todo take group id as argument to allow multiple consumers??
        properties.put(ConsumerConfig.GROUP_ID_CONFIG, ATLAS_GROUP);

Having Kafka consumers for both the HOOK and ENTITIES topics in a single group causes this issue ...

    java.lang.IllegalArgumentException: requirement failed: Round-robin assignment is allowed only if all consumers in the group subscribe to the same topics, AND if the stream counts across topics are identical for a given consumer instance.
    Topic ATLAS_ENTITIES has the following available consumer streams: Set(atlas_c6401.ambari.apache.org-1443714948705-e518b690-0)
    Topic ATLAS_HOOK has the following available consumer streams: Set(atlas_c6401.ambari.apache.org-1443714214755-8fd9a5f8-0)

We shouldn't have HOOK consumers and ENTITIES consumers in the same group.


Diffs (updated)
-----

  notification/pom.xml 2e12520 
  notification/src/main/java/org/apache/atlas/kafka/KafkaNotification.java 7b3cf89 
  notification/src/test/java/org/apache/atlas/kafka/KafkaNotificationTest.java 735655c 

Diff: https://reviews.apache.org/r/39157/diff/


Testing
-------

new unit test added

mvn clean test

[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 12.246 s
[INFO] Finished at: 2015-10-08T21:37:55-04:00
[INFO] Final Memory: 40M/605M
[INFO] ------------------------------------------------------------------------


Thanks,

Tom Beerbower

Re: Review Request 39157: Configure Consumer Groups for Notifications

Posted by Suma Shivaprasad <su...@gmail.com>.

> On Oct. 9, 2015, 4 p.m., Suma Shivaprasad wrote:
> > notification/src/main/java/org/apache/atlas/kafka/KafkaNotification.java, line 226
> > <https://reviews.apache.org/r/39157/diff/1/?file=1093468#file1093468line226>
> >
> >     consumerConnector need to be shutdown

This seems to be taken care of in close() already


- Suma


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39157/#review102064
-----------------------------------------------------------


On Oct. 9, 2015, 1:40 a.m., Tom Beerbower wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39157/
> -----------------------------------------------------------
> 
> (Updated Oct. 9, 2015, 1:40 a.m.)
> 
> 
> Review request for atlas, John Speidel and Shwetha GS.
> 
> 
> Bugs: ATLAS-215
>     https://issues.apache.org/jira/browse/ATLAS-215
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> Currently with the notification framework we set the properties so that all consumers are created in a single group...
> 
>         //todo take group id as argument to allow multiple consumers??
>         properties.put(ConsumerConfig.GROUP_ID_CONFIG, ATLAS_GROUP);
> 
> Having Kafka consumers for both the HOOK and ENTITIES topics in a single group causes this issue ...
> 
>     java.lang.IllegalArgumentException: requirement failed: Round-robin assignment is allowed only if all consumers in the group subscribe to the same topics, AND if the stream counts across topics are identical for a given consumer instance.
>     Topic ATLAS_ENTITIES has the following available consumer streams: Set(atlas_c6401.ambari.apache.org-1443714948705-e518b690-0)
>     Topic ATLAS_HOOK has the following available consumer streams: Set(atlas_c6401.ambari.apache.org-1443714214755-8fd9a5f8-0)
> 
> We shouldn't have HOOK consumers and ENTITIES consumers in the same group.
> 
> 
> Diffs
> -----
> 
>   notification/pom.xml 2e12520 
>   notification/src/main/java/org/apache/atlas/kafka/KafkaNotification.java 7b3cf89 
>   notification/src/main/java/org/apache/atlas/notification/NotificationHookConsumer.java 29194a4 
>   notification/src/main/java/org/apache/atlas/notification/NotificationInterface.java d4be07b 
>   notification/src/test/java/org/apache/atlas/kafka/KafkaNotificationTest.java 735655c 
> 
> Diff: https://reviews.apache.org/r/39157/diff/
> 
> 
> Testing
> -------
> 
> new unit test added
> 
> mvn clean test
> 
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 12.246 s
> [INFO] Finished at: 2015-10-08T21:37:55-04:00
> [INFO] Final Memory: 40M/605M
> [INFO] ------------------------------------------------------------------------
> 
> 
> Thanks,
> 
> Tom Beerbower
> 
>

Re: Review Request 39157: Configure Consumer Groups for Notifications

Posted by Suma Shivaprasad <su...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39157/#review102064
-----------------------------------------------------------



notification/src/main/java/org/apache/atlas/kafka/KafkaNotification.java (line 223)
<https://reviews.apache.org/r/39157/#comment159590>

    consumerConnector need to be shutdown


- Suma Shivaprasad


On Oct. 9, 2015, 1:40 a.m., Tom Beerbower wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39157/
> -----------------------------------------------------------
> 
> (Updated Oct. 9, 2015, 1:40 a.m.)
> 
> 
> Review request for atlas, John Speidel and Shwetha GS.
> 
> 
> Bugs: ATLAS-215
>     https://issues.apache.org/jira/browse/ATLAS-215
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> Currently with the notification framework we set the properties so that all consumers are created in a single group...
> 
>         //todo take group id as argument to allow multiple consumers??
>         properties.put(ConsumerConfig.GROUP_ID_CONFIG, ATLAS_GROUP);
> 
> Having Kafka consumers for both the HOOK and ENTITIES topics in a single group causes this issue ...
> 
>     java.lang.IllegalArgumentException: requirement failed: Round-robin assignment is allowed only if all consumers in the group subscribe to the same topics, AND if the stream counts across topics are identical for a given consumer instance.
>     Topic ATLAS_ENTITIES has the following available consumer streams: Set(atlas_c6401.ambari.apache.org-1443714948705-e518b690-0)
>     Topic ATLAS_HOOK has the following available consumer streams: Set(atlas_c6401.ambari.apache.org-1443714214755-8fd9a5f8-0)
> 
> We shouldn't have HOOK consumers and ENTITIES consumers in the same group.
> 
> 
> Diffs
> -----
> 
>   notification/pom.xml 2e12520 
>   notification/src/main/java/org/apache/atlas/kafka/KafkaNotification.java 7b3cf89 
>   notification/src/main/java/org/apache/atlas/notification/NotificationHookConsumer.java 29194a4 
>   notification/src/main/java/org/apache/atlas/notification/NotificationInterface.java d4be07b 
>   notification/src/test/java/org/apache/atlas/kafka/KafkaNotificationTest.java 735655c 
> 
> Diff: https://reviews.apache.org/r/39157/diff/
> 
> 
> Testing
> -------
> 
> new unit test added
> 
> mvn clean test
> 
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 12.246 s
> [INFO] Finished at: 2015-10-08T21:37:55-04:00
> [INFO] Final Memory: 40M/605M
> [INFO] ------------------------------------------------------------------------
> 
> 
> Thanks,
> 
> Tom Beerbower
> 
>

Re: Review Request 39157: Configure Consumer Groups for Notifications

Posted by Suma Shivaprasad <su...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39157/#review102184
-----------------------------------------------------------

Ship it!


Ship It!

- Suma Shivaprasad


On Oct. 9, 2015, 1:40 a.m., Tom Beerbower wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39157/
> -----------------------------------------------------------
> 
> (Updated Oct. 9, 2015, 1:40 a.m.)
> 
> 
> Review request for atlas, John Speidel and Shwetha GS.
> 
> 
> Bugs: ATLAS-215
>     https://issues.apache.org/jira/browse/ATLAS-215
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> Currently with the notification framework we set the properties so that all consumers are created in a single group...
> 
>         //todo take group id as argument to allow multiple consumers??
>         properties.put(ConsumerConfig.GROUP_ID_CONFIG, ATLAS_GROUP);
> 
> Having Kafka consumers for both the HOOK and ENTITIES topics in a single group causes this issue ...
> 
>     java.lang.IllegalArgumentException: requirement failed: Round-robin assignment is allowed only if all consumers in the group subscribe to the same topics, AND if the stream counts across topics are identical for a given consumer instance.
>     Topic ATLAS_ENTITIES has the following available consumer streams: Set(atlas_c6401.ambari.apache.org-1443714948705-e518b690-0)
>     Topic ATLAS_HOOK has the following available consumer streams: Set(atlas_c6401.ambari.apache.org-1443714214755-8fd9a5f8-0)
> 
> We shouldn't have HOOK consumers and ENTITIES consumers in the same group.
> 
> 
> Diffs
> -----
> 
>   notification/pom.xml 2e12520 
>   notification/src/main/java/org/apache/atlas/kafka/KafkaNotification.java 7b3cf89 
>   notification/src/main/java/org/apache/atlas/notification/NotificationHookConsumer.java 29194a4 
>   notification/src/main/java/org/apache/atlas/notification/NotificationInterface.java d4be07b 
>   notification/src/test/java/org/apache/atlas/kafka/KafkaNotificationTest.java 735655c 
> 
> Diff: https://reviews.apache.org/r/39157/diff/
> 
> 
> Testing
> -------
> 
> new unit test added
> 
> mvn clean test
> 
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 12.246 s
> [INFO] Finished at: 2015-10-08T21:37:55-04:00
> [INFO] Final Memory: 40M/605M
> [INFO] ------------------------------------------------------------------------
> 
> 
> Thanks,
> 
> Tom Beerbower
> 
>