You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@atlas.apache.org by Saqeeb Shaikh <sa...@freestoneinfotech.com> on 2019/07/19 20:48:59 UTC

Review Request 71129: [ATLAS-3344]-Atlas Metrics to include topic partition wise additional information

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

Review request for atlas, Ashutosh Mestry, Madhan Neethiraj, Nixon Rodrigues, and Sarath Subramanian.


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


Repository: atlas


Description
-------

As of now Atlas metrics includes partition wise current offset and start offset of Kafka topics. Include additional information like last message processed time and failed message count.


Diffs
-----

  intg/src/main/java/org/apache/atlas/model/metrics/AtlasMetrics.java a48d93b 
  repository/src/main/java/org/apache/atlas/util/AtlasMetricsUtil.java 2c78cbc 
  repository/src/test/java/org/apache/atlas/services/MetricsServiceTest.java b2f2633 
  webapp/src/main/java/org/apache/atlas/notification/NotificationHookConsumer.java f7df6b3 


Diff: https://reviews.apache.org/r/71129/diff/1/


Testing
-------

Tested mvn clean install package.
Some integration test cases which are not related to this patch are failing on my local, not sure why. I'll look into those tests and fix them.

Tested on cluster 172.22.88.47.


Thanks,

Saqeeb Shaikh


Re: Review Request 71129: [ATLAS-3344]-Atlas Metrics to include topic partition wise additional information

Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71129/#review216776
-----------------------------------------------------------




repository/src/main/java/org/apache/atlas/util/AtlasMetricsUtil.java
Lines 113 (patched)
<https://reviews.apache.org/r/71129/#comment304008>

    replace line #113 with:
      if (stats.isFailedMsg) {
        partitionStat.incrFailedMessageCount(); 
      }



repository/src/main/java/org/apache/atlas/util/AtlasMetricsUtil.java
Lines 114 (patched)
<https://reviews.apache.org/r/71129/#comment304009>

    lastMessageProcessedTime parameter is not needed. Please replace line #114 with:
      partitionStat.setLastMessageProcessedTime(messagesProcessed.getLastIncrTime().toEpochMilli());
      
    With this, changes in NotificationHookConsumer.java and MetricsServiceTest.java can be reverted.


- Madhan Neethiraj


On July 19, 2019, 8:48 p.m., Saqeeb Shaikh wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71129/
> -----------------------------------------------------------
> 
> (Updated July 19, 2019, 8:48 p.m.)
> 
> 
> Review request for atlas, Ashutosh Mestry, Madhan Neethiraj, Nixon Rodrigues, and Sarath Subramanian.
> 
> 
> Bugs: ATLAS-3344
>     https://issues.apache.org/jira/browse/ATLAS-3344
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> As of now Atlas metrics includes partition wise current offset and start offset of Kafka topics. Include additional information like last message processed time and failed message count.
> 
> 
> Diffs
> -----
> 
>   intg/src/main/java/org/apache/atlas/model/metrics/AtlasMetrics.java a48d93b 
>   repository/src/main/java/org/apache/atlas/util/AtlasMetricsUtil.java 2c78cbc 
>   repository/src/test/java/org/apache/atlas/services/MetricsServiceTest.java b2f2633 
>   webapp/src/main/java/org/apache/atlas/notification/NotificationHookConsumer.java f7df6b3 
> 
> 
> Diff: https://reviews.apache.org/r/71129/diff/1/
> 
> 
> Testing
> -------
> 
> Tested mvn clean install package.
> Some integration test cases which are not related to this patch are failing on my local, not sure why. I'll look into those tests and fix them.
> 
> Tested on cluster 172.22.88.47.
> 
> 
> Thanks,
> 
> Saqeeb Shaikh
> 
>


Re: Review Request 71129: [ATLAS-3344]-Atlas Metrics to include topic partition wise additional information

Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71129/#review216788
-----------------------------------------------------------


Fix it, then Ship it!





repository/src/main/java/org/apache/atlas/util/AtlasMetricsUtil.java
Lines 115 (patched)
<https://reviews.apache.org/r/71129/#comment304013>

    processedMessageCount should be incremented irrespective of success or not - similar to overall count at line #86 above.


- Madhan Neethiraj


On July 22, 2019, 12:35 p.m., Saqeeb Shaikh wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71129/
> -----------------------------------------------------------
> 
> (Updated July 22, 2019, 12:35 p.m.)
> 
> 
> Review request for atlas, Ashutosh Mestry, Madhan Neethiraj, Nixon Rodrigues, and Sarath Subramanian.
> 
> 
> Bugs: ATLAS-3344
>     https://issues.apache.org/jira/browse/ATLAS-3344
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> As of now Atlas metrics includes partition wise current offset and start offset of Kafka topics. Include additional information like last message processed time and failed message count.
> 
> 
> Diffs
> -----
> 
>   dashboardv2/public/js/templates/site/Statistics_Topic_Offset_table_tmpl.html 4693edf 
>   dashboardv2/public/js/templates/site/Statistics_tmpl.html ecd84d6 
>   dashboardv2/public/js/utils/Enums.js e38af74 
>   dashboardv2/public/js/views/site/Statistics.js a8e351e 
>   intg/src/main/java/org/apache/atlas/model/metrics/AtlasMetrics.java a48d93b 
>   repository/src/main/java/org/apache/atlas/util/AtlasMetricsUtil.java 2c78cbc 
> 
> 
> Diff: https://reviews.apache.org/r/71129/diff/2/
> 
> 
> Testing
> -------
> 
> Tested mvn clean install package.
> Some integration test cases which are not related to this patch are failing on my local, not sure why. I'll look into those tests and fix them.
> 
> Tested on cluster 172.22.88.47.
> 
> 
> Thanks,
> 
> Saqeeb Shaikh
> 
>


