You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2022/12/25 02:42:19 UTC

[GitHub] [pulsar] tisonkun opened a new pull request, #19051: [fix][broker] retry letter producer respect auto schema

tisonkun opened a new pull request, #19051:
URL: https://github.com/apache/pulsar/pull/19051

   * This supersedes #11500.
   * This closes #11474.
   
   ### Verifying this change
   
   Add a new test `testAutoConsumeSchemaRetryLetter`.
   
   ### Does this pull request potentially affect one of the following parts:
   
   <!-- DO NOT REMOVE THIS SECTION. CHECK THE PROPER BOX ONLY. -->
   
   *If the box was checked, please highlight the changes*
   
   - [ ] Dependencies (add or upgrade a dependency)
   - [ ] The public API
   - [ ] The schema
   - [ ] The default values of configurations
   - [ ] The threading model
   - [ ] The binary protocol
   - [ ] The REST endpoints
   - [ ] The admin CLI options
   - [ ] The metrics
   - [ ] Anything that affects deployment
   
   ### Documentation
   
   <!-- DO NOT REMOVE THIS SECTION. CHECK THE PROPER BOX ONLY. -->
   
   - [ ] `doc` <!-- Your PR contains doc changes. Please attach the local preview screenshots (run `sh start.sh` at `pulsar/site2/website`) to your PR description, or else your PR might not get merged. -->
   - [ ] `doc-required` <!-- Your PR changes impact docs and you will update later -->
   - [x] `doc-not-needed` <!-- Your PR changes do not impact docs -->
   - [ ] `doc-complete` <!-- Docs have been already added -->
   
   ### Matching PR in forked repository
   
   PR in forked repository: <!-- ENTER URL HERE -->
   
   <!--
   After opening this PR, the build in apache/pulsar will fail and instructions will
   be provided for opening a PR in the PR author's forked repository.
   
   apache/pulsar pull requests should be first tested in your own fork since the 
   apache/pulsar CI based on GitHub Actions has constrained resources and quota.
   GitHub Actions provides separate quota for pull requests that are executed in 
   a forked repository.
   
   The tests will be run in the forked repository until all PR review comments have
   been handled, the tests pass and the PR is approved by a reviewer.
   -->
   


-- 
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: commits-unsubscribe@pulsar.apache.org

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

[GitHub] [pulsar] tisonkun commented on pull request #19051: [fix][client] retry letter producer respect auto schema

Posted by GitBox <gi...@apache.org>.
tisonkun commented on PR #19051:
URL: https://github.com/apache/pulsar/pull/19051#issuecomment-1365773712

   @poorbarcode aha. It's due to a rebase on merge master commit. I can fix it later.


-- 
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: commits-unsubscribe@pulsar.apache.org

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

[GitHub] [pulsar] tisonkun commented on a diff in pull request #19051: [fix][client] retry letter producer respect auto schema

Posted by GitBox <gi...@apache.org>.
tisonkun commented on code in PR #19051:
URL: https://github.com/apache/pulsar/pull/19051#discussion_r1056973867


##########
pulsar-broker/src/test/java/org/apache/pulsar/client/api/RetryTopicTest.java:
##########
@@ -128,6 +133,100 @@ public void testRetryTopic() throws Exception {
         checkConsumer.close();
     }
 
+    @Data
+    public static class Foo {
+        @Nullable
+        private String field1;
+        @Nullable
+        private String field2;
+    }
+
+    @Data
+    public static class FooV2 {
+        @Nullable
+        private String field1;
+        @Nullable
+        private String field2;
+        @Nullable
+        private String field3;
+    }
+
+    @Test(timeOut = 20000)
+    public void testAutoConsumeSchemaRetryLetter() throws Exception {
+        final String topic = "persistent://my-property/my-ns/retry-letter-topic";
+        final String subName = "my-subscription";
+        final int maxRedeliveryCount = 1;
+        final int sendMessages = 10;
+
+        admin.topics().createNonPartitionedTopic(topic);
+
+        @Cleanup
+        PulsarClient newPulsarClient = newPulsarClient(lookupUrl.toString(), 0);// Creates new client connection
+        Consumer<FooV2> deadLetterConsumer = newPulsarClient.newConsumer(Schema.AVRO(FooV2.class))

Review Comment:
   I follow the test pattern from `testRetryTopic` and `testAutoConsumeSchemaDeadLetter`. Even `testRetryTopic` set up the DLQ.
   
   May you directly submit a patch onto this pull request?



-- 
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: commits-unsubscribe@pulsar.apache.org

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

[GitHub] [pulsar] tisonkun commented on pull request #19051: [fix][client] retry letter producer respect auto schema

Posted by GitBox <gi...@apache.org>.
tisonkun commented on PR #19051:
URL: https://github.com/apache/pulsar/pull/19051#issuecomment-1367884794

   @yaalsn 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: commits-unsubscribe@pulsar.apache.org

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

[GitHub] [pulsar] tisonkun commented on a diff in pull request #19051: [fix][client] retry letter producer respect auto schema

Posted by GitBox <gi...@apache.org>.
tisonkun commented on code in PR #19051:
URL: https://github.com/apache/pulsar/pull/19051#discussion_r1057287855


##########
pulsar-client/src/main/java/org/apache/pulsar/client/impl/ConsumerImpl.java:
##########
@@ -672,8 +672,10 @@ protected CompletableFuture<Void> doReconsumeLater(Message<?> message, AckType a
                         return null;
                     });
                 } else {
-                    TypedMessageBuilder<T> typedMessageBuilderNew = retryLetterProducer.newMessage()
-                            .value(retryMessage.getValue())
+                    assert retryMessage != null;
+                    TypedMessageBuilder<byte[]> typedMessageBuilderNew = retryLetterProducer
+                            .newMessage(Schema.AUTO_PRODUCE_BYTES(message.getReaderSchema().get()))

Review Comment:
   @poorbarcode I read https://github.com/apache/pulsar/pull/19051#discussion_r1056922378 is enough for a resolution :)



-- 
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: commits-unsubscribe@pulsar.apache.org

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

[GitHub] [pulsar] poorbarcode commented on a diff in pull request #19051: [fix][client] retry letter producer respect auto schema

Posted by GitBox <gi...@apache.org>.
poorbarcode commented on code in PR #19051:
URL: https://github.com/apache/pulsar/pull/19051#discussion_r1057285527


##########
pulsar-client/src/main/java/org/apache/pulsar/client/impl/ConsumerImpl.java:
##########
@@ -672,8 +672,10 @@ protected CompletableFuture<Void> doReconsumeLater(Message<?> message, AckType a
                         return null;
                     });
                 } else {
-                    TypedMessageBuilder<T> typedMessageBuilderNew = retryLetterProducer.newMessage()
-                            .value(retryMessage.getValue())
+                    assert retryMessage != null;
+                    TypedMessageBuilder<byte[]> typedMessageBuilderNew = retryLetterProducer
+                            .newMessage(Schema.AUTO_PRODUCE_BYTES(message.getReaderSchema().get()))

Review Comment:
   Hi @congbobo184 Could you give some advice?



-- 
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: commits-unsubscribe@pulsar.apache.org

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

[GitHub] [pulsar] poorbarcode commented on pull request #19051: [fix][client] retry letter producer respect auto schema

Posted by GitBox <gi...@apache.org>.
poorbarcode commented on PR #19051:
URL: https://github.com/apache/pulsar/pull/19051#issuecomment-1365763638

   Hi @tisonkun 
   
   I found that the Files Changed have been dirty, could you deal with it?
   
   <img width="1185" alt="截屏2022-12-27 17 44 19" src="https://user-images.githubusercontent.com/25195800/209647384-ffc2229a-94f7-48fa-97fe-19f502cb6d6f.png">
   


-- 
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: commits-unsubscribe@pulsar.apache.org

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

[GitHub] [pulsar] congbobo184 commented on a diff in pull request #19051: [fix][client] retry letter producer respect auto schema

Posted by GitBox <gi...@apache.org>.
congbobo184 commented on code in PR #19051:
URL: https://github.com/apache/pulsar/pull/19051#discussion_r1056922378


##########
pulsar-client/src/main/java/org/apache/pulsar/client/impl/ConsumerImpl.java:
##########
@@ -672,8 +672,10 @@ protected CompletableFuture<Void> doReconsumeLater(Message<?> message, AckType a
                         return null;
                     });
                 } else {
-                    TypedMessageBuilder<T> typedMessageBuilderNew = retryLetterProducer.newMessage()
-                            .value(retryMessage.getValue())
+                    assert retryMessage != null;
+                    TypedMessageBuilder<byte[]> typedMessageBuilderNew = retryLetterProducer
+                            .newMessage(Schema.AUTO_PRODUCE_BYTES(message.getReaderSchema().get()))

Review Comment:
   Unless the user code like this `new TypedMessageBuilderImpl(null, null)` and this is not the `reconsumeLaster` correct use way.
   we use `reconsumeLaster` should use the invoke `consumer.receive()` returned messages,  or construct the correct message yourself



-- 
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: commits-unsubscribe@pulsar.apache.org

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

[GitHub] [pulsar] tisonkun commented on pull request #19051: [fix][client] retry letter producer respect auto schema

Posted by GitBox <gi...@apache.org>.
tisonkun commented on PR #19051:
URL: https://github.com/apache/pulsar/pull/19051#issuecomment-1365756831

   Thank you!


-- 
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: commits-unsubscribe@pulsar.apache.org

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

[GitHub] [pulsar] congbobo184 commented on a diff in pull request #19051: [fix][client] retry letter producer respect auto schema

Posted by GitBox <gi...@apache.org>.
congbobo184 commented on code in PR #19051:
URL: https://github.com/apache/pulsar/pull/19051#discussion_r1056924662


##########
pulsar-broker/src/test/java/org/apache/pulsar/client/api/RetryTopicTest.java:
##########
@@ -128,6 +133,100 @@ public void testRetryTopic() throws Exception {
         checkConsumer.close();
     }
 
+    @Data
+    public static class Foo {
+        @Nullable
+        private String field1;
+        @Nullable
+        private String field2;
+    }
+
+    @Data
+    public static class FooV2 {
+        @Nullable
+        private String field1;
+        @Nullable
+        private String field2;
+        @Nullable
+        private String field3;
+    }
+
+    @Test(timeOut = 20000)
+    public void testAutoConsumeSchemaRetryLetter() throws Exception {
+        final String topic = "persistent://my-property/my-ns/retry-letter-topic";
+        final String subName = "my-subscription";
+        final int maxRedeliveryCount = 1;
+        final int sendMessages = 10;
+
+        admin.topics().createNonPartitionedTopic(topic);
+
+        @Cleanup
+        PulsarClient newPulsarClient = newPulsarClient(lookupUrl.toString(), 0);// Creates new client connection
+        Consumer<FooV2> deadLetterConsumer = newPulsarClient.newConsumer(Schema.AVRO(FooV2.class))
+                .topic(topic + "-" + subName + "-DLQ")
+                .subscriptionName("my-subscription")
+                .subscriptionInitialPosition(SubscriptionInitialPosition.Earliest)
+                .subscribe();
+
+        Producer<byte[]> producer = pulsarClient.newProducer(Schema.AUTO_PRODUCE_BYTES())
+                .topic(topic)
+                .create();
+        Set<String> messageIds = new HashSet<>();
+        for (int i = 0; i < sendMessages; i++) {
+            if (i % 2 == 0) {
+                Foo foo = new Foo();
+                foo.field1 = i + "";
+                foo.field2 = i + "";
+                messageIds.add(producer.newMessage(Schema.AVRO(Foo.class)).value(foo).send().toString());
+            } else {
+                FooV2 foo = new FooV2();
+                foo.field1 = i + "";
+                foo.field2 = i + "";
+                foo.field3 = i + "";
+                messageIds.add(producer.newMessage(Schema.AVRO(FooV2.class)).value(foo).send().toString());
+            }
+        }
+        Consumer<GenericRecord> consumer = pulsarClient.newConsumer(Schema.AUTO_CONSUME())
+                .topic(topic)
+                .subscriptionName(subName)
+                .subscriptionType(SubscriptionType.Shared)
+                .ackTimeout(1, TimeUnit.SECONDS)
+                .enableRetry(true)
+                .receiverQueueSize(100)
+                .deadLetterPolicy(DeadLetterPolicy.builder().maxRedeliverCount(maxRedeliveryCount).build())

Review Comment:
   we don't need to test deal letter right?



##########
pulsar-broker/src/test/java/org/apache/pulsar/client/api/RetryTopicTest.java:
##########
@@ -128,6 +133,100 @@ public void testRetryTopic() throws Exception {
         checkConsumer.close();
     }
 
+    @Data
+    public static class Foo {
+        @Nullable
+        private String field1;
+        @Nullable
+        private String field2;
+    }
+
+    @Data
+    public static class FooV2 {
+        @Nullable
+        private String field1;
+        @Nullable
+        private String field2;
+        @Nullable
+        private String field3;
+    }
+
+    @Test(timeOut = 20000)
+    public void testAutoConsumeSchemaRetryLetter() throws Exception {
+        final String topic = "persistent://my-property/my-ns/retry-letter-topic";
+        final String subName = "my-subscription";
+        final int maxRedeliveryCount = 1;
+        final int sendMessages = 10;
+
+        admin.topics().createNonPartitionedTopic(topic);
+
+        @Cleanup
+        PulsarClient newPulsarClient = newPulsarClient(lookupUrl.toString(), 0);// Creates new client connection
+        Consumer<FooV2> deadLetterConsumer = newPulsarClient.newConsumer(Schema.AVRO(FooV2.class))

Review Comment:
   why we should use `deadLetterConsumer`?



-- 
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: commits-unsubscribe@pulsar.apache.org

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

[GitHub] [pulsar] tisonkun commented on a diff in pull request #19051: [fix][broker] retry letter producer respect auto schema

Posted by GitBox <gi...@apache.org>.
tisonkun commented on code in PR #19051:
URL: https://github.com/apache/pulsar/pull/19051#discussion_r1056893395


##########
pulsar-client/src/main/java/org/apache/pulsar/client/impl/ConsumerImpl.java:
##########
@@ -672,8 +672,10 @@ protected CompletableFuture<Void> doReconsumeLater(Message<?> message, AckType a
                         return null;
                     });
                 } else {
-                    TypedMessageBuilder<T> typedMessageBuilderNew = retryLetterProducer.newMessage()
-                            .value(retryMessage.getValue())
+                    assert retryMessage != null;
+                    TypedMessageBuilder<byte[]> typedMessageBuilderNew = retryLetterProducer
+                            .newMessage(Schema.AUTO_PRODUCE_BYTES(message.getReaderSchema().get()))

Review Comment:
   This line is copied from how we work for deadletter, I'm wondering whether it can produce NPE at `message.getReaderSchema().get()`. cc @congbobo184 



##########
pulsar-client/src/main/java/org/apache/pulsar/client/impl/ConsumerImpl.java:
##########
@@ -672,8 +672,10 @@ protected CompletableFuture<Void> doReconsumeLater(Message<?> message, AckType a
                         return null;
                     });
                 } else {
-                    TypedMessageBuilder<T> typedMessageBuilderNew = retryLetterProducer.newMessage()
-                            .value(retryMessage.getValue())
+                    assert retryMessage != null;
+                    TypedMessageBuilder<byte[]> typedMessageBuilderNew = retryLetterProducer
+                            .newMessage(Schema.AUTO_PRODUCE_BYTES(message.getReaderSchema().get()))

Review Comment:
   This line is copied from how we work for deadletter. I'm wondering whether it can produce NPE at `message.getReaderSchema().get()`. cc @congbobo184 



-- 
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: commits-unsubscribe@pulsar.apache.org

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

[GitHub] [pulsar] tisonkun commented on pull request #19051: [fix][client] retry letter producer respect auto schema

Posted by GitBox <gi...@apache.org>.
tisonkun commented on PR #19051:
URL: https://github.com/apache/pulsar/pull/19051#issuecomment-1365917931

   Merging...


-- 
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: commits-unsubscribe@pulsar.apache.org

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

[GitHub] [pulsar] tisonkun commented on pull request #19051: [fix][client] retry letter producer respect auto schema

Posted by GitBox <gi...@apache.org>.
tisonkun commented on PR #19051:
URL: https://github.com/apache/pulsar/pull/19051#issuecomment-1365756781

   @congbobo184 I saw your patch and it seems code style check failed now. I'll try to fix it in hours.


-- 
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: commits-unsubscribe@pulsar.apache.org

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

[GitHub] [pulsar] yaalsn commented on pull request #19051: [fix][client] retry letter producer respect auto schema

Posted by GitBox <gi...@apache.org>.
yaalsn commented on PR #19051:
URL: https://github.com/apache/pulsar/pull/19051#issuecomment-1367880021

   @tisonkun It seems that it passed after you rerun the job. Did you modify any code before the rerun?


-- 
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: commits-unsubscribe@pulsar.apache.org

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

[GitHub] [pulsar] tisonkun commented on pull request #19051: [fix][client] retry letter producer respect auto schema

Posted by GitBox <gi...@apache.org>.
tisonkun commented on PR #19051:
URL: https://github.com/apache/pulsar/pull/19051#issuecomment-1365899434

   A weird error:
   
   ```
     Error:  Failed to execute goal org.jacoco:jacoco-maven-plugin:0.8.7:report (post-test) on project pulsar-broker: An error has occurred in JaCoCo report generation. Error while creating report: malformed input around byte 2 -> [Help 1]
     Error:  
     Error:  To see the full stack trace of the errors, re-run Maven with the -e switch.
     Error:  Re-run Maven using the -X switch to enable full debug logging.
     Error:  
     Error:  For more information about the errors and possible solutions, please read the following articles:
     Error:  [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoExecutionException
     Error: Process completed with exit code 1.
   ```
   
   I'm trying rerun, but it seems an unexpected error. cc @yaalsn IIRC jacoco report is introduced by you.


-- 
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: commits-unsubscribe@pulsar.apache.org

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

[GitHub] [pulsar] codecov-commenter commented on pull request #19051: [fix][client] retry letter producer respect auto schema

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #19051:
URL: https://github.com/apache/pulsar/pull/19051#issuecomment-1364619402

   # [Codecov](https://codecov.io/gh/apache/pulsar/pull/19051?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#19051](https://codecov.io/gh/apache/pulsar/pull/19051?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (9d74a4c) into [master](https://codecov.io/gh/apache/pulsar/commit/d8569cd4ec6da14f8b2b9338db1ed2f6a3eacf0a?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (d8569cd) will **decrease** coverage by `9.86%`.
   > The diff coverage is `0.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/pulsar/pull/19051/graphs/tree.svg?width=650&height=150&src=pr&token=acYqCpsK9J&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/pulsar/pull/19051?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master   #19051      +/-   ##
   ============================================
   - Coverage     46.82%   36.96%   -9.87%     
   + Complexity    10567     1970    -8597     
   ============================================
     Files           709      209     -500     
     Lines         69421    14437   -54984     
     Branches       7449     1575    -5874     
   ============================================
   - Hits          32508     5336   -27172     
   + Misses        33239     8516   -24723     
   + Partials       3674      585    -3089     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | unittests | `36.96% <0.00%> (-9.87%)` | :arrow_down: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/pulsar/pull/19051?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...va/org/apache/pulsar/client/impl/ConsumerImpl.java](https://codecov.io/gh/apache/pulsar/pull/19051/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHVsc2FyLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2NsaWVudC9pbXBsL0NvbnN1bWVySW1wbC5qYXZh) | `15.06% <0.00%> (-0.04%)` | :arrow_down: |
   | [...a/org/apache/pulsar/proxy/server/ProxyService.java](https://codecov.io/gh/apache/pulsar/pull/19051/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHVsc2FyLXByb3h5L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9wdWxzYXIvcHJveHkvc2VydmVyL1Byb3h5U2VydmljZS5qYXZh) | | |
   | [...a/org/apache/bookkeeper/mledger/ManagedCursor.java](https://codecov.io/gh/apache/pulsar/pull/19051/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-bWFuYWdlZC1sZWRnZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2Jvb2trZWVwZXIvbWxlZGdlci9NYW5hZ2VkQ3Vyc29yLmphdmE=) | | |
   | [...he/pulsar/broker/delayed/bucket/MutableBucket.java](https://codecov.io/gh/apache/pulsar/pull/19051/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2Jyb2tlci9kZWxheWVkL2J1Y2tldC9NdXRhYmxlQnVja2V0LmphdmE=) | | |
   | [...keeper/mledger/impl/cache/PendingReadsManager.java](https://codecov.io/gh/apache/pulsar/pull/19051/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-bWFuYWdlZC1sZWRnZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2Jvb2trZWVwZXIvbWxlZGdlci9pbXBsL2NhY2hlL1BlbmRpbmdSZWFkc01hbmFnZXIuamF2YQ==) | | |
   | [...a/org/apache/pulsar/client/impl/RawReaderImpl.java](https://codecov.io/gh/apache/pulsar/pull/19051/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2NsaWVudC9pbXBsL1Jhd1JlYWRlckltcGwuamF2YQ==) | | |
   | [.../timeout/TransactionTimeoutTrackerFactoryImpl.java](https://codecov.io/gh/apache/pulsar/pull/19051/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2Jyb2tlci90cmFuc2FjdGlvbi90aW1lb3V0L1RyYW5zYWN0aW9uVGltZW91dFRyYWNrZXJGYWN0b3J5SW1wbC5qYXZh) | | |
   | [...keeper/mledger/impl/cache/RangeEntryCacheImpl.java](https://codecov.io/gh/apache/pulsar/pull/19051/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-bWFuYWdlZC1sZWRnZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2Jvb2trZWVwZXIvbWxlZGdlci9pbXBsL2NhY2hlL1JhbmdlRW50cnlDYWNoZUltcGwuamF2YQ==) | | |
   | [...g/apache/pulsar/broker/service/StreamingStats.java](https://codecov.io/gh/apache/pulsar/pull/19051/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2Jyb2tlci9zZXJ2aWNlL1N0cmVhbWluZ1N0YXRzLmphdmE=) | | |
   | ... and [496 more](https://codecov.io/gh/apache/pulsar/pull/19051/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   


-- 
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: commits-unsubscribe@pulsar.apache.org

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

[GitHub] [pulsar] congbobo184 commented on a diff in pull request #19051: [fix][client] retry letter producer respect auto schema

Posted by GitBox <gi...@apache.org>.
congbobo184 commented on code in PR #19051:
URL: https://github.com/apache/pulsar/pull/19051#discussion_r1056924662


##########
pulsar-broker/src/test/java/org/apache/pulsar/client/api/RetryTopicTest.java:
##########
@@ -128,6 +133,100 @@ public void testRetryTopic() throws Exception {
         checkConsumer.close();
     }
 
+    @Data
+    public static class Foo {
+        @Nullable
+        private String field1;
+        @Nullable
+        private String field2;
+    }
+
+    @Data
+    public static class FooV2 {
+        @Nullable
+        private String field1;
+        @Nullable
+        private String field2;
+        @Nullable
+        private String field3;
+    }
+
+    @Test(timeOut = 20000)
+    public void testAutoConsumeSchemaRetryLetter() throws Exception {
+        final String topic = "persistent://my-property/my-ns/retry-letter-topic";
+        final String subName = "my-subscription";
+        final int maxRedeliveryCount = 1;
+        final int sendMessages = 10;
+
+        admin.topics().createNonPartitionedTopic(topic);
+
+        @Cleanup
+        PulsarClient newPulsarClient = newPulsarClient(lookupUrl.toString(), 0);// Creates new client connection
+        Consumer<FooV2> deadLetterConsumer = newPulsarClient.newConsumer(Schema.AVRO(FooV2.class))
+                .topic(topic + "-" + subName + "-DLQ")
+                .subscriptionName("my-subscription")
+                .subscriptionInitialPosition(SubscriptionInitialPosition.Earliest)
+                .subscribe();
+
+        Producer<byte[]> producer = pulsarClient.newProducer(Schema.AUTO_PRODUCE_BYTES())
+                .topic(topic)
+                .create();
+        Set<String> messageIds = new HashSet<>();
+        for (int i = 0; i < sendMessages; i++) {
+            if (i % 2 == 0) {
+                Foo foo = new Foo();
+                foo.field1 = i + "";
+                foo.field2 = i + "";
+                messageIds.add(producer.newMessage(Schema.AVRO(Foo.class)).value(foo).send().toString());
+            } else {
+                FooV2 foo = new FooV2();
+                foo.field1 = i + "";
+                foo.field2 = i + "";
+                foo.field3 = i + "";
+                messageIds.add(producer.newMessage(Schema.AVRO(FooV2.class)).value(foo).send().toString());
+            }
+        }
+        Consumer<GenericRecord> consumer = pulsarClient.newConsumer(Schema.AUTO_CONSUME())
+                .topic(topic)
+                .subscriptionName(subName)
+                .subscriptionType(SubscriptionType.Shared)
+                .ackTimeout(1, TimeUnit.SECONDS)
+                .enableRetry(true)
+                .receiverQueueSize(100)
+                .deadLetterPolicy(DeadLetterPolicy.builder().maxRedeliverCount(maxRedeliveryCount).build())

Review Comment:
   we don't need to test dead letter right?



-- 
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: commits-unsubscribe@pulsar.apache.org

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

[GitHub] [pulsar] tisonkun merged pull request #19051: [fix][client] retry letter producer respect auto schema

Posted by GitBox <gi...@apache.org>.
tisonkun merged PR #19051:
URL: https://github.com/apache/pulsar/pull/19051


-- 
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: commits-unsubscribe@pulsar.apache.org

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

[GitHub] [pulsar] tisonkun commented on pull request #19051: [fix][client] retry letter producer respect auto schema

Posted by GitBox <gi...@apache.org>.
tisonkun commented on PR #19051:
URL: https://github.com/apache/pulsar/pull/19051#issuecomment-1365825997

   @poorbarcode updated.


-- 
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: commits-unsubscribe@pulsar.apache.org

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