[I] Change broker exception from TopicBacklogQuotaExceededException to TopicBlockQuotaExceededException [pulsar]

["geniusjoe (via GitHub)" <gi...@apache.org>]

geniusjoe opened a new issue, #25168:
URL: https://github.com/apache/pulsar/issues/25168

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


-- 
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.apache.org

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

Re: [I] Change broker exception from TopicBacklogQuotaExceededException to TopicBlockQuotaExceededException [pulsar]

["merlimat (via GitHub)" <gi...@apache.org>]

merlimat closed issue #25168: Change broker exception from TopicBacklogQuotaExceededException to TopicBlockQuotaExceededException
URL: https://github.com/apache/pulsar/issues/25168


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

Re: [I] Change broker exception from TopicBacklogQuotaExceededException to TopicBlockQuotaExceededException [pulsar]

["merlimat (via GitHub)" <gi...@apache.org>]

merlimat commented on issue #25168:
URL: https://github.com/apache/pulsar/issues/25168#issuecomment-6388676924

   Thanks @geniusjoe. The protocol names `ProducerBlockedQuotaExceededError`/`Exception` can't change for compatibility, as you note. The broker's `TopicBacklogQuotaExceededException` is the more accurate name for the condition, though, so renaming it to match the wire name would only move the inconsistency to the broker. A clearer improvement would be to make the error message the client receives mention the backlog quota and the retention policy, which makes it searchable on both sides. Closing as not planned; feel free to send a PR for the message, or reopen if there's a case this doesn't cover.


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