You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@gobblin.apache.org by "linweihs (via GitHub)" <gi...@apache.org> on 2024/07/17 22:11:40 UTC

[PR] [GOBBLIN-2114] Add fields to show computed delta between consume and … [gobblin]

linweihs opened a new pull request, #4005:
URL: https://github.com/apache/gobblin/pull/4005

   …producer/append time respectively
   
   Dear Gobblin maintainers,
   
   Please accept this PR. I understand that it will not be reviewed until I have checked off all the steps below!
   
   ### JIRA
   - [ ] My PR addresses the following [Gobblin JIRA](https://issues.apache.org/jira/browse/GOBBLIN/) issues and references them in the PR title. For example, "[GOBBLIN-XXX] My Gobblin PR"
       - https://issues.apache.org/jira/browse/GOBBLIN-2114
   
   
   ### Description
   - [ ] Here are some details about my PR, including screenshots (if applicable):
   Add fields to show computed delta between consume and producer/append time respectively
   
   This is the follow up https://github.com/apache/gobblin/pull/3980 to add more metrics for analysis purpose when switching to creationTimestamp
   
   ### Tests
   - [ ] My PR adds the following unit tests __OR__ does not need testing for this extremely good reason:
   Printed out the events and see the timestamp get set correctly.
   
   
   ### Commits
   - [ ] My commits all reference JIRA issues in their subject lines, and I have squashed multiple commits if they address the same issue. In addition, my commits follow the guidelines from "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)":
       1. Subject is separated from body by a blank line
       2. Subject is limited to 50 characters
       3. Subject does not end with a period
       4. Subject uses the imperative mood ("add", not "adding")
       5. Body wraps at 72 characters
       6. Body explains "what" and "why", not "how"
   
   


-- 
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.

To unsubscribe, e-mail: dev-unsubscribe@gobblin.apache.org

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


Re: [PR] [GOBBLIN-2114] Add fields to show computed delta between consume and … [gobblin]

Posted by "AndyJiang99 (via GitHub)" <gi...@apache.org>.
AndyJiang99 commented on code in PR #4005:
URL: https://github.com/apache/gobblin/pull/4005#discussion_r1683347509


##########
gobblin-modules/gobblin-kafka-common/src/main/java/org/apache/gobblin/source/extractor/extract/kafka/KafkaExtractorStatsTracker.java:
##########
@@ -306,6 +310,13 @@ public void onDecodeableRecord(int partitionIdx, long readStartTime, long decode
         if (logAppendTimestamp > 0 && (System.currentTimeMillis() - logAppendTimestamp > recordLevelSlaMillis)) {
           v.slaMissedRecordCount++;
         }
+        // See context in GOBBLIN-2114

Review Comment:
   Can you add the short description here instead of referencing the Gobblin ticket? Since we have so many stats tracker here, it would be good to have a short description so we know what we are tracking and going forward, we know what we already have.
   The naming for the variables might be more descriptive/understandable maybe as appendDurationTime and creationDurationTime to describe that it is amount of time taken between HDFS and when it was produced / appended. What do you think?



-- 
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.

To unsubscribe, e-mail: dev-unsubscribe@gobblin.apache.org

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


Re: [PR] [GOBBLIN-2114] Add fields to show computed delta between consume and … [gobblin]

Posted by "linweihs (via GitHub)" <gi...@apache.org>.
linweihs commented on code in PR #4005:
URL: https://github.com/apache/gobblin/pull/4005#discussion_r1683520845


##########
gobblin-modules/gobblin-kafka-common/src/main/java/org/apache/gobblin/source/extractor/extract/kafka/KafkaExtractorStatsTracker.java:
##########
@@ -306,6 +310,13 @@ public void onDecodeableRecord(int partitionIdx, long readStartTime, long decode
         if (logAppendTimestamp > 0 && (System.currentTimeMillis() - logAppendTimestamp > recordLevelSlaMillis)) {
           v.slaMissedRecordCount++;
         }
+        // See context in GOBBLIN-2114

Review Comment:
   comment addressed



-- 
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.

To unsubscribe, e-mail: dev-unsubscribe@gobblin.apache.org

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


Re: [PR] [GOBBLIN-2114] Add fields to show computed delta between consume and … [gobblin]

Posted by "Will-Lo (via GitHub)" <gi...@apache.org>.
Will-Lo merged PR #4005:
URL: https://github.com/apache/gobblin/pull/4005


-- 
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.

To unsubscribe, e-mail: dev-unsubscribe@gobblin.apache.org

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