[PIP-236][fix][broker]Fix using schema to create consumer fails after using AUTO_CONSUME consumer to subscribe topic #17449

Denovo1998 · 2022-09-03T16:30:04Z

Fixes #17354
PIP #19113

Motivation

Fixed the failure to use schema to create consumer after using AUTO-CONSUME consumer to subscribe an empty topic, and Broker returned the error message as IncompatibleSchemaException("Topic does not have schema to check").

Modifications

In PersistentTopic::addSchemaIfIdleOrCheckCompatible, when there is an active consumer, but the consumer is using the AUTO_CONSUME schema to subscribe to the topic. Continuing to create a schema consumer to subscribe to the topic will fail.

When numActiveConsumers != 0, and check the schema of the currently existing consumers is AUTO_CONSUME schema.

Verifying this change

Make sure that the change passes the CI checks.

Does this pull request potentially affect one of the following parts:

If the box was checked, please highlight the changes

Documentation

Check the box below or label this PR directly.

Need to update docs?

doc-required
(Your PR needs to update docs and you will update later)
doc-not-needed
(Please explain why)
doc
(Your PR contains doc changes)
doc-complete
(Docs have been already added)

Matching PR in forked repository

PR in forked repository: Denovo1998#1

…g AUTO_CONSUME consumer to subscribe topic

Technoboy- · 2022-09-06T07:31:47Z

Hi @Denovo1998 , we'd better add test for this.

…g AUTO_CONSUME consumer to subscribe topic

…ls after using AUTO_CONSUME consumer to subscribe topic

Denovo1998 · 2022-09-06T12:15:08Z

Already done @Technoboy-

…g AUTO_CONSUME consumer to subscribe topic

…ls after using AUTO_CONSUME consumer to subscribe topic

Denovo1998 · 2022-10-02T10:09:52Z

/pulsarbot rerun-failure-checks

Denovo1998 · 2022-10-18T01:13:04Z

@congbobo184 @mattisonchao PTAL

Denovo1998 · 2022-11-01T06:50:27Z

@codelipenghui @gaoran10 @liangyepianzhou PTAL

pulsar-client/src/main/java/org/apache/pulsar/client/impl/schema/AutoConsumeSchema.java

pulsar-client/src/main/java/org/apache/pulsar/client/impl/ConsumerImpl.java

pulsar-common/src/main/java/org/apache/pulsar/common/protocol/Commands.java

pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ServerCnx.java

pulsar-broker/src/test/java/org/apache/pulsar/client/api/SimpleSchemaTest.java

…O. Reduce the duplication of convertAutoConsumeSchema code and reuse convertSchema.

…hemaType(SchemaType type), and delete the judgment of schema as AutoConsume in ServerCnx.

eolivelli

+1

Denovo1998 · 2023-02-08T14:53:48Z

This is my first contribution. Maybe now I need more review and approval? Or add a ready-to-test label to the PR? In my own forked repository, all the tests are already passing.

congbobo184 · 2023-02-09T01:12:02Z

@liangyepianzhou please help review this PR

codelipenghui · 2023-02-22T02:53:27Z

/pulsarbot run-failure-checks

Denovo1998 · 2023-02-28T11:03:56Z

/pulsarbot run-failure-checks

congbobo184 · 2023-02-28T11:32:40Z

@Denovo1998 hi, could you merge apache/master and rerun the test?

Denovo1998 · 2023-02-28T15:33:34Z

@congbobo184 I'm done. But in this pr I see 2 workflows awaiting approval.

tisonkun · 2023-03-02T11:21:31Z

@Denovo1998 I've triggered the CI workflow for you :)

Denovo1998 · 2023-03-02T11:30:03Z

@tisonkun Thanks for your help.

Denovo1998 · 2023-03-02T12:42:22Z

/pulsarbot run-failure-checks

Denovo1998 · 2023-03-02T14:08:54Z

/pulsarbot run-failure-checks

Denovo1998 · 2023-03-02T15:03:32Z

That's weird. I ran this test a few times and sometimes it worked and sometimes it failed.
org.apache.pulsar.client.api.TopicReaderTest#testMultiReaderIsAbleToSeekWithTimeOnMiddleOfTopic


Expected :true
Actual   :false
<Click to see difference>

