You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2022/05/06 13:13:10 UTC

[GitHub] [flink] JingGe opened a new pull request, #19664: [hotfix][doc] add content for using numRecordsSend and deprecating numRecordsOutErrors

JingGe opened a new pull request, #19664:
URL: https://github.com/apache/flink/pull/19664

   ## What is the purpose of the change
   
   This PR will let Flink be aware of changes of metrics e.g. `numRecordsSend`, `numRecordsOutErrors`, etc.
   
   
   ## Brief change log
   
     - add the description of using `numRecordsSend` and deprecating `numRecordsOutErrors`
   
   GitHub PR
   
   ## Does this pull request potentially affect one of the following parts:
   
     - Dependencies (does it add or upgrade a dependency): (yes / **no**)
     - The public API, i.e., is any changed class annotated with `@Public(Evolving)`: (yes / **no**)
     - The serializers: (yes / **no** / don't know)
     - The runtime per-record code paths (performance sensitive): (yes / **no** / don't know)
     - Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn, ZooKeeper: (yes / **no** / don't know)
     - The S3 file system connector: (yes / **no** / don't know)
   
   ## Documentation
   
     - Does this pull request introduce a new feature? (yes / **no**)
   


-- 
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: issues-unsubscribe@flink.apache.org

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


[GitHub] [flink] fapaul commented on a diff in pull request #19664: [hotfix][doc] add content for using numRecordsSend and deprecating numRecordsOutErrors

Posted by GitBox <gi...@apache.org>.
fapaul commented on code in PR #19664:
URL: https://github.com/apache/flink/pull/19664#discussion_r866877842


##########
docs/content/release-notes/flink-1.15.md:
##########
@@ -406,6 +406,13 @@ updating the client dependency to a version >= 7.14.0 is required due to interna
 The old JDBC connector (indicated by `connector.type=jdbc` in DDL) has been removed.
 If not done already, users need to upgrade to the newer stack (indicated by `connector=jdbc` in DDL).
 
+#### [FLINK-26126](https://issues.apache.org/jira/browse/FLINK-26126)
+
+New metric `numRecordsSend` has been introduced to denote the number of records sent to the external system 
+while the numRecordsOut will be used to denote the number of records transferred between tasks.

Review Comment:
   ```suggestion
   while the numRecordsOut will be used to denote the number of records transferred between sink tasks.
   ```



##########
docs/content/release-notes/flink-1.15.md:
##########
@@ -406,6 +406,13 @@ updating the client dependency to a version >= 7.14.0 is required due to interna
 The old JDBC connector (indicated by `connector.type=jdbc` in DDL) has been removed.
 If not done already, users need to upgrade to the newer stack (indicated by `connector=jdbc` in DDL).
 
+#### [FLINK-26126](https://issues.apache.org/jira/browse/FLINK-26126)

Review Comment:
   I think it would be good to add a separate headline as "Extensible unified Sink uses new metric to capture outgoing records".



##########
docs/content/release-notes/flink-1.15.md:
##########
@@ -406,6 +406,13 @@ updating the client dependency to a version >= 7.14.0 is required due to interna
 The old JDBC connector (indicated by `connector.type=jdbc` in DDL) has been removed.
 If not done already, users need to upgrade to the newer stack (indicated by `connector=jdbc` in DDL).
 
+#### [FLINK-26126](https://issues.apache.org/jira/browse/FLINK-26126)
+
+New metric `numRecordsSend` has been introduced to denote the number of records sent to the external system 
+while the numRecordsOut will be used to denote the number of records transferred between tasks.
+
+Considering naming consistency, `numRecordsOutErrors` is deprecated, please use `numRecordsSendErrors` instead.

Review Comment:
   WDYT about adding two different paragraphs? The first describes the perspective of a connector user that now needs to use `numRecordsSend` `numRecordsSendErrors` to monitor the records going to the external system.
   The second paragraph holds information for connector developers that have to eventually update their connectors to use the new metrics because `numRecordsOurErrors` is deprecated and `numRecordsOut` has a different meaning. 



-- 
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: issues-unsubscribe@flink.apache.org

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


[GitHub] [flink] JingGe commented on a diff in pull request #19664: [hotfix][doc] add content for using numRecordsSend and deprecating numRecordsOutErrors

Posted by GitBox <gi...@apache.org>.
JingGe commented on code in PR #19664:
URL: https://github.com/apache/flink/pull/19664#discussion_r868138403


##########
docs/content/release-notes/flink-1.15.md:
##########
@@ -406,6 +406,18 @@ updating the client dependency to a version >= 7.14.0 is required due to interna
 The old JDBC connector (indicated by `connector.type=jdbc` in DDL) has been removed.
 If not done already, users need to upgrade to the newer stack (indicated by `connector=jdbc` in DDL).
 
+
+#### Extensible unified Sink uses new metric to capture outgoing records
+
+##### [FLINK-26126](https://issues.apache.org/jira/browse/FLINK-26126)
+
+New metrics `numRecordsSend` and `numRecordsSendErrors` has been introduced for users to monitor the number of 
+records sent to the external system. The `numRecordsOut` should be used to monitor the number of records 
+transferred between sink tasks.
+
+Connector developers should Consider using the new metric `numRecordsSendErrors` to monitor errors, 
+since `numRecordsOut` has a different and more general meaning, and `numRecordsOutErrors` is therefore deprecated.

Review Comment:
   This is confused for users and connector developers, tbh.  1. `numRecordsOut` will ONLY count the records(technically Commitables) sent between sink tasks. 2. Connectors still use `numRecordsOut`. 3. the information of using `numRecordsSend` has been described in the first part and there is no use case for "changing the connector"(all connectors that used `numRecordsOut` in a wrong way have been changed). From now on, it is actually mandatory for connector developers to use them except `numRecordsOutErrors`. 
   
   How about this version:
   
   Connector developers should pay attention to the usage of these metrics `numRecordsOut`, `numRecordsSend` and `numRecordsSendErrors` while building sink connectors. Please refer to the new Kafka Sink for details. Additionally since `numRecordsOut` now only counts the records sent between sink tasks and `numRecordsOutErrors` was designed for counting the records sent to the external system, we deprecated `numRecordsOutErrors` and recommend using `numRecordsSendErrors` instead.
   
   Event with this description, it is still unclear, since `numRecordsOut` is still used, why is `numRecordsOutErrors` deprecated? It should be used to monitor errors happened while Commitables were send between sink tasks. WDYT?
   
   
   



-- 
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: issues-unsubscribe@flink.apache.org

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


[GitHub] [flink] JingGe commented on a diff in pull request #19664: [hotfix][doc] add content for using numRecordsSend and deprecating numRecordsOutErrors

Posted by GitBox <gi...@apache.org>.
JingGe commented on code in PR #19664:
URL: https://github.com/apache/flink/pull/19664#discussion_r868138403


##########
docs/content/release-notes/flink-1.15.md:
##########
@@ -406,6 +406,18 @@ updating the client dependency to a version >= 7.14.0 is required due to interna
 The old JDBC connector (indicated by `connector.type=jdbc` in DDL) has been removed.
 If not done already, users need to upgrade to the newer stack (indicated by `connector=jdbc` in DDL).
 
+
+#### Extensible unified Sink uses new metric to capture outgoing records
+
+##### [FLINK-26126](https://issues.apache.org/jira/browse/FLINK-26126)
+
+New metrics `numRecordsSend` and `numRecordsSendErrors` has been introduced for users to monitor the number of 
+records sent to the external system. The `numRecordsOut` should be used to monitor the number of records 
+transferred between sink tasks.
+
+Connector developers should Consider using the new metric `numRecordsSendErrors` to monitor errors, 
+since `numRecordsOut` has a different and more general meaning, and `numRecordsOutErrors` is therefore deprecated.

Review Comment:
   This is confused for users and connector developers, tbh.  1. `numRecordsOut` will ONLY count the records(technically Commitables) sent between sink tasks. 2. Connectors still use `numRecordsOut` teamed up with `numRecordsSend` and `numRecordsSendErrors`. 3. the information of using `numRecordsSend` has been described in the first part. From now on, it is actually mandatory for connector developers to use all of them except `numRecordsOutErrors`. 
   
   How about this version:
   
   Connector developers should pay attention to the usage of these metrics `numRecordsOut`, `numRecordsSend` and `numRecordsSendErrors` while building sink connectors. Please refer to the new Kafka Sink for details. Additionally since `numRecordsOut` now only counts the records sent between sink tasks and `numRecordsOutErrors` was designed for counting the records sent to the external system, we deprecated `numRecordsOutErrors` and recommend using `numRecordsSendErrors` instead.
   
   Even with this description, it is still unclear, since `numRecordsOut` is still used, why is `numRecordsOutErrors` deprecated? It should be used to monitor errors happened while Commitables were send between sink tasks. WDYT?
   
   
   



-- 
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: issues-unsubscribe@flink.apache.org

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


[GitHub] [flink] JingGe commented on a diff in pull request #19664: [hotfix][doc] add content for using numRecordsSend and deprecating numRecordsOutErrors

Posted by GitBox <gi...@apache.org>.
JingGe commented on code in PR #19664:
URL: https://github.com/apache/flink/pull/19664#discussion_r868138403


##########
docs/content/release-notes/flink-1.15.md:
##########
@@ -406,6 +406,18 @@ updating the client dependency to a version >= 7.14.0 is required due to interna
 The old JDBC connector (indicated by `connector.type=jdbc` in DDL) has been removed.
 If not done already, users need to upgrade to the newer stack (indicated by `connector=jdbc` in DDL).
 
+
+#### Extensible unified Sink uses new metric to capture outgoing records
+
+##### [FLINK-26126](https://issues.apache.org/jira/browse/FLINK-26126)
+
+New metrics `numRecordsSend` and `numRecordsSendErrors` has been introduced for users to monitor the number of 
+records sent to the external system. The `numRecordsOut` should be used to monitor the number of records 
+transferred between sink tasks.
+
+Connector developers should Consider using the new metric `numRecordsSendErrors` to monitor errors, 
+since `numRecordsOut` has a different and more general meaning, and `numRecordsOutErrors` is therefore deprecated.

Review Comment:
   Hmm, this is confused for users and connector developers, tbh.  1. `numRecordsOut` will ONLY count the records(technically Commitables) sent between sink tasks. 2. Connectors still use `numRecordsOut` teamed up with `numRecordsSend` and `numRecordsSendErrors`. 3. the information of using `numRecordsSend` has been described in the first part. From now on, it is actually mandatory for connector developers to use all of them except `numRecordsOutErrors`. 
   
   How about this version:
   
   Connector developers should pay attention to the usage of these metrics `numRecordsOut`, `numRecordsSend` and `numRecordsSendErrors` while building sink connectors. Please refer to the new Kafka Sink for details. Additionally since `numRecordsOut` now only counts the records sent between sink tasks and `numRecordsOutErrors` was designed for counting the records sent to the external system, we deprecated `numRecordsOutErrors` and recommend using `numRecordsSendErrors` instead.
   
   Even with this description, it is still unclear, since `numRecordsOut` is still used, why is `numRecordsOutErrors` deprecated? It should be used to monitor errors happened while Commitables were send between sink tasks. WDYT?
   
   
   



-- 
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: issues-unsubscribe@flink.apache.org

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


[GitHub] [flink] MartijnVisser merged pull request #19664: [hotfix][doc] add content for using numRecordsSend and deprecating numRecordsOutErrors

Posted by GitBox <gi...@apache.org>.
MartijnVisser merged PR #19664:
URL: https://github.com/apache/flink/pull/19664


-- 
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: issues-unsubscribe@flink.apache.org

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


[GitHub] [flink] JingGe commented on a diff in pull request #19664: [hotfix][doc] add content for using numRecordsSend and deprecating numRecordsOutErrors

Posted by GitBox <gi...@apache.org>.
JingGe commented on code in PR #19664:
URL: https://github.com/apache/flink/pull/19664#discussion_r866974112


##########
docs/content/release-notes/flink-1.15.md:
##########
@@ -406,6 +406,13 @@ updating the client dependency to a version >= 7.14.0 is required due to interna
 The old JDBC connector (indicated by `connector.type=jdbc` in DDL) has been removed.
 If not done already, users need to upgrade to the newer stack (indicated by `connector=jdbc` in DDL).
 
+#### [FLINK-26126](https://issues.apache.org/jira/browse/FLINK-26126)

Review Comment:
   Excellent! I forgot that. Thanks!



-- 
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: issues-unsubscribe@flink.apache.org

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


[GitHub] [flink] JingGe commented on pull request #19664: [hotfix][doc] add content for using numRecordsSend and deprecating numRecordsOutErrors

Posted by GitBox <gi...@apache.org>.
JingGe commented on PR #19664:
URL: https://github.com/apache/flink/pull/19664#issuecomment-1121011528

   Thanks for your feedback @fapaul , I addressed your comments, please let me know your thoughts.


-- 
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: issues-unsubscribe@flink.apache.org

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


[GitHub] [flink] fapaul commented on a diff in pull request #19664: [hotfix][doc] add content for using numRecordsSend and deprecating numRecordsOutErrors

Posted by GitBox <gi...@apache.org>.
fapaul commented on code in PR #19664:
URL: https://github.com/apache/flink/pull/19664#discussion_r867991493


##########
docs/content/release-notes/flink-1.15.md:
##########
@@ -406,6 +406,18 @@ updating the client dependency to a version >= 7.14.0 is required due to interna
 The old JDBC connector (indicated by `connector.type=jdbc` in DDL) has been removed.
 If not done already, users need to upgrade to the newer stack (indicated by `connector=jdbc` in DDL).
 
+
+#### Extensible unified Sink uses new metric to capture outgoing records
+
+##### [FLINK-26126](https://issues.apache.org/jira/browse/FLINK-26126)
+
+New metrics `numRecordsSend` and `numRecordsSendErrors` has been introduced for users to monitor the number of 
+records sent to the external system. The `numRecordsOut` should be used to monitor the number of records 
+transferred between sink tasks.
+
+Connector developers should Consider using the new metric `numRecordsSendErrors` to monitor errors, 

Review Comment:
   ```suggestion
   Connector developers should consider using the new metric `numRecordsSendErrors` to register errors when sending to the external system instead of `numRecordsOutErrors` which is deprecated from now on.
   ```



##########
docs/content/release-notes/flink-1.15.md:
##########
@@ -406,6 +406,18 @@ updating the client dependency to a version >= 7.14.0 is required due to interna
 The old JDBC connector (indicated by `connector.type=jdbc` in DDL) has been removed.
 If not done already, users need to upgrade to the newer stack (indicated by `connector=jdbc` in DDL).
 
+
+#### Extensible unified Sink uses new metric to capture outgoing records
+
+##### [FLINK-26126](https://issues.apache.org/jira/browse/FLINK-26126)
+
+New metrics `numRecordsSend` and `numRecordsSendErrors` has been introduced for users to monitor the number of 

Review Comment:
   ```suggestion
   New metrics `numRecordsSend` and `numRecordsSendErrors` have been introduced for users to monitor the number of 
   ```



##########
docs/content/release-notes/flink-1.15.md:
##########
@@ -406,6 +406,18 @@ updating the client dependency to a version >= 7.14.0 is required due to interna
 The old JDBC connector (indicated by `connector.type=jdbc` in DDL) has been removed.
 If not done already, users need to upgrade to the newer stack (indicated by `connector=jdbc` in DDL).
 
+
+#### Extensible unified Sink uses new metric to capture outgoing records
+
+##### [FLINK-26126](https://issues.apache.org/jira/browse/FLINK-26126)
+
+New metrics `numRecordsSend` and `numRecordsSendErrors` has been introduced for users to monitor the number of 
+records sent to the external system. The `numRecordsOut` should be used to monitor the number of records 
+transferred between sink tasks.
+
+Connector developers should Consider using the new metric `numRecordsSendErrors` to monitor errors, 
+since `numRecordsOut` has a different and more general meaning, and `numRecordsOutErrors` is therefore deprecated.

Review Comment:
   ```suggestion
   Additionally since `numRecordsOut` now also counts the records sent between sink tasks we recommend changing the connector to use `numRecordsSend` to be fully accurate.
   ```



-- 
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: issues-unsubscribe@flink.apache.org

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


[GitHub] [flink] fapaul commented on a diff in pull request #19664: [hotfix][doc] add content for using numRecordsSend and deprecating numRecordsOutErrors

Posted by GitBox <gi...@apache.org>.
fapaul commented on code in PR #19664:
URL: https://github.com/apache/flink/pull/19664#discussion_r868923997


##########
docs/content/release-notes/flink-1.15.md:
##########
@@ -406,6 +406,18 @@ updating the client dependency to a version >= 7.14.0 is required due to interna
 The old JDBC connector (indicated by `connector.type=jdbc` in DDL) has been removed.
 If not done already, users need to upgrade to the newer stack (indicated by `connector=jdbc` in DDL).
 
+
+#### Extensible unified Sink uses new metric to capture outgoing records
+
+##### [FLINK-26126](https://issues.apache.org/jira/browse/FLINK-26126)
+
+New metrics `numRecordsSend` and `numRecordsSendErrors` has been introduced for users to monitor the number of 
+records sent to the external system. The `numRecordsOut` should be used to monitor the number of records 
+transferred between sink tasks.
+
+Connector developers should Consider using the new metric `numRecordsSendErrors` to monitor errors, 
+since `numRecordsOut` has a different and more general meaning, and `numRecordsOutErrors` is therefore deprecated.

Review Comment:
   Sounds good to me 👍 



-- 
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: issues-unsubscribe@flink.apache.org

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


[GitHub] [flink] flinkbot commented on pull request #19664: [hotfix][doc] add content for using numRecordsSend and deprecating numRecordsOutErrors

Posted by GitBox <gi...@apache.org>.
flinkbot commented on PR #19664:
URL: https://github.com/apache/flink/pull/19664#issuecomment-1119609351

   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "d7a71c91e7c3ec0799b12a33694715ac8ccedfcb",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "d7a71c91e7c3ec0799b12a33694715ac8ccedfcb",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * d7a71c91e7c3ec0799b12a33694715ac8ccedfcb UNKNOWN
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run azure` re-run the last Azure build
   </details>


-- 
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: issues-unsubscribe@flink.apache.org

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


[GitHub] [flink] JingGe commented on a diff in pull request #19664: [hotfix][doc] add content for using numRecordsSend and deprecating numRecordsOutErrors

Posted by GitBox <gi...@apache.org>.
JingGe commented on code in PR #19664:
URL: https://github.com/apache/flink/pull/19664#discussion_r866974112


##########
docs/content/release-notes/flink-1.15.md:
##########
@@ -406,6 +406,13 @@ updating the client dependency to a version >= 7.14.0 is required due to interna
 The old JDBC connector (indicated by `connector.type=jdbc` in DDL) has been removed.
 If not done already, users need to upgrade to the newer stack (indicated by `connector=jdbc` in DDL).
 
+#### [FLINK-26126](https://issues.apache.org/jira/browse/FLINK-26126)

Review Comment:
   Excellent! I forgot that.



-- 
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: issues-unsubscribe@flink.apache.org

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


[GitHub] [flink] JingGe commented on a diff in pull request #19664: [hotfix][doc] add content for using numRecordsSend and deprecating numRecordsOutErrors

Posted by GitBox <gi...@apache.org>.
JingGe commented on code in PR #19664:
URL: https://github.com/apache/flink/pull/19664#discussion_r868138403


##########
docs/content/release-notes/flink-1.15.md:
##########
@@ -406,6 +406,18 @@ updating the client dependency to a version >= 7.14.0 is required due to interna
 The old JDBC connector (indicated by `connector.type=jdbc` in DDL) has been removed.
 If not done already, users need to upgrade to the newer stack (indicated by `connector=jdbc` in DDL).
 
+
+#### Extensible unified Sink uses new metric to capture outgoing records
+
+##### [FLINK-26126](https://issues.apache.org/jira/browse/FLINK-26126)
+
+New metrics `numRecordsSend` and `numRecordsSendErrors` has been introduced for users to monitor the number of 
+records sent to the external system. The `numRecordsOut` should be used to monitor the number of records 
+transferred between sink tasks.
+
+Connector developers should Consider using the new metric `numRecordsSendErrors` to monitor errors, 
+since `numRecordsOut` has a different and more general meaning, and `numRecordsOutErrors` is therefore deprecated.

Review Comment:
   This is confused for users and connector developers, tbh.  1. `numRecordsOut` will ONLY count the records(technically Commitables) sent between sink tasks. 2. Connectors still use `numRecordsOut` teamed up with `numRecordsSend` and `numRecordsSendErrors`. 3. the information of using `numRecordsSend` has been described in the first part and there is no use case for "changing the connector"(all connectors that used `numRecordsOut` in a wrong way have been changed). From now on, it is actually mandatory for connector developers to use them except `numRecordsOutErrors`. 
   
   How about this version:
   
   Connector developers should pay attention to the usage of these metrics `numRecordsOut`, `numRecordsSend` and `numRecordsSendErrors` while building sink connectors. Please refer to the new Kafka Sink for details. Additionally since `numRecordsOut` now only counts the records sent between sink tasks and `numRecordsOutErrors` was designed for counting the records sent to the external system, we deprecated `numRecordsOutErrors` and recommend using `numRecordsSendErrors` instead.
   
   Event with this description, it is still unclear, since `numRecordsOut` is still used, why is `numRecordsOutErrors` deprecated? It should be used to monitor errors happened while Commitables were send between sink tasks. WDYT?
   
   
   



-- 
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: issues-unsubscribe@flink.apache.org

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


[GitHub] [flink] JingGe commented on a diff in pull request #19664: [hotfix][doc] add content for using numRecordsSend and deprecating numRecordsOutErrors

Posted by GitBox <gi...@apache.org>.
JingGe commented on code in PR #19664:
URL: https://github.com/apache/flink/pull/19664#discussion_r868138403


##########
docs/content/release-notes/flink-1.15.md:
##########
@@ -406,6 +406,18 @@ updating the client dependency to a version >= 7.14.0 is required due to interna
 The old JDBC connector (indicated by `connector.type=jdbc` in DDL) has been removed.
 If not done already, users need to upgrade to the newer stack (indicated by `connector=jdbc` in DDL).
 
+
+#### Extensible unified Sink uses new metric to capture outgoing records
+
+##### [FLINK-26126](https://issues.apache.org/jira/browse/FLINK-26126)
+
+New metrics `numRecordsSend` and `numRecordsSendErrors` has been introduced for users to monitor the number of 
+records sent to the external system. The `numRecordsOut` should be used to monitor the number of records 
+transferred between sink tasks.
+
+Connector developers should Consider using the new metric `numRecordsSendErrors` to monitor errors, 
+since `numRecordsOut` has a different and more general meaning, and `numRecordsOutErrors` is therefore deprecated.

Review Comment:
   This is confused for users and connector developers, tbh.  1. `numRecordsOut` will ONLY count the records(technically Commitables) sent between sink tasks. 2. Connectors still use `numRecordsOut` teamed up with `numRecordsSend` and `numRecordsSendErrors`. 3. the information of using `numRecordsSend` has been described in the first part. From now on, it is actually mandatory for connector developers to use them except `numRecordsOutErrors`. 
   
   How about this version:
   
   Connector developers should pay attention to the usage of these metrics `numRecordsOut`, `numRecordsSend` and `numRecordsSendErrors` while building sink connectors. Please refer to the new Kafka Sink for details. Additionally since `numRecordsOut` now only counts the records sent between sink tasks and `numRecordsOutErrors` was designed for counting the records sent to the external system, we deprecated `numRecordsOutErrors` and recommend using `numRecordsSendErrors` instead.
   
   Even with this description, it is still unclear, since `numRecordsOut` is still used, why is `numRecordsOutErrors` deprecated? It should be used to monitor errors happened while Commitables were send between sink tasks. WDYT?
   
   
   



-- 
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: issues-unsubscribe@flink.apache.org

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


[GitHub] [flink] JingGe commented on a diff in pull request #19664: [hotfix][doc] add content for using numRecordsSend and deprecating numRecordsOutErrors

Posted by GitBox <gi...@apache.org>.
JingGe commented on code in PR #19664:
URL: https://github.com/apache/flink/pull/19664#discussion_r868138403


##########
docs/content/release-notes/flink-1.15.md:
##########
@@ -406,6 +406,18 @@ updating the client dependency to a version >= 7.14.0 is required due to interna
 The old JDBC connector (indicated by `connector.type=jdbc` in DDL) has been removed.
 If not done already, users need to upgrade to the newer stack (indicated by `connector=jdbc` in DDL).
 
+
+#### Extensible unified Sink uses new metric to capture outgoing records
+
+##### [FLINK-26126](https://issues.apache.org/jira/browse/FLINK-26126)
+
+New metrics `numRecordsSend` and `numRecordsSendErrors` has been introduced for users to monitor the number of 
+records sent to the external system. The `numRecordsOut` should be used to monitor the number of records 
+transferred between sink tasks.
+
+Connector developers should Consider using the new metric `numRecordsSendErrors` to monitor errors, 
+since `numRecordsOut` has a different and more general meaning, and `numRecordsOutErrors` is therefore deprecated.

Review Comment:
   This is confused for users and connector developers, tbh.  1. `numRecordsOut` will ONLY count the records(technically Commitables) sent between sink tasks. 2. Connectors still use `numRecordsOut`. 3. the information of using `numRecordsSend` has been described in the first part and there is no use case for "changing the connector"(all connectors that used `numRecordsOut` in a wrong way have been changed). From now on, it is actually mandatory for connector developers to use them except `numRecordsOutErrors`. 
   
   How about this version:
   
   Connector developers should pay attention to the usage of these metrics `numRecordsOut`, `numRecordsSend` and `numRecordsSendErrors` while building your sink connectors. Please refer to the new Kafka Sink for more information. Additionally since `numRecordsOut` now only counts the records sent between sink tasks and `numRecordsOutErrors` was designed for counting the records sent to the external system, we deprecated `numRecordsOutErrors` and recommend using `numRecordsSendErrors` instead.
   
   Event with this description, it is still unclear, since `numRecordsOut` is still used, why is `numRecordsOutErrors` deprecated? It should be used to monitor errors happened while Commitables were send between sink tasks. WDYT?
   
   
   



-- 
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: issues-unsubscribe@flink.apache.org

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


[GitHub] [flink] JingGe commented on a diff in pull request #19664: [hotfix][doc] add content for using numRecordsSend and deprecating numRecordsOutErrors

Posted by GitBox <gi...@apache.org>.
JingGe commented on code in PR #19664:
URL: https://github.com/apache/flink/pull/19664#discussion_r868138403


##########
docs/content/release-notes/flink-1.15.md:
##########
@@ -406,6 +406,18 @@ updating the client dependency to a version >= 7.14.0 is required due to interna
 The old JDBC connector (indicated by `connector.type=jdbc` in DDL) has been removed.
 If not done already, users need to upgrade to the newer stack (indicated by `connector=jdbc` in DDL).
 
+
+#### Extensible unified Sink uses new metric to capture outgoing records
+
+##### [FLINK-26126](https://issues.apache.org/jira/browse/FLINK-26126)
+
+New metrics `numRecordsSend` and `numRecordsSendErrors` has been introduced for users to monitor the number of 
+records sent to the external system. The `numRecordsOut` should be used to monitor the number of records 
+transferred between sink tasks.
+
+Connector developers should Consider using the new metric `numRecordsSendErrors` to monitor errors, 
+since `numRecordsOut` has a different and more general meaning, and `numRecordsOutErrors` is therefore deprecated.

Review Comment:
   This is confused for users and connector developers, tbh.  1. `numRecordsOut` will ONLY count the records(technically Commitables) sent between sink tasks. 2. Connectors still use `numRecordsOut`. 3. the information of using `numRecordsSend` has been described in the first part and there is no use case for "changing the connector"(all connectors that used `numRecordsOut` in a wrong way have been changed). From now on, it is actually mandatory for connector developers to use them except `numRecordsOutErrors`. 
   
   How about this version:
   
   Connector developers should pay attention to the usage of these metrics `numRecordsOut`, `numRecordsSend` and `numRecordsSendErrors` while building sink connectors. Please refer to the new Kafka Sink for more information. Additionally since `numRecordsOut` now only counts the records sent between sink tasks and `numRecordsOutErrors` was designed for counting the records sent to the external system, we deprecated `numRecordsOutErrors` and recommend using `numRecordsSendErrors` instead.
   
   Event with this description, it is still unclear, since `numRecordsOut` is still used, why is `numRecordsOutErrors` deprecated? It should be used to monitor errors happened while Commitables were send between sink tasks. WDYT?
   
   
   



-- 
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: issues-unsubscribe@flink.apache.org

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


[GitHub] [flink] JingGe commented on a diff in pull request #19664: [hotfix][doc] add content for using numRecordsSend and deprecating numRecordsOutErrors

Posted by GitBox <gi...@apache.org>.
JingGe commented on code in PR #19664:
URL: https://github.com/apache/flink/pull/19664#discussion_r868138403


##########
docs/content/release-notes/flink-1.15.md:
##########
@@ -406,6 +406,18 @@ updating the client dependency to a version >= 7.14.0 is required due to interna
 The old JDBC connector (indicated by `connector.type=jdbc` in DDL) has been removed.
 If not done already, users need to upgrade to the newer stack (indicated by `connector=jdbc` in DDL).
 
+
+#### Extensible unified Sink uses new metric to capture outgoing records
+
+##### [FLINK-26126](https://issues.apache.org/jira/browse/FLINK-26126)
+
+New metrics `numRecordsSend` and `numRecordsSendErrors` has been introduced for users to monitor the number of 
+records sent to the external system. The `numRecordsOut` should be used to monitor the number of records 
+transferred between sink tasks.
+
+Connector developers should Consider using the new metric `numRecordsSendErrors` to monitor errors, 
+since `numRecordsOut` has a different and more general meaning, and `numRecordsOutErrors` is therefore deprecated.

Review Comment:
   This is confused for users and connector developers, tbh.  1. `numRecordsOut` will ONLY count the records(technically Commitables) sent between sink tasks. 2. Connectors still use `numRecordsOut` teamed up with `numRecordsSend` and `numRecordsSendErrors`. 3. the information of using `numRecordsSend` has been described in the first part and there is no use case for "changing the connector"(all connectors that used `numRecordsOut` in a wrong way have been changed). From now on, it is actually mandatory for connector developers to use them except `numRecordsOutErrors`. 
   
   How about this version:
   
   Connector developers should pay attention to the usage of these metrics `numRecordsOut`, `numRecordsSend` and `numRecordsSendErrors` while building sink connectors. Please refer to the new Kafka Sink for details. Additionally since `numRecordsOut` now only counts the records sent between sink tasks and `numRecordsOutErrors` was designed for counting the records sent to the external system, we deprecated `numRecordsOutErrors` and recommend using `numRecordsSendErrors` instead.
   
   Even with this description, it is still unclear, since `numRecordsOut` is still used, why is `numRecordsOutErrors` deprecated? It should be used to monitor errors happened while Commitables were send between sink tasks. WDYT?
   
   
   



-- 
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: issues-unsubscribe@flink.apache.org

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


[GitHub] [flink] JingGe commented on a diff in pull request #19664: [hotfix][doc] add content for using numRecordsSend and deprecating numRecordsOutErrors

Posted by GitBox <gi...@apache.org>.
JingGe commented on code in PR #19664:
URL: https://github.com/apache/flink/pull/19664#discussion_r868138403


##########
docs/content/release-notes/flink-1.15.md:
##########
@@ -406,6 +406,18 @@ updating the client dependency to a version >= 7.14.0 is required due to interna
 The old JDBC connector (indicated by `connector.type=jdbc` in DDL) has been removed.
 If not done already, users need to upgrade to the newer stack (indicated by `connector=jdbc` in DDL).
 
+
+#### Extensible unified Sink uses new metric to capture outgoing records
+
+##### [FLINK-26126](https://issues.apache.org/jira/browse/FLINK-26126)
+
+New metrics `numRecordsSend` and `numRecordsSendErrors` has been introduced for users to monitor the number of 
+records sent to the external system. The `numRecordsOut` should be used to monitor the number of records 
+transferred between sink tasks.
+
+Connector developers should Consider using the new metric `numRecordsSendErrors` to monitor errors, 
+since `numRecordsOut` has a different and more general meaning, and `numRecordsOutErrors` is therefore deprecated.

Review Comment:
   This is confused for users and connector developers, tbh.  1. `numRecordsOut` will ONLY count the records(technically Commitables) sent between sink tasks. 2. the information about the deprecated metric `numRecordsOutErrors` is missing. 3. the information of using `numRecordsSend` has been described in the first part and there is no use case for "changing the connector"(all connectors that used `numRecordsOut` in a wrong way have been changed). From now on, it is actually mandatory for connector developers to use them except `numRecordsOutErrors`. 
   
   How about this version:
   
   Connector developers should pay attention to the usage these metrics `numRecordsOut`, `numRecordsSend` and `numRecordsSendErrors` while building your sink connectors. Please refer to the new Kafka Sink for more information. Additionally since `numRecordsOut` now only counts the records sent between sink tasks and `numRecordsOutErrors` was designed for counting the records sent to the external system, we deprecated `numRecordsOutErrors` and recommend using `numRecordsSendErrors` instead.
   
   Event with this description, it is still unclear, since `numRecordsOut` is still used, why is `numRecordsOutErrors` deprecated? It should be used to monitor errors happened while Commitables were send between sink tasks. WDYT?
   
   
   



-- 
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: issues-unsubscribe@flink.apache.org

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


[GitHub] [flink] JingGe commented on pull request #19664: [hotfix][doc] add content for using numRecordsSend and deprecating numRecordsOutErrors

Posted by GitBox <gi...@apache.org>.
JingGe commented on PR #19664:
URL: https://github.com/apache/flink/pull/19664#issuecomment-1122480127

   > LGTM 👍 @JingGe can you also backport the change to release-1.15?
   
   sure, will do. Would you like to merge this PR? Thanks!


-- 
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: issues-unsubscribe@flink.apache.org

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


[GitHub] [flink] JingGe commented on a diff in pull request #19664: [hotfix][doc] add content for using numRecordsSend and deprecating numRecordsOutErrors

Posted by GitBox <gi...@apache.org>.
JingGe commented on code in PR #19664:
URL: https://github.com/apache/flink/pull/19664#discussion_r868138403


##########
docs/content/release-notes/flink-1.15.md:
##########
@@ -406,6 +406,18 @@ updating the client dependency to a version >= 7.14.0 is required due to interna
 The old JDBC connector (indicated by `connector.type=jdbc` in DDL) has been removed.
 If not done already, users need to upgrade to the newer stack (indicated by `connector=jdbc` in DDL).
 
+
+#### Extensible unified Sink uses new metric to capture outgoing records
+
+##### [FLINK-26126](https://issues.apache.org/jira/browse/FLINK-26126)
+
+New metrics `numRecordsSend` and `numRecordsSendErrors` has been introduced for users to monitor the number of 
+records sent to the external system. The `numRecordsOut` should be used to monitor the number of records 
+transferred between sink tasks.
+
+Connector developers should Consider using the new metric `numRecordsSendErrors` to monitor errors, 
+since `numRecordsOut` has a different and more general meaning, and `numRecordsOutErrors` is therefore deprecated.

Review Comment:
   This is confused for users and connector developers, tbh.  1. `numRecordsOut` will ONLY count the records(technically Commitables) sent between sink tasks. 2. Connectors still use `numRecordsOut`. 3. the information of using `numRecordsSend` has been described in the first part and there is no use case for "changing the connector"(all connectors that used `numRecordsOut` in a wrong way have been changed). From now on, it is actually mandatory for connector developers to use them except `numRecordsOutErrors`. 
   
   How about this version:
   
   Connector developers should pay attention to the usage these metrics `numRecordsOut`, `numRecordsSend` and `numRecordsSendErrors` while building your sink connectors. Please refer to the new Kafka Sink for more information. Additionally since `numRecordsOut` now only counts the records sent between sink tasks and `numRecordsOutErrors` was designed for counting the records sent to the external system, we deprecated `numRecordsOutErrors` and recommend using `numRecordsSendErrors` instead.
   
   Event with this description, it is still unclear, since `numRecordsOut` is still used, why is `numRecordsOutErrors` deprecated? It should be used to monitor errors happened while Commitables were send between sink tasks. WDYT?
   
   
   



-- 
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: issues-unsubscribe@flink.apache.org

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