You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@falcon.apache.org by Suma Shivaprasad <su...@gmail.com> on 2014/11/10 15:54:15 UTC

Re: Review Request 27029: FALCON-697 - Messaging improvements on Producer

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

(Updated Nov. 10, 2014, 2:54 p.m.)


Review request for Falcon, Ajay Yadava, shwethags, and Srikanth Sundarrajan.


Bugs: FALCON-697
    https://issues.apache.org/jira/browse/FALCON-697


Repository: falcon-git


Description
-------

In case of EVICTION and REPLICATION, generating one message per feed to both FALCON topic and user topic with all feed paths

In case of GENERATE, in system topic(FALCON.ENITITY.TOPIC) generating one message per process with all the feed paths. In User topic(FALCON Feedname/process), generating one message per feed for feed name topics and one message per process topic with all the feed paths


Diffs
-----

  messaging/src/main/java/org/apache/falcon/messaging/FalconJMSMessageProducer.java PRE-CREATION 
  messaging/src/main/java/org/apache/falcon/messaging/JMSMessageProducer.java 629e6a5 
  messaging/src/main/java/org/apache/falcon/messaging/UserJMSMessageProducer.java PRE-CREATION 
  messaging/src/test/java/org/apache/falcon/messaging/FeedProducerTest.java 2f5aa70 
  messaging/src/test/java/org/apache/falcon/messaging/JMSMessageProducerTest.java d4373de 
  messaging/src/test/java/org/apache/falcon/messaging/ProcessProducerTest.java 1cd3310 

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


Testing
-------

Made UT changes to current tests


Thanks,

Suma Shivaprasad


Re: Review Request 27029: FALCON-697 - Messaging improvements on Producer

Posted by sh...@inmobi.com.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27029/#review60762
-----------------------------------------------------------



messaging/src/main/java/org/apache/falcon/messaging/FalconJMSMessageProducer.java
<https://reviews.apache.org/r/27029/#comment102170>

    System message on falcon topic has 1 message. So, feedNames is list of output feed names and feedPaths is list of corresponding output paths. This breaks for retention as feedNames will contain just 1 name and feedPaths will contain list of paths. We need to take care of this



messaging/src/main/java/org/apache/falcon/messaging/FalconJMSMessageProducer.java
<https://reviews.apache.org/r/27029/#comment102171>

    Also, at retention we delete even stats and meta paths of the feed(in addition to data paths) and all these paths are returned in feedPaths. The message should contain just the data paths. We can handle it in another jira as well



messaging/src/main/java/org/apache/falcon/messaging/JMSMessageProducer.java
<https://reviews.apache.org/r/27029/#comment102167>

    Move getBrokerImplClass () and getBrokerUrl () to concrete class?



messaging/src/main/java/org/apache/falcon/messaging/UserJMSMessageProducer.java
<https://reviews.apache.org/r/27029/#comment102169>

    The function shouldn't take feedNames as argument(If there are multiple outputs for the same feed). Isn't feedNamesVsPaths.keySet() enough?


- shwethags


On Nov. 10, 2014, 2:54 p.m., Suma Shivaprasad wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27029/
> -----------------------------------------------------------
> 
> (Updated Nov. 10, 2014, 2:54 p.m.)
> 
> 
> Review request for Falcon, Ajay Yadava, shwethags, and Srikanth Sundarrajan.
> 
> 
> Bugs: FALCON-697
>     https://issues.apache.org/jira/browse/FALCON-697
> 
> 
> Repository: falcon-git
> 
> 
> Description
> -------
> 
> In case of EVICTION and REPLICATION, generating one message per feed to both FALCON topic and user topic with all feed paths
> 
> In case of GENERATE, in system topic(FALCON.ENITITY.TOPIC) generating one message per process with all the feed paths. In User topic(FALCON Feedname/process), generating one message per feed for feed name topics and one message per process topic with all the feed paths
> 
> 
> Diffs
> -----
> 
>   messaging/src/main/java/org/apache/falcon/messaging/FalconJMSMessageProducer.java PRE-CREATION 
>   messaging/src/main/java/org/apache/falcon/messaging/JMSMessageProducer.java 629e6a5 
>   messaging/src/main/java/org/apache/falcon/messaging/UserJMSMessageProducer.java PRE-CREATION 
>   messaging/src/test/java/org/apache/falcon/messaging/FeedProducerTest.java 2f5aa70 
>   messaging/src/test/java/org/apache/falcon/messaging/JMSMessageProducerTest.java d4373de 
>   messaging/src/test/java/org/apache/falcon/messaging/ProcessProducerTest.java 1cd3310 
> 
> Diff: https://reviews.apache.org/r/27029/diff/
> 
> 
> Testing
> -------
> 
> Made UT changes to current tests
> 
> 
> Thanks,
> 
> Suma Shivaprasad
> 
>


