apache / apache/pulsar

Change broker exception from TopicBacklogQuotaExceededException to TopicBlockQuotaExceededException

Open
#25,168 0 comments 0 reactions 0 assignees View on GitHub
type/enhancement
Dominant language
Java
Stars
15.3k
Forks
3.8k
Avg merge
1d 14h
Merged PRs (30d)
160

Description

### Search before reporting

- [x] I searched in the [issues](https://github.com/apache/pulsar/issues) and found nothing similar.

### Motivation

Based on the code, there appears to be a typo previously: when the server throws `ProducerBacklogQuotaExceededError`, the exception actually generated in `PulsarApi.proto` is `ProducerBlockedQuotaExceededException`.
This makes it difficult for developers to locate key information from the server-side code when the client receives the related exception. To keep consistency between server and client error reporting, maybe we should update the server-side exception to `TopicBlockQuotaExceededException` as well.

https://github.com/apache/pulsar/blob/85625e0f100479dd95fb1311aeb52411b6b0a25d/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ServerCnx.java#L1804
```Java
if (cause instanceof BrokerServiceException.TopicBacklogQuotaExceededException) {
BrokerServiceException.TopicBacklogQuotaExceededException tbqe =
(BrokerServiceException.TopicBacklogQuotaExceededException) cause;
IllegalStateException illegalStateException = new IllegalStateException(tbqe);
BacklogQuota.RetentionPolicy retentionPolicy = tbqe.getRetentionPolicy();
if (producerFuture.completeExceptionally(illegalStateException)) {
if (retentionPolicy == BacklogQuota.RetentionPolicy.producer_request_hold) {
commandSender.sendErrorResponse(requestId,
ServerError.ProducerBlockedQuotaExceededError,
illegalStateException.getMessage());
} else if (retentionPolicy == BacklogQuota.RetentionPolicy.producer_exception) {
commandSender.sendErrorResponse(requestId,
ServerError.ProducerBlockedQuotaExceededException,
illegalStateException.getMessage());
```

### Solution

Update the server-side exception to `TopicBlockQuotaExceededException` as well.

### Alternatives

We could also completely resolve this issue by changing both the pb and client exceptions to `ProducerBacklogQuotaExceededError`. However, due to compatibility considerations between lower versions of the client and pb, this modification would have a significant impact on the client, potentially causing incompatibility between older client versions and newer broker versions.

### Anything else?

_No response_

### Are you willing to submit a PR?

- [x] I'm willing to submit a PR!

Contributor guide

Open the contributing guide

Research direction

Start in pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ServerCnx.java at the producer quota exception handling near line 1804, then compare the related names in PulsarApi.proto. The work is complete when the server-side exception name matches the client/protocol naming without changing the compatibility-sensitive error behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
backend, distributed-systems
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.