Re: Review Request 71129: [ATLAS-3344]-Atlas Metrics to include topic partition wise additional information

Posted by Nixon Rodrigues <ni...@freestoneinfotech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71129/#review216854
-----------------------------------------------------------


Ship it!




Ship It!

- Nixon Rodrigues


On July 23, 2019, 8:59 a.m., Saqeeb Shaikh wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71129/
> -----------------------------------------------------------
> 
> (Updated July 23, 2019, 8:59 a.m.)
> 
> 
> Review request for atlas, Ashutosh Mestry, Madhan Neethiraj, Nixon Rodrigues, and Sarath Subramanian.
> 
> 
> Bugs: ATLAS-3344
>     https://issues.apache.org/jira/browse/ATLAS-3344
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> As of now Atlas metrics includes partition wise current offset and start offset of Kafka topics. Include additional information like last message processed time and failed message count.
> 
> 
> Diffs
> -----
> 
>   dashboardv2/public/js/templates/site/Statistics_Topic_Offset_table_tmpl.html 4693edf 
>   dashboardv2/public/js/templates/site/Statistics_tmpl.html ecd84d6 
>   dashboardv2/public/js/utils/Enums.js e38af74 
>   dashboardv2/public/js/views/site/Statistics.js a8e351e 
>   intg/src/main/java/org/apache/atlas/model/metrics/AtlasMetrics.java a48d93b 
>   repository/src/main/java/org/apache/atlas/util/AtlasMetricsUtil.java 2c78cbc 
> 
> 
> Diff: https://reviews.apache.org/r/71129/diff/3/
> 
> 
> Testing
> -------
> 
> Tested mvn clean install package.
> Some integration test cases which are not related to this patch are failing on my local, not sure why. I'll look into those tests and fix them.
> 
> Tested on cluster 172.22.88.47.
> 
> 
> Thanks,
> 
> Saqeeb Shaikh
> 
>


Re: Review Request 71129: [ATLAS-3344]-Atlas Metrics to include topic partition wise additional information

Posted by Saqeeb Shaikh <sa...@freestoneinfotech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71129/
-----------------------------------------------------------

(Updated July 23, 2019, 8:59 a.m.)


Review request for atlas, Ashutosh Mestry, Madhan Neethiraj, Nixon Rodrigues, and Sarath Subramanian.


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


Repository: atlas


Description
-------

As of now Atlas metrics includes partition wise current offset and start offset of Kafka topics. Include additional information like last message processed time and failed message count.


Diffs (updated)
-----

  dashboardv2/public/js/templates/site/Statistics_Topic_Offset_table_tmpl.html 4693edf 
  dashboardv2/public/js/templates/site/Statistics_tmpl.html ecd84d6 
  dashboardv2/public/js/utils/Enums.js e38af74 
  dashboardv2/public/js/views/site/Statistics.js a8e351e 
  intg/src/main/java/org/apache/atlas/model/metrics/AtlasMetrics.java a48d93b 
  repository/src/main/java/org/apache/atlas/util/AtlasMetricsUtil.java 2c78cbc 


Diff: https://reviews.apache.org/r/71129/diff/3/

Changes: https://reviews.apache.org/r/71129/diff/2-3/


Testing
-------

Tested mvn clean install package.
Some integration test cases which are not related to this patch are failing on my local, not sure why. I'll look into those tests and fix them.

Tested on cluster 172.22.88.47.


Thanks,

Saqeeb Shaikh


Re: Review Request 71129: [ATLAS-3344]-Atlas Metrics to include topic partition wise additional information

Posted by Saqeeb Shaikh <sa...@freestoneinfotech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71129/
-----------------------------------------------------------

(Updated July 22, 2019, 12:35 p.m.)


Review request for atlas, Ashutosh Mestry, Madhan Neethiraj, Nixon Rodrigues, and Sarath Subramanian.


Changes
-------

Addressed review comments.
Added UI side changes.
Introduced processed message count.


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


Repository: atlas


Description
-------

As of now Atlas metrics includes partition wise current offset and start offset of Kafka topics. Include additional information like last message processed time and failed message count.


Diffs (updated)
-----

  dashboardv2/public/js/templates/site/Statistics_Topic_Offset_table_tmpl.html 4693edf 
  dashboardv2/public/js/templates/site/Statistics_tmpl.html ecd84d6 
  dashboardv2/public/js/utils/Enums.js e38af74 
  dashboardv2/public/js/views/site/Statistics.js a8e351e 
  intg/src/main/java/org/apache/atlas/model/metrics/AtlasMetrics.java a48d93b 
  repository/src/main/java/org/apache/atlas/util/AtlasMetricsUtil.java 2c78cbc 


Diff: https://reviews.apache.org/r/71129/diff/2/

Changes: https://reviews.apache.org/r/71129/diff/1-2/


Testing
-------

Tested mvn clean install package.
Some integration test cases which are not related to this patch are failing on my local, not sure why. I'll look into those tests and fix them.

Tested on cluster 172.22.88.47.


Thanks,

Saqeeb Shaikh