Re: Review Request 27029: FALCON-697 - Messaging improvements on Producer

Posted by Ajay Yadava <aj...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27029/#review60729
-----------------------------------------------------------



messaging/src/main/java/org/apache/falcon/messaging/UserJMSMessageProducer.java
<https://reviews.apache.org/r/27029/#comment102121>

    Documenting an example of how a message for feed/process looks like will help readers to understand what's going on.



messaging/src/main/java/org/apache/falcon/messaging/UserJMSMessageProducer.java
<https://reviews.apache.org/r/27029/#comment102120>

    It is probably better for the function to return a map instead of receiving one and modifying it as it will reduce coupling between the two methods and improve testability.



messaging/src/main/java/org/apache/falcon/messaging/UserJMSMessageProducer.java
<https://reviews.apache.org/r/27029/#comment102117>

    I think these lines can be removed.



messaging/src/test/java/org/apache/falcon/messaging/JMSMessageProducerTest.java
<https://reviews.apache.org/r/27029/#comment102115>

    Use logger.



messaging/src/test/java/org/apache/falcon/messaging/ProcessProducerTest.java
<https://reviews.apache.org/r/27029/#comment102116>

    Use Logger please.


- Ajay Yadava


On Nov. 10, 2014, 2:54 p.m., Suma Shivaprasad wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27029/
> -----------------------------------------------------------
> 
> (Updated Nov. 10, 2014, 2:54 p.m.)
> 
> 
> Review request for Falcon, Ajay Yadava, shwethags, and Srikanth Sundarrajan.
> 
> 
> Bugs: FALCON-697
>     https://issues.apache.org/jira/browse/FALCON-697
> 
> 
> Repository: falcon-git
> 
> 
> Description
> -------
> 
> In case of EVICTION and REPLICATION, generating one message per feed to both FALCON topic and user topic with all feed paths
> 
> In case of GENERATE, in system topic(FALCON.ENITITY.TOPIC) generating one message per process with all the feed paths. In User topic(FALCON Feedname/process), generating one message per feed for feed name topics and one message per process topic with all the feed paths
> 
> 
> Diffs
> -----
> 
>   messaging/src/main/java/org/apache/falcon/messaging/FalconJMSMessageProducer.java PRE-CREATION 
>   messaging/src/main/java/org/apache/falcon/messaging/JMSMessageProducer.java 629e6a5 
>   messaging/src/main/java/org/apache/falcon/messaging/UserJMSMessageProducer.java PRE-CREATION 
>   messaging/src/test/java/org/apache/falcon/messaging/FeedProducerTest.java 2f5aa70 
>   messaging/src/test/java/org/apache/falcon/messaging/JMSMessageProducerTest.java d4373de 
>   messaging/src/test/java/org/apache/falcon/messaging/ProcessProducerTest.java 1cd3310 
> 
> Diff: https://reviews.apache.org/r/27029/diff/
> 
> 
> Testing
> -------
> 
> Made UT changes to current tests
> 
> 
> Thanks,
> 
> Suma Shivaprasad
> 
>