You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2020/09/05 07:45:04 UTC

[GitHub] [kafka] vgvineet4 opened a new pull request #9254: KAFKA-10462: Added support to pass headers in producerPerformance script

vgvineet4 opened a new pull request #9254:
URL: https://github.com/apache/kafka/pull/9254


   *More detailed description of your change,
   if necessary. The PR title and PR message become
   the squashed commit message, so use a separate
   comment to ping reviewers.*
   
   *Summary of testing strategy (including rationale)
   for the feature or bug fix. Unit and/or integration
   tests are expected for any behaviour change and
   system tests should be considered for larger changes.*
   
   ### Committer Checklist (excluded from commit message)
   - [ ] Verify design and implementation 
   - [ ] Verify test coverage and CI build status
   - [ ] Verify documentation (including upgrade notes)
   


----------------------------------------------------------------
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] [kafka] vgvineet4 commented on pull request #9254: KAFKA-10462: Added support to pass headers in producerPerformance script

Posted by GitBox <gi...@apache.org>.
vgvineet4 commented on pull request #9254:
URL: https://github.com/apache/kafka/pull/9254#issuecomment-691447099






----------------------------------------------------------------
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] [kafka] vgvineet4 commented on pull request #9254: KAFKA-10462: Added support to pass headers in producerPerformance script

Posted by GitBox <gi...@apache.org>.
vgvineet4 commented on pull request #9254:
URL: https://github.com/apache/kafka/pull/9254#issuecomment-687620244


   Ran command :
   ./gradlew clean compileJava compileScala compileTestJava compileTestScala spotlessScalaCheck checkstyleMain checkstyleTest spotbugsMain rat --profile --no-daemon --continue -PxmlSpotBugsReport=true
    which show success in local, not sure why its showing as error in checks
   
   Also the tests failing are I doubt related to my changes.


----------------------------------------------------------------
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] [kafka] vgvineet4 commented on pull request #9254: KAFKA-10462: Added support to pass headers in producerPerformance script

Posted by GitBox <gi...@apache.org>.
vgvineet4 commented on pull request #9254:
URL: https://github.com/apache/kafka/pull/9254#issuecomment-694948975


   @vvcephei please have a look


----------------------------------------------------------------
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] [kafka] vgvineet4 commented on pull request #9254: KAFKA-10462: Added support to pass headers in producerPerformance script

Posted by GitBox <gi...@apache.org>.
vgvineet4 commented on pull request #9254:
URL: https://github.com/apache/kafka/pull/9254#issuecomment-691447099


   @ijuma Please have a look 


----------------------------------------------------------------
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] [kafka] vgvineet4 commented on pull request #9254: KAFKA-10462: Added support to pass headers in producerPerformance script

Posted by GitBox <gi...@apache.org>.
vgvineet4 commented on pull request #9254:
URL: https://github.com/apache/kafka/pull/9254#issuecomment-731613175


   @omkreddy , Please have a look


----------------------------------------------------------------
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] [kafka] vgvineet4 commented on a change in pull request #9254: KAFKA-10462: Added support to pass headers in producerPerformance script

Posted by GitBox <gi...@apache.org>.
vgvineet4 commented on a change in pull request #9254:
URL: https://github.com/apache/kafka/pull/9254#discussion_r491690401



##########
File path: tools/src/main/java/org/apache/kafka/tools/ProducerPerformance.java
##########
@@ -132,12 +138,16 @@ public static void main(String[] args) throws Exception {
                     transactionStartTime = System.currentTimeMillis();
                 }
 
-
                 if (payloadFilePath != null) {
                     payload = payloadByteList.get(random.nextInt(payloadByteList.size()));
                 }
                 record = new ProducerRecord<>(topicName, payload);
-
+                Enumeration e = headerProps.propertyNames();
+                while (e.hasMoreElements()) {
+                    String key = (String) e.nextElement();
+                    System.out.println(key + " -- " + headerProps.getProperty(key));
+                    record.headers().add(key, headerProps.getProperty(key).getBytes("UTF-8"));

Review comment:
       Nice catch!!..changes incorporated.




----------------------------------------------------------------
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] [kafka] vgvineet4 commented on a change in pull request #9254: KAFKA-10462: Added support to pass headers in producerPerformance script

Posted by GitBox <gi...@apache.org>.
vgvineet4 commented on a change in pull request #9254:
URL: https://github.com/apache/kafka/pull/9254#discussion_r491690401



##########
File path: tools/src/main/java/org/apache/kafka/tools/ProducerPerformance.java
##########
@@ -132,12 +138,16 @@ public static void main(String[] args) throws Exception {
                     transactionStartTime = System.currentTimeMillis();
                 }
 
