[PR] fix(pulsaradmin): preserve explicit zero receiver queue size [pulsar-client-go]

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

freeznet opened a new pull request, #1529:
URL: https://github.com/apache/pulsar-client-go/pull/1529

   <!--
   ### Contribution Checklist
     
     - Name the pull request in the form "[Issue XYZ][component] Title of the pull request", where *XYZ* should be replaced by the actual issue number.
       Skip *Issue XYZ* if there is no associated github issue for this pull request.
       Skip *component* if you are unsure about which is the best component. E.g. `[docs] Fix typo in produce method`.
   
     - Fill out the template below to describe the changes contributed by the pull request. That will give reviewers the context they need to do the review.
     
     - Each pull request should address only one issue, not mix up code from multiple issues.
     
     - Each commit in the pull request has a meaningful commit message
   
     - Once all items of the checklist are addressed, remove the above text and this checklist, leaving only the filled out template below.
   
   **(The sections below can be removed for hotfixes of typos)**
   -->
   
   *(If this PR fixes a github issue, please add `Fixes #<xyz>`.)*
   
   Fixes #<xyz>
   
   *(or if this PR is one task of a github issue, please add `Master Issue: #<xyz>` to link to the master issue.)*
   
   Master Issue: #<xyz>
   
   ### Motivation
   
   
   *Explain here the context, and why you're making that change. What is the problem you're trying to solve.*
   
   ### Modifications
   
   *Describe the modifications you've done.*
   
   ### Verifying this change
   
   - [ ] Make sure that the change passes the CI checks.
   
   *(Please pick either of the following options)*
   
   This change is a trivial rework / code cleanup without any test coverage.
   
   *(or)*
   
   This change is already covered by existing tests, such as *(please describe tests)*.
   
   *(or)*
   
   This change added tests and can be verified as follows:
   
   *(example:)*
     - *Added integration tests for end-to-end deployment with large payloads (10MB)*
     - *Extended integration test for recovery after broker failure*
   
   ### Does this pull request potentially affect one of the following parts:
   
   *If `yes` was chosen, please highlight the changes*
   
     - Dependencies (does it add or upgrade a dependency): (yes / no)
     - The public API: (yes / no)
     - The schema: (yes / no / don't know)
     - The default values of configurations: (yes / no)
     - The wire protocol: (yes / no)
   
   ### Documentation
   
     - Does this pull request introduce a new feature? (yes / no)
     - If yes, how is the feature documented? (not applicable / docs / GoDocs / not documented)
     - If a feature is not applicable for documentation, explain why?
     - If a feature is not documented yet in this PR, please create a followup issue for adding the documentation
   


-- 
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: [PR] fix(pulsaradmin): preserve explicit zero receiver queue size [pulsar-client-go]

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

freeznet merged PR #1529:
URL: https://github.com/apache/pulsar-client-go/pull/1529


-- 
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: [PR] fix(pulsaradmin): preserve explicit zero receiver queue size [pulsar-client-go]

["david-streamlio (via GitHub)" <gi...@apache.org>]

david-streamlio commented on PR #1529:
URL: https://github.com/apache/pulsar-client-go/pull/1529#issuecomment-5358058121

   Thanks for putting this together — I verified the change end to end against Pulsar 4.0.3 while reviewing the downstream Terraform PR (streamnative/terraform-provider-pulsar#219), and it does exactly what is needed: an explicitly configured `receiverQueueSize: 0` now round-trips with `HasReceiverQueueSize() == true` and value `0`, where `omitempty` previously dropped it and the broker silently applied the 1000 default.
   
   I had filed #1528 for this before seeing your PR. Since the template body here is still unfilled, it may be worth adding `Fixes #1528` and some motivation so a reviewer has the context without reconstructing it — offered as a starting point, edit or ignore as you see fit:
   
   > ### Motivation
   >
   > `ConsumerConfig.ReceiverQueueSize` is an `int` tagged `json:"receiverQueueSize,omitempty"`, so an explicitly configured zero is indistinguishable from an unset field and is omitted from the request. Pulsar treats a missing value as "use the default" and applies 1000.
   >
   > Zero is a meaningful value: Pulsar validates `receiverQueueSize >= 0` (`FunctionConfigUtils.doCommonChecks`), and zero disables consumer prefetch, which is what latency-sensitive callers want. The Java admin API models the field as a nullable `Integer` precisely so unset and zero stay distinct; the Go model cannot currently express that.
   >
   > This blocks infrastructure-as-code callers in particular — the Terraform provider (streamnative/terraform-provider-pulsar#219) has to pin a fork to expose the setting at all.
   >
   > ### Modifications
   >
   > Tag `ReceiverQueueSize` as `json:"-"` and track explicitness in an unexported `receiverQueueSizeSet` flag, with `MarshalJSON`/`UnmarshalJSON` mapping it to a `*int` on the wire, plus `SetReceiverQueueSize()` and `HasReceiverQueueSize()`.
   >
   > The exported field stays an `int`, so existing callers that assign a non-zero value compile and behave exactly as before; `HasReceiverQueueSize()` falls back to `ReceiverQueueSize != 0` for values assigned directly rather than through the setter.
   
   One question on the round trip, which is the only thing I could not settle by reading: `UnmarshalJSON` resets `receiverQueueSizeSet` to false and re-derives it from the payload, so a server response omitting `receiverQueueSize` yields `HasReceiverQueueSize() == false`. That is right for a fresh decode. Is a decode ever performed onto a struct a caller has already populated — where clearing a previously-set flag would lose the distinction? I could not find such a path in `pulsaradmin`, so I suspect the answer is no and this is purely defensive; flagging it only because it is the kind of thing that is easier to confirm now than to debug later.
   
   Happy to help with the downstream bump in the provider once this lands and there is a tagged `apache/pulsar-client-go` version to depend on, so the `replace` directive in #219 can come out.
   


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