You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hudi.apache.org by GitBox <gi...@apache.org> on 2020/05/20 13:21:19 UTC

[GitHub] [incubator-hudi] pratyakshsharma opened a new pull request #1647: [HUDI-867]: fixed IllegalArgumentException from graphite metrics in deltaStreamer continuous mode

pratyakshsharma opened a new pull request #1647:
URL: https://github.com/apache/incubator-hudi/pull/1647


   
   ## *Tips*
   - *Thank you very much for contributing to Apache Hudi.*
   - *Please review https://hudi.apache.org/contributing.html before opening a pull request.*
   
   ## What is the purpose of the pull request
   
   Added a way to create metric names with updated table name in every iteration so that IllegalArgumentException does not comes up. 
   
   ## Brief change log
   
   *(for example:)*
     - *Modify AnnotationLocation checkstyle rule in checkstyle.xml*
   
   ## Verify this pull request
   
   *(Please pick either of the following options)*
   
   This pull request is a trivial rework / code cleanup without any test coverage.
   
   *(or)*
   
   This pull request is already covered by existing tests, such as *(please describe tests)*.
   
   (or)
   
   This change added tests and can be verified as follows:
   
   *(example:)*
   
     - *Added integration tests for end-to-end.*
     - *Added HoodieClientWriteTest to verify the change.*
     - *Manually verified the change by running a job locally.*
   
   ## Committer checklist
   
    - [ ] Has a corresponding JIRA in PR title & commit
    
    - [ ] Commit message is descriptive of the change
    
    - [ ] CI is green
   
    - [ ] Necessary doc changes done or have another open PR
          
    - [ ] For large changes, please consider breaking it into sub-tasks under an umbrella JIRA.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-hudi] pratyakshsharma commented on a change in pull request #1647: [HUDI-867]: fixed IllegalArgumentException from graphite metrics in deltaStreamer continuous mode

Posted by GitBox <gi...@apache.org>.
pratyakshsharma commented on a change in pull request #1647:
URL: https://github.com/apache/incubator-hudi/pull/1647#discussion_r429158430



##########
File path: hudi-utilities/src/main/java/org/apache/hudi/utilities/deltastreamer/HoodieDeltaStreamer.java
##########
@@ -416,10 +425,12 @@ public DeltaSync getDeltaSync() {
           jssc.setLocalProperty("spark.scheduler.pool", SchedulerConfGenerator.DELTASYNC_POOL_NAME);
         }
         try {
+          int iteration = 1;
           while (!isShutdownRequested()) {
             try {
               long start = System.currentTimeMillis();
-              Option<String> scheduledCompactionInstant = deltaSync.syncOnce();
+              HoodieMetrics.setTableName(cfg.metricsTableName + "_" + iteration);

Review comment:
       IllegalArgumentException is happening because the metrics name are generated in same way using tableName in each run. So we need some way of differentiating metrics names for every run and the easiest way to do that is altering the table name like "tableName_iteration". We need to do this change at 2 places, for HoodieDeltaStreamerMetrics and for HoodieMetrics. 
   The table name getting passed with syncOnce() method takes care of HoodieDeltaStreamerMetrics only. For all the other metrics, we need to reset table name in HoodieMetrics class. 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-hudi] codecov-commenter commented on pull request #1647: [HUDI-867]: fixed IllegalArgumentException from graphite metrics in deltaStreamer continuous mode

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #1647:
URL: https://github.com/apache/incubator-hudi/pull/1647#issuecomment-631511240


   # [Codecov](https://codecov.io/gh/apache/incubator-hudi/pull/1647?src=pr&el=h1) Report
   > Merging [#1647](https://codecov.io/gh/apache/incubator-hudi/pull/1647?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-hudi/commit/244d47494e2d4d5b3ca60e460e1feb9351fb8e69&el=desc) will **increase** coverage by `1.76%`.
   > The diff coverage is `0.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-hudi/pull/1647/graphs/tree.svg?width=650&height=150&src=pr&token=VTTXabwbs2)](https://codecov.io/gh/apache/incubator-hudi/pull/1647?src=pr&el=tree)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #1647      +/-   ##
   ============================================
   + Coverage     16.60%   18.37%   +1.76%     
   - Complexity      800      855      +55     
   ============================================
     Files           344      344              
     Lines         15172    15165       -7     
     Branches       1512     1512              
   ============================================
   + Hits           2520     2786     +266     
   + Misses        12320    12026     -294     
   - Partials        332      353      +21     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-hudi/pull/1647?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | [...java/org/apache/hudi/config/HoodieWriteConfig.java](https://codecov.io/gh/apache/incubator-hudi/pull/1647/diff?src=pr&el=tree#diff-aHVkaS1jbGllbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY29uZmlnL0hvb2RpZVdyaXRlQ29uZmlnLmphdmE=) | `43.91% <ø> (+1.65%)` | `48.00 <0.00> (ø)` | |
   | [...in/java/org/apache/hudi/metrics/HoodieMetrics.java](https://codecov.io/gh/apache/incubator-hudi/pull/1647/diff?src=pr&el=tree#diff-aHVkaS1jbGllbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvbWV0cmljcy9Ib29kaWVNZXRyaWNzLmphdmE=) | `18.86% <0.00%> (-0.37%)` | `6.00 <0.00> (ø)` | |
   | [...le/view/IncrementalTimelineSyncFileSystemView.java](https://codecov.io/gh/apache/incubator-hudi/pull/1647/diff?src=pr&el=tree#diff-aHVkaS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY29tbW9uL3RhYmxlL3ZpZXcvSW5jcmVtZW50YWxUaW1lbGluZVN5bmNGaWxlU3lzdGVtVmlldy5qYXZh) | `4.51% <0.00%> (+0.56%)` | `4.00% <0.00%> (+1.00%)` | |
   | [...common/table/view/AbstractTableFileSystemView.java](https://codecov.io/gh/apache/incubator-hudi/pull/1647/diff?src=pr&el=tree#diff-aHVkaS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY29tbW9uL3RhYmxlL3ZpZXcvQWJzdHJhY3RUYWJsZUZpbGVTeXN0ZW1WaWV3LmphdmE=) | `8.59% <0.00%> (+2.34%)` | `6.00% <0.00%> (+1.00%)` | |
   | [...i/common/table/timeline/HoodieDefaultTimeline.java](https://codecov.io/gh/apache/incubator-hudi/pull/1647/diff?src=pr&el=tree#diff-aHVkaS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY29tbW9uL3RhYmxlL3RpbWVsaW5lL0hvb2RpZURlZmF1bHRUaW1lbGluZS5qYXZh) | `52.30% <0.00%> (+4.61%)` | `28.00% <0.00%> (+4.00%)` | |
   | [.../main/java/org/apache/hudi/common/util/Option.java](https://codecov.io/gh/apache/incubator-hudi/pull/1647/diff?src=pr&el=tree#diff-aHVkaS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY29tbW9uL3V0aWwvT3B0aW9uLmphdmE=) | `51.35% <0.00%> (+5.40%)` | `18.00% <0.00%> (+1.00%)` | |
   | [...udi/timeline/service/handlers/BaseFileHandler.java](https://codecov.io/gh/apache/incubator-hudi/pull/1647/diff?src=pr&el=tree#diff-aHVkaS10aW1lbGluZS1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9odWRpL3RpbWVsaW5lL3NlcnZpY2UvaGFuZGxlcnMvQmFzZUZpbGVIYW5kbGVyLmphdmE=) | `11.11% <0.00%> (+11.11%)` | `1.00% <0.00%> (+1.00%)` | |
   | [...common/table/view/PriorityBasedFileSystemView.java](https://codecov.io/gh/apache/incubator-hudi/pull/1647/diff?src=pr&el=tree#diff-aHVkaS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY29tbW9uL3RhYmxlL3ZpZXcvUHJpb3JpdHlCYXNlZEZpbGVTeXN0ZW1WaWV3LmphdmE=) | `11.94% <0.00%> (+11.94%)` | `4.00% <0.00%> (+4.00%)` | |
   | [...i/common/table/view/HoodieTableFileSystemView.java](https://codecov.io/gh/apache/incubator-hudi/pull/1647/diff?src=pr&el=tree#diff-aHVkaS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY29tbW9uL3RhYmxlL3ZpZXcvSG9vZGllVGFibGVGaWxlU3lzdGVtVmlldy5qYXZh) | `37.93% <0.00%> (+13.79%)` | `9.00% <0.00%> (+2.00%)` | |
   | [...common/table/view/FileSystemViewStorageConfig.java](https://codecov.io/gh/apache/incubator-hudi/pull/1647/diff?src=pr&el=tree#diff-aHVkaS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY29tbW9uL3RhYmxlL3ZpZXcvRmlsZVN5c3RlbVZpZXdTdG9yYWdlQ29uZmlnLmphdmE=) | `63.49% <0.00%> (+14.28%)` | `8.00% <0.00%> (+3.00%)` | |
   | ... and [9 more](https://codecov.io/gh/apache/incubator-hudi/pull/1647/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-hudi/pull/1647?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-hudi/pull/1647?src=pr&el=footer). Last update [244d474...464aa36](https://codecov.io/gh/apache/incubator-hudi/pull/1647?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-hudi] pratyakshsharma commented on a change in pull request #1647: [HUDI-867]: fixed IllegalArgumentException from graphite metrics in deltaStreamer continuous mode

Posted by GitBox <gi...@apache.org>.
pratyakshsharma commented on a change in pull request #1647:
URL: https://github.com/apache/incubator-hudi/pull/1647#discussion_r429381950



##########
File path: hudi-utilities/src/main/java/org/apache/hudi/utilities/deltastreamer/HoodieDeltaStreamer.java
##########
@@ -416,10 +425,12 @@ public DeltaSync getDeltaSync() {
           jssc.setLocalProperty("spark.scheduler.pool", SchedulerConfGenerator.DELTASYNC_POOL_NAME);
         }
         try {
+          int iteration = 1;
           while (!isShutdownRequested()) {
             try {
               long start = System.currentTimeMillis();
-              Option<String> scheduledCompactionInstant = deltaSync.syncOnce();
+              HoodieMetrics.setTableName(cfg.metricsTableName + "_" + iteration);

Review comment:
       @leesf yes. This PR only fixes this for continuous mode of HoodieDeltaStreamer. If you can point me to relevant code of spark datasource from where this can be executed, I can try fixing there as well. 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-hudi] leesf commented on a change in pull request #1647: [HUDI-867]: fixed IllegalArgumentException from graphite metrics in deltaStreamer continuous mode

Posted by GitBox <gi...@apache.org>.
leesf commented on a change in pull request #1647:
URL: https://github.com/apache/incubator-hudi/pull/1647#discussion_r429542752



##########
File path: hudi-utilities/src/main/java/org/apache/hudi/utilities/deltastreamer/HoodieDeltaStreamer.java
##########
@@ -416,10 +425,12 @@ public DeltaSync getDeltaSync() {
           jssc.setLocalProperty("spark.scheduler.pool", SchedulerConfGenerator.DELTASYNC_POOL_NAME);
         }
         try {
+          int iteration = 1;
           while (!isShutdownRequested()) {
             try {
               long start = System.currentTimeMillis();
-              Option<String> scheduledCompactionInstant = deltaSync.syncOnce();
+              HoodieMetrics.setTableName(cfg.metricsTableName + "_" + iteration);

Review comment:
       @pratyakshsharma just a raw idea, how about adding a static variable (iterator) in HoodieMetrics, and  ++iterator after `updateCommitMetrics`. And back to the PR, if we only fix the HoodieDeltaStreamer, I think we would simply pass the iteration_time to `syncOnce` method to create a new metric name instead of adding a static tableName to HoodieMetrics, I think it is a little bit weird.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-hudi] pratyakshsharma commented on a change in pull request #1647: [HUDI-867]: fixed IllegalArgumentException from graphite metrics in deltaStreamer continuous mode

Posted by GitBox <gi...@apache.org>.
pratyakshsharma commented on a change in pull request #1647:
URL: https://github.com/apache/incubator-hudi/pull/1647#discussion_r429158430



##########
File path: hudi-utilities/src/main/java/org/apache/hudi/utilities/deltastreamer/HoodieDeltaStreamer.java
##########
@@ -416,10 +425,12 @@ public DeltaSync getDeltaSync() {
           jssc.setLocalProperty("spark.scheduler.pool", SchedulerConfGenerator.DELTASYNC_POOL_NAME);
         }
         try {
+          int iteration = 1;
           while (!isShutdownRequested()) {
             try {
               long start = System.currentTimeMillis();
-              Option<String> scheduledCompactionInstant = deltaSync.syncOnce();
+              HoodieMetrics.setTableName(cfg.metricsTableName + "_" + iteration);

Review comment:
       IllegalArgumentException is happening because the metrics name are generated in same way using tableName in each run. So we need some way of differentiating metrics names for every run and the easiest way to do that is altering the table name like <tableName>_<iteration>. We need to do this change at 2 places, for HoodieDeltaStreamerMetrics and for HoodieMetrics. 
   The table name getting passed with syncOnce() method takes care of HoodieDeltaStreamerMetrics only. For all the other metrics, we need to reset table name in HoodieMetrics class. 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-hudi] pratyakshsharma commented on a change in pull request #1647: [HUDI-867]: fixed IllegalArgumentException from graphite metrics in deltaStreamer continuous mode

Posted by GitBox <gi...@apache.org>.
pratyakshsharma commented on a change in pull request #1647:
URL: https://github.com/apache/incubator-hudi/pull/1647#discussion_r429158430



##########
File path: hudi-utilities/src/main/java/org/apache/hudi/utilities/deltastreamer/HoodieDeltaStreamer.java
##########
@@ -416,10 +425,12 @@ public DeltaSync getDeltaSync() {
           jssc.setLocalProperty("spark.scheduler.pool", SchedulerConfGenerator.DELTASYNC_POOL_NAME);
         }
         try {
+          int iteration = 1;
           while (!isShutdownRequested()) {
             try {
               long start = System.currentTimeMillis();
-              Option<String> scheduledCompactionInstant = deltaSync.syncOnce();
+              HoodieMetrics.setTableName(cfg.metricsTableName + "_" + iteration);

Review comment:
       IllegalArgumentException is happening because the metrics name are generated in same way using tableName in each run. So we need some way of differentiating metrics names for every run and the easiest way to do that is altering the table name like "<tableName>_<iteration>". We need to do this change at 2 places, for HoodieDeltaStreamerMetrics and for HoodieMetrics. 
   The table name getting passed with syncOnce() method takes care of HoodieDeltaStreamerMetrics only. For all the other metrics, we need to reset table name in HoodieMetrics class. 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-hudi] codecov-commenter edited a comment on pull request #1647: [HUDI-867]: fixed IllegalArgumentException from graphite metrics in deltaStreamer continuous mode

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #1647:
URL: https://github.com/apache/incubator-hudi/pull/1647#issuecomment-631511240


   # [Codecov](https://codecov.io/gh/apache/incubator-hudi/pull/1647?src=pr&el=h1) Report
   > Merging [#1647](https://codecov.io/gh/apache/incubator-hudi/pull/1647?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-hudi/commit/244d47494e2d4d5b3ca60e460e1feb9351fb8e69&el=desc) will **increase** coverage by `1.76%`.
   > The diff coverage is `0.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-hudi/pull/1647/graphs/tree.svg?width=650&height=150&src=pr&token=VTTXabwbs2)](https://codecov.io/gh/apache/incubator-hudi/pull/1647?src=pr&el=tree)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #1647      +/-   ##
   ============================================
   + Coverage     16.60%   18.37%   +1.76%     
   - Complexity      800      855      +55     
   ============================================
     Files           344      344              
     Lines         15172    15165       -7     
     Branches       1512     1512              
   ============================================
   + Hits           2520     2786     +266     
   + Misses        12320    12026     -294     
   - Partials        332      353      +21     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-hudi/pull/1647?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | [...java/org/apache/hudi/config/HoodieWriteConfig.java](https://codecov.io/gh/apache/incubator-hudi/pull/1647/diff?src=pr&el=tree#diff-aHVkaS1jbGllbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY29uZmlnL0hvb2RpZVdyaXRlQ29uZmlnLmphdmE=) | `43.91% <ø> (+1.65%)` | `48.00 <0.00> (ø)` | |
   | [...in/java/org/apache/hudi/metrics/HoodieMetrics.java](https://codecov.io/gh/apache/incubator-hudi/pull/1647/diff?src=pr&el=tree#diff-aHVkaS1jbGllbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvbWV0cmljcy9Ib29kaWVNZXRyaWNzLmphdmE=) | `18.86% <0.00%> (-0.37%)` | `6.00 <0.00> (ø)` | |
   | [...le/view/IncrementalTimelineSyncFileSystemView.java](https://codecov.io/gh/apache/incubator-hudi/pull/1647/diff?src=pr&el=tree#diff-aHVkaS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY29tbW9uL3RhYmxlL3ZpZXcvSW5jcmVtZW50YWxUaW1lbGluZVN5bmNGaWxlU3lzdGVtVmlldy5qYXZh) | `4.51% <0.00%> (+0.56%)` | `4.00% <0.00%> (+1.00%)` | |
   | [...common/table/view/AbstractTableFileSystemView.java](https://codecov.io/gh/apache/incubator-hudi/pull/1647/diff?src=pr&el=tree#diff-aHVkaS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY29tbW9uL3RhYmxlL3ZpZXcvQWJzdHJhY3RUYWJsZUZpbGVTeXN0ZW1WaWV3LmphdmE=) | `8.59% <0.00%> (+2.34%)` | `6.00% <0.00%> (+1.00%)` | |
   | [...i/common/table/timeline/HoodieDefaultTimeline.java](https://codecov.io/gh/apache/incubator-hudi/pull/1647/diff?src=pr&el=tree#diff-aHVkaS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY29tbW9uL3RhYmxlL3RpbWVsaW5lL0hvb2RpZURlZmF1bHRUaW1lbGluZS5qYXZh) | `52.30% <0.00%> (+4.61%)` | `28.00% <0.00%> (+4.00%)` | |
   | [.../main/java/org/apache/hudi/common/util/Option.java](https://codecov.io/gh/apache/incubator-hudi/pull/1647/diff?src=pr&el=tree#diff-aHVkaS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY29tbW9uL3V0aWwvT3B0aW9uLmphdmE=) | `51.35% <0.00%> (+5.40%)` | `18.00% <0.00%> (+1.00%)` | |
   | [...udi/timeline/service/handlers/BaseFileHandler.java](https://codecov.io/gh/apache/incubator-hudi/pull/1647/diff?src=pr&el=tree#diff-aHVkaS10aW1lbGluZS1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9odWRpL3RpbWVsaW5lL3NlcnZpY2UvaGFuZGxlcnMvQmFzZUZpbGVIYW5kbGVyLmphdmE=) | `11.11% <0.00%> (+11.11%)` | `1.00% <0.00%> (+1.00%)` | |
   | [...common/table/view/PriorityBasedFileSystemView.java](https://codecov.io/gh/apache/incubator-hudi/pull/1647/diff?src=pr&el=tree#diff-aHVkaS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY29tbW9uL3RhYmxlL3ZpZXcvUHJpb3JpdHlCYXNlZEZpbGVTeXN0ZW1WaWV3LmphdmE=) | `11.94% <0.00%> (+11.94%)` | `4.00% <0.00%> (+4.00%)` | |
   | [...i/common/table/view/HoodieTableFileSystemView.java](https://codecov.io/gh/apache/incubator-hudi/pull/1647/diff?src=pr&el=tree#diff-aHVkaS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY29tbW9uL3RhYmxlL3ZpZXcvSG9vZGllVGFibGVGaWxlU3lzdGVtVmlldy5qYXZh) | `37.93% <0.00%> (+13.79%)` | `9.00% <0.00%> (+2.00%)` | |
   | [...common/table/view/FileSystemViewStorageConfig.java](https://codecov.io/gh/apache/incubator-hudi/pull/1647/diff?src=pr&el=tree#diff-aHVkaS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY29tbW9uL3RhYmxlL3ZpZXcvRmlsZVN5c3RlbVZpZXdTdG9yYWdlQ29uZmlnLmphdmE=) | `63.49% <0.00%> (+14.28%)` | `8.00% <0.00%> (+3.00%)` | |
   | ... and [9 more](https://codecov.io/gh/apache/incubator-hudi/pull/1647/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-hudi/pull/1647?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-hudi/pull/1647?src=pr&el=footer). Last update [244d474...464aa36](https://codecov.io/gh/apache/incubator-hudi/pull/1647?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-hudi] nsivabalan commented on a change in pull request #1647: [HUDI-867]: fixed IllegalArgumentException from graphite metrics in deltaStreamer continuous mode

Posted by GitBox <gi...@apache.org>.
nsivabalan commented on a change in pull request #1647:
URL: https://github.com/apache/incubator-hudi/pull/1647#discussion_r428967198



##########
File path: hudi-utilities/src/main/java/org/apache/hudi/utilities/deltastreamer/HoodieDeltaStreamer.java
##########
@@ -416,10 +425,12 @@ public DeltaSync getDeltaSync() {
           jssc.setLocalProperty("spark.scheduler.pool", SchedulerConfGenerator.DELTASYNC_POOL_NAME);
         }
         try {
+          int iteration = 1;
           while (!isShutdownRequested()) {
             try {
               long start = System.currentTimeMillis();
-              Option<String> scheduledCompactionInstant = deltaSync.syncOnce();
+              HoodieMetrics.setTableName(cfg.metricsTableName + "_" + iteration);

Review comment:
       sorry, I don't quite get why we need to set table name here? Next line will in turn rely on the arg passed for tablename. So, don't really understand why we need static fix (i.e. setTableName). From the diff, I see that cfg.tableName is set passed into  DeltaSync.syncOnce(tblName) and HoodieDeltaStreamerMetrics(HoodieWriteConfig tableName). Can you help me understand the case where the static set method for table name is required. 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-hudi] leesf commented on a change in pull request #1647: [HUDI-867]: fixed IllegalArgumentException from graphite metrics in deltaStreamer continuous mode

Posted by GitBox <gi...@apache.org>.
leesf commented on a change in pull request #1647:
URL: https://github.com/apache/incubator-hudi/pull/1647#discussion_r429313527



##########
File path: hudi-utilities/src/main/java/org/apache/hudi/utilities/deltastreamer/HoodieDeltaStreamer.java
##########
@@ -416,10 +425,12 @@ public DeltaSync getDeltaSync() {
           jssc.setLocalProperty("spark.scheduler.pool", SchedulerConfGenerator.DELTASYNC_POOL_NAME);
         }
         try {
+          int iteration = 1;
           while (!isShutdownRequested()) {
             try {
               long start = System.currentTimeMillis();
-              Option<String> scheduledCompactionInstant = deltaSync.syncOnce();
+              HoodieMetrics.setTableName(cfg.metricsTableName + "_" + iteration);

Review comment:
       @pratyakshsharma Thanks for the fix, I have a small question, when using Spark Streaming writing to Hudi (which seems like the continuous mode of deltastreamer), the exception will happen again? 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org