java.lang.AssertionError: Received duplicate message msg num 6
	at org.testng.Assert.fail(Assert.java:110)
	at org.testng.Assert.failNotEquals(Assert.java:1413)
	at org.testng.Assert.assertTrue(Assert.java:56)
	at org.apache.pulsar.client.api.TopicReaderTest.testMultiReaderIsAbleToSeekWithTimeOnMiddleOfTopic(TopicReaderTest.java:1401)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:568)
	at org.testng.internal.invokers.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:139)
	at org.testng.internal.invokers.TestInvoker.invokeMethod(TestInvoker.java:677)
	at org.testng.internal.invokers.TestInvoker.invokeTestMethod(TestInvoker.java:221)
	at org.testng.internal.invokers.MethodRunner.runInSequence(MethodRunner.java:50)
	at org.testng.internal.invokers.TestInvoker$MethodInvocationAgent.invoke(TestInvoker.java:969)
	at org.testng.internal.invokers.TestInvoker.invokeTestMethods(TestInvoker.java:194)
	at org.testng.internal.invokers.TestMethodWorker.invokeTestMethods(TestMethodWorker.java:148)
	at org.testng.internal.invokers.TestMethodWorker.run(TestMethodWorker.java:128)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at org.testng.TestRunner.privateRun(TestRunner.java:829)
	at org.testng.TestRunner.run(TestRunner.java:602)
	at org.testng.SuiteRunner.runTest(SuiteRunner.java:437)
	at org.testng.SuiteRunner.runSequentially(SuiteRunner.java:431)
	at org.testng.SuiteRunner.privateRun(SuiteRunner.java:391)
	at org.testng.SuiteRunner.run(SuiteRunner.java:330)
	at org.testng.SuiteRunnerWorker.runSuite(SuiteRunnerWorker.java:52)
	at org.testng.SuiteRunnerWorker.run(SuiteRunnerWorker.java:95)
	at org.testng.TestNG.runSuitesSequentially(TestNG.java:1256)
	at org.testng.TestNG.runSuitesLocally(TestNG.java:1176)
	at org.testng.TestNG.runSuites(TestNG.java:1099)
	at org.testng.TestNG.run(TestNG.java:1067)
	at com.intellij.rt.testng.IDEARemoteTestNG.run(IDEARemoteTestNG.java:66)
	at com.intellij.rt.testng.RemoteTestNGStarter.main(RemoteTestNGStarter.java:109)

I found some issues and pr that might be related.
#17407
#17443
#18202

Do I need to continue running `run-fail-checks`` for the test to succeed?

Denovo1998 · 2023-03-07T03:00:36Z

/pulsarbot run-failure-checks

Denovo1998 · 2023-03-07T04:56:12Z

@congbobo184 Now we can merge.

Fix the problem that using schema to create consumer fails after usin…

cf12237

…g AUTO_CONSUME consumer to subscribe topic

github-actions bot added the doc-not-needed Your PR changes do not impact docs label Sep 3, 2022

Denovo1998 closed this Sep 3, 2022

Denovo1998 deleted the issue17354 branch September 3, 2022 16:32

Denovo1998 restored the issue17354 branch September 3, 2022 16:32

Denovo1998 reopened this Sep 3, 2022

Denovo1998 changed the title ~~Fix using schema to create consumer fails after using AUTO_CONSUME consumer to subscribe topic~~ [fix][broker]Fix using schema to create consumer fails after using AUTO_CONSUME consumer to subscribe topic Sep 3, 2022

Denovo1998 mentioned this pull request Sep 3, 2022

[Bug] Subscription with schema fails if there is an AUTO-CONSUME consumer attached to a topic #17354

Closed

2 tasks

Merge branch 'master' of github.com:apache/pulsar into issue17354

74f23b4

Denovo1998 force-pushed the issue17354 branch from 67220fe to 74f23b4 Compare September 4, 2022 05:29

Fix the problem that using schema to create consumer fails after usin…

d1c40ff

…g AUTO_CONSUME consumer to subscribe topic

Denovo1998 force-pushed the issue17354 branch from 74f23b4 to d1c40ff Compare September 6, 2022 12:01

Denovo1998 added 2 commits September 6, 2022 20:03

Merge remote-tracking branch 'origin/issue17354' into issue17354

dc602ef

add test for Fix the problem that using schema to create consumer fai…

22d6a06

…ls after using AUTO_CONSUME consumer to subscribe topic

Technoboy- force-pushed the issue17354 branch from 22d6a06 to 37a48e4 Compare September 8, 2022 02:38

Denovo1998 added 2 commits September 9, 2022 10:23

Fix the problem that using schema to create consumer fails after usin…

81847df

…g AUTO_CONSUME consumer to subscribe topic

add test for Fix the problem that using schema to create consumer fai…

350bef1

…ls after using AUTO_CONSUME consumer to subscribe topic

Denovo1998 force-pushed the issue17354 branch from 37a48e4 to 350bef1 Compare September 9, 2022 02:23

Technoboy- assigned Denovo1998 Sep 28, 2022

Technoboy- added this to the 2.12.0 milestone Sep 28, 2022

Technoboy- requested review from congbobo184 and mattisonchao September 28, 2022 01:41

congbobo184 requested review from liangyepianzhou, codelipenghui and gaoran10 October 18, 2022 07:42

eolivelli reviewed Jan 31, 2023

View reviewed changes

pulsar-client/src/main/java/org/apache/pulsar/client/impl/schema/AutoConsumeSchema.java Outdated Show resolved Hide resolved

congbobo184 reviewed Feb 1, 2023

View reviewed changes

pulsar-client/src/main/java/org/apache/pulsar/client/impl/ConsumerImpl.java Show resolved Hide resolved

pulsar-common/src/main/java/org/apache/pulsar/common/protocol/Commands.java Outdated Show resolved Hide resolved

BewareMyPower requested changes Feb 1, 2023

View reviewed changes

pulsar-common/src/main/java/org/apache/pulsar/common/protocol/Commands.java Outdated Show resolved Hide resolved

BewareMyPower reviewed Feb 1, 2023

View reviewed changes

pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ServerCnx.java Outdated Show resolved Hide resolved

BewareMyPower reviewed Feb 1, 2023

View reviewed changes

pulsar-broker/src/test/java/org/apache/pulsar/client/api/SimpleSchemaTest.java Show resolved Hide resolved

Denovo1998 added 3 commits February 1, 2023 16:55

change SCHEMA_INFO public and reference AutoConsumerSchema.SCHEMA_INF…

276b462

…O. Reduce the duplication of convertAutoConsumeSchema code and reuse convertSchema.

delete convertAutoConsumeSchema and set AutoConsume in getSchemaType.

4cc9c46

fix SchemaType getSchemaType(Schema.Type type) like Schema.Type getSc…

0b92863

…hemaType(SchemaType type), and delete the judgment of schema as AutoConsume in ServerCnx.

eolivelli approved these changes Feb 1, 2023

View reviewed changes

BewareMyPower approved these changes Feb 2, 2023

View reviewed changes

congbobo184 approved these changes Feb 9, 2023

View reviewed changes

liangyepianzhou approved these changes Feb 21, 2023

View reviewed changes

codelipenghui approved these changes Feb 22, 2023

View reviewed changes

codelipenghui added the ready-to-test label Feb 22, 2023

Merge branch 'master' into issue17354

ba186d7

congbobo184 merged commit af1360f into apache:master Mar 7, 2023

Denovo1998 deleted the issue17354 branch March 18, 2023 10:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[PIP-236][fix][broker]Fix using schema to create consumer fails after using AUTO_CONSUME consumer to subscribe topic #17449

[PIP-236][fix][broker]Fix using schema to create consumer fails after using AUTO_CONSUME consumer to subscribe topic #17449

Denovo1998 commented Sep 3, 2022 •

edited

Loading

Technoboy- commented Sep 6, 2022

Denovo1998 commented Sep 6, 2022

Denovo1998 commented Oct 2, 2022

Denovo1998 commented Oct 18, 2022

Denovo1998 commented Nov 1, 2022

eolivelli left a comment

Denovo1998 commented Feb 8, 2023

congbobo184 commented Feb 9, 2023 •

edited by liangyepianzhou

Loading

codelipenghui commented Feb 22, 2023

Denovo1998 commented Feb 28, 2023

congbobo184 commented Feb 28, 2023 •

edited

Loading

Denovo1998 commented Feb 28, 2023

tisonkun commented Mar 2, 2023

Denovo1998 commented Mar 2, 2023

Denovo1998 commented Mar 2, 2023

Denovo1998 commented Mar 2, 2023

Denovo1998 commented Mar 2, 2023

Denovo1998 commented Mar 7, 2023

Denovo1998 commented Mar 7, 2023

[PIP-236][fix][broker]Fix using schema to create consumer fails after using AUTO_CONSUME consumer to subscribe topic #17449

[PIP-236][fix][broker]Fix using schema to create consumer fails after using AUTO_CONSUME consumer to subscribe topic #17449

Conversation

Denovo1998 commented Sep 3, 2022 • edited Loading

Motivation

Modifications

Verifying this change

Does this pull request potentially affect one of the following parts:

Documentation

Matching PR in forked repository

Technoboy- commented Sep 6, 2022

Denovo1998 commented Sep 6, 2022

Denovo1998 commented Oct 2, 2022

Denovo1998 commented Oct 18, 2022

Denovo1998 commented Nov 1, 2022

eolivelli left a comment

Choose a reason for hiding this comment

Denovo1998 commented Feb 8, 2023

congbobo184 commented Feb 9, 2023 • edited by liangyepianzhou Loading

codelipenghui commented Feb 22, 2023

Denovo1998 commented Feb 28, 2023

congbobo184 commented Feb 28, 2023 • edited Loading

Denovo1998 commented Feb 28, 2023

tisonkun commented Mar 2, 2023

Denovo1998 commented Mar 2, 2023

Denovo1998 commented Mar 2, 2023

Denovo1998 commented Mar 2, 2023

Denovo1998 commented Mar 2, 2023

Denovo1998 commented Mar 7, 2023

Denovo1998 commented Mar 7, 2023

Denovo1998 commented Sep 3, 2022 •

edited

Loading

congbobo184 commented Feb 9, 2023 •

edited by liangyepianzhou

Loading

congbobo184 commented Feb 28, 2023 •

edited

Loading