-
                 if (payloadFilePath != null) {
                     payload = payloadByteList.get(random.nextInt(payloadByteList.size()));
                 }
                 record = new ProducerRecord<>(topicName, payload);
-
+                Enumeration e = headerProps.propertyNames();
+                while (e.hasMoreElements()) {
+                    String key = (String) e.nextElement();
+                    System.out.println(key + " -- " + headerProps.getProperty(key));
+                    record.headers().add(key, headerProps.getProperty(key).getBytes("UTF-8"));

Review comment:
       Nice catch!!..changes incorporated.




----------------------------------------------------------------
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] [kafka] vgvineet4 commented on a change in pull request #9254: KAFKA-10462: Added support to pass headers in producerPerformance script

Posted by GitBox <gi...@apache.org>.
vgvineet4 commented on a change in pull request #9254:
URL: https://github.com/apache/kafka/pull/9254#discussion_r491649810



##########
File path: tools/src/main/java/org/apache/kafka/tools/ProducerPerformance.java
##########
@@ -87,6 +88,11 @@ public static void main(String[] args) throws Exception {
                     payloadByteList.add(payload.getBytes(StandardCharsets.UTF_8));
                 }
             }
+            
+            Properties headerProps = new Properties();
+            if (headersPath != null) {
+                headerProps.putAll(Utils.loadProps(headersPath));

Review comment:
       @omkreddy Thanks for reviewing the PR. Line number 142 <code>payload = payloadByteList.get(random.nextInt(payloadByteList.size()));</code> pulls the record in file randomly. It would be hard to map headers to the required payload. My thinking was this could serve well for those who want to check against a particular workflow where the payload or data in each record varies but header remain constant. 
   
   




----------------------------------------------------------------
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] [kafka] vgvineet4 commented on pull request #9254: KAFKA-10462: Added support to pass headers in producerPerformance script

Posted by GitBox <gi...@apache.org>.
vgvineet4 commented on pull request #9254:
URL: https://github.com/apache/kafka/pull/9254#issuecomment-691447099






----------------------------------------------------------------
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] [kafka] vgvineet4 commented on pull request #9254: KAFKA-10462: Added support to pass headers in producerPerformance script

Posted by GitBox <gi...@apache.org>.
vgvineet4 commented on pull request #9254:
URL: https://github.com/apache/kafka/pull/9254#issuecomment-687743208


   @omkreddy Please have a look


----------------------------------------------------------------
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] [kafka] vgvineet4 commented on pull request #9254: KAFKA-10462: Added support to pass headers in producerPerformance script

Posted by GitBox <gi...@apache.org>.
vgvineet4 commented on pull request #9254:
URL: https://github.com/apache/kafka/pull/9254#issuecomment-694001111


   @wcarlson5 please have a look 


----------------------------------------------------------------
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] [kafka] omkreddy commented on a change in pull request #9254: KAFKA-10462: Added support to pass headers in producerPerformance script

Posted by GitBox <gi...@apache.org>.
omkreddy commented on a change in pull request #9254:
URL: https://github.com/apache/kafka/pull/9254#discussion_r491421315



##########
File path: tools/src/main/java/org/apache/kafka/tools/ProducerPerformance.java
##########
@@ -87,6 +88,11 @@ public static void main(String[] args) throws Exception {
                     payloadByteList.add(payload.getBytes(StandardCharsets.UTF_8));
                 }
             }
+            
+            Properties headerProps = new Properties();
+            if (headersPath != null) {
+                headerProps.putAll(Utils.loadProps(headersPath));

Review comment:
       @vgvineet4 Thanks for the PR. we are adding same header values for each record. Should we set different values for each record?  maybe add string header value?

##########
File path: tools/src/main/java/org/apache/kafka/tools/ProducerPerformance.java
##########
@@ -132,12 +138,16 @@ public static void main(String[] args) throws Exception {
                     transactionStartTime = System.currentTimeMillis();
                 }
 
-
                 if (payloadFilePath != null) {
                     payload = payloadByteList.get(random.nextInt(payloadByteList.size()));
                 }
                 record = new ProducerRecord<>(topicName, payload);
-
+                Enumeration e = headerProps.propertyNames();
+                while (e.hasMoreElements()) {
+                    String key = (String) e.nextElement();
+                    System.out.println(key + " -- " + headerProps.getProperty(key));
+                    record.headers().add(key, headerProps.getProperty(key).getBytes("UTF-8"));

Review comment:
       Instead of adding headers each time, maybe we can pre-create the headers list and pass to ProducerRecord() constructor.




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