Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix broker return error code confuse when not set subscription name. #289

Merged
merged 1 commit into from
Jun 21, 2023

Conversation

shibd
Copy link
Member

@shibd shibd commented Jun 21, 2023

Motivation

When creating a consumer and not set a subscription name, users will receive a ResutlProducerBusy result, which is confusing.

2023-06-19 08:48:54.412 INFO  [0x7ff84d51c640] Client:90 | Subscribing on Topic :persistent://public/default/test-1
2023-06-19 08:48:54.416 INFO  [0x7ff84d51c640] ClientConnection:184 | [<none> -> pulsar+ssl://v2-10-o.chaos-platform.test.sn2.dev:6651] Create ClientConnection, timeout=10000
2023-06-19 08:48:59.379 INFO  [0x7ff84d51c640] ConnectionPool:106 | Created connection for pulsar+ssl://v2-10-o.chaos-platform.test.sn2.dev:6651
2023-06-19 08:48:59.381 INFO  [0x70000011e000] ClientConnection:382 | [198.18.0.1:52214 -> 198.18.2.113:6651] Connected to broker
2023-06-19 08:49:01.687 INFO  [0x70000011e000] HandlerBase:72 | [0x600001d1a628, , 0] Getting connection from pool
2023-06-19 08:49:01.937 INFO  [0x70000011e000] ClientConnection:184 | [<none> -> pulsar+ssl://v2-10-o.chaos-platform.test.sn2.dev:6651] Create ClientConnection, timeout=10000
2023-06-19 08:49:01.955 INFO  [0x70000011e000] ConnectionPool:106 | Created connection for pulsar://v2-10-o-broker-1.v2-10-o-broker-headless.chaos-platform.svc.cluster.local:6650
2023-06-19 08:49:01.956 INFO  [0x70000011e000] ClientConnection:384 | [198.18.0.1:52222 -> 198.18.2.113:6651] Connected to broker through proxy. Logical broker: pulsar://v2-10-o-broker-1.v2-10-o-broker-headless.chaos-platform.svc.cluster.local:6650
2023-06-19 08:49:03.534 WARN  [0x70000011e000] ClientConnection:1597 | [198.18.0.1:52222 -> 198.18.2.113:6651] Received error response from server: ProducerBusy (Empty subscription name) -- req_id: 0
2023-06-19 08:49:03.534 ERROR [0x70000011e000] ConsumerImpl:325 | [0x600001d1a628, , 0] Failed to create consumer: ProducerBusy
2023-06-19 08:49:03.534 ERROR [0x70000011e000] MultiTopicsConsumerImpl:138 | Failed when subscribed to topic persistent://public/default/test-1 in TopicsConsumer. Error - ProducerBusy
2023-06-19 08:49:03.534 ERROR [0x70000011e000] MultiTopicsConsumerImpl:149 | Unable to create Consumer - [Muti Topics Consumer: TopicName - 0x600001d2a748 - Subscription - ] Error - ProducerBusy
2023-06-19 08:49:03.535 WARN  [0x70000011e000] ConsumerImpl:1213 | [0x600001d1a628, , 0] Failed to close consumer: AlreadyClosed
2023-06-19 08:49:03.535 ERROR [0x70000011e000] MultiTopicsConsumerImpl:502 | Closing the consumer failed for partition - persistent://public/default/test-1-partition-0 with error - AlreadyClosed
2023-06-19 08:49:03.535 WARN  [0x70000011e000] MultiTopicsConsumerImpl:462 | [Muti Topics Consumer: TopicName - 0x600001d2a748 - Subscription - ]Failed to close consumer: AlreadyClosed
2023-06-19 08:49:03.535 ERROR [0x70000011e000] MultiTopicsConsumerImpl:299 | Unable to create Consumer - [Muti Topics Consumer: TopicName - 0x600001d2a748 - Subscription - ] Error - ProducerBusy
libc++abi: terminating due to uncaught exception of type std::runtime_error: Error creating consumer: 19

The root cause is broker returns the error code ProducerBusy on this scene.

https://github.com/apache/pulsar/blob/cd2aa550d0fe4e72b5ff88c4f6c1c2795b3ff2cd/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerServiceException.java#L240-L241

Modifications

  • On the cpp client side, if receive a ProducerBusy error, trans it to ResultInvalidConfiguration.

Verifying this change

  • Add testNotSetSubscriptionName unit test to cover it.

Documentation

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

@shibd shibd added this to the 3.3.0 milestone Jun 21, 2023
@shibd shibd self-assigned this Jun 21, 2023
Copy link
Contributor

@BewareMyPower BewareMyPower left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch!

@BewareMyPower BewareMyPower merged commit 0202218 into apache:main Jun 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants