feat(function): expose input_specs for per-topic consumer config - #219
Conversation
pulsar_function had no way to set Pulsar's inputSpecs, which made receiverQueueSize unreachable from Terraform. It defaults to 1000, so a Shared-subscription function at high parallelism buffers that many messages per instance, and the only way to tune it was pulsarctl - outside the user's Terraform, where the next apply can neither see nor preserve the change. Add an input_specs block mirroring the one pulsar_sink already ships (key, schema_type, serde_class_name, is_regex_pattern, receiver_queue_size), plus pool_messages, schema_properties and consumer_properties. Unlike the sink block, only key is required. Three behaviours in Pulsar's FunctionConfigUtils shape the implementation: - The read path never returns inputs. convertFromDetails() emits a ConsumerConfig for every input topic - including ones declared through inputs and the topics_pattern entry - and only calls setInputSpecs(). Mirroring the response into state would invent input_specs blocks for configurations that never wrote one, so specs already represented by inputs or topics_pattern are skipped. - inputs clobbers inputSpecs on update. validateUpdate() folds inputs into the inputSpecs map with a fresh ConsumerConfig before iterating it, so a topic listed in both would lose its consumer settings on every apply. Topics carried by input_specs are stripped from inputs on the wire. - The topic set and isRegexPattern are immutable but everything else is not, so receiver_queue_size updates in place. CustomizeDiff forces replacement only when the effective topic set or a regex flag changes, which keeps adopting input_specs for a topic already in inputs an in-place update rather than a recreate. Nested ForceNew is deliberately absent: on a TypeSet it would rehash the element on a queue-size change and replace the function on the very edit this enables. The set-level ForceNew alone is not enough either, since diffSet only carries it through the .# count attribute, so the changed nested attribute is flagged as well. Fixes streamnative#217 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
b12100d to
0538adb
Compare
freeznet
left a comment
There was a problem hiding this comment.
Blocking correctness issues remain against the repository's Pulsar 4.0.3 acceptance baseline and the pinned pulsar-client-go model. The core approach and the tests for plain inputs overlap and in-place queue-size updates are good, but the other legacy input forms and several newly exposed values do not yet round-trip safely. Please address the inline findings and add focused regression or acceptance coverage.
|
apache/pulsar-client-go#1529 need this fix, once it get merged, will change the go mod to upstream master |
|
@freeznet quick question on the This is driving a customer PoC where Terraform is a hard requirement rather than a convenience: they manage Pulsar entirely as code, so This PR is approved and green, and the only thing holding it is the Would you be open to landing this without the explicit-zero support, and adding it back once #1529 is released upstream? The coupling is small — two call sites ( Entirely your call, and I am not trying to rush your fix — if you think #1529 will be reviewed shortly then waiting is cleaner and I would rather not churn your work for a few days. Mostly I want to avoid a provider release depending on a personal fork, and decoupling the two timelines seemed like the way to do that. Either way, thanks for the review here — the |
|
@freeznet — since you reviewed this one, a short summary of what else is now open here, so it is one notification rather than six. All are green and mergeable; none is urgent. Blocked on upstream, not on review:
Ready to review:
If you only have time for one, #221 is the smallest and the most security-relevant. |
Fixes #217
Motivation
pulsar_functionsupportsinputs,custom_serde_inputs,custom_schema_inputsand friends, but there is no way to set Pulsar'sinputSpecs— the per-topic consumer configuration. The practical consequence is thatreceiverQueueSizeis unreachable from Terraform. It defaults to 1000, so a Shared-subscription function at high parallelism buffers up to that many messages per instance (at 15 instances, up to 15,000 messages parked in memory). The only way to change it today ispulsarctl, i.e. outside the user's Terraform, where the nextapplycan neither see nor preserve the change.utils.FunctionConfig.InputSpecsandutils.ConsumerConfigare already present in the admin client the provider depends on — only the schema and its marshal/unmarshal were missing.Modifications
Adds an
input_specsblock topulsar_function, deliberately mirroring the blockpulsar_sinkalready ships so the two resources read the same:key,schema_type,serde_class_name,is_regex_pattern,receiver_queue_size, pluspool_messages,schema_propertiesandconsumer_properties. Unlike the sink block, onlykeyisRequired(the sink's all-Requiredblock is #218, not touched here).crypto_configis intentionally out of scope.Three behaviours in
FunctionConfigUtils.javashape the implementation, and each is worth a reviewer's attention:1. The read path never returns
inputs.convertFromDetails()builds aConsumerConfigfor every input topic — including topics declared throughinputs, and thetopics_patternentry as a spec withisRegexPattern=true— and only callssetInputSpecs(). Mirroring the response into state would inventinput_specsblocks for configurations that never wrote one, producing a diff on every plan forever. So a returned spec is skipped wheninputsortopics_patternalready represents that topic, unless the configuration also declares it ininput_specs(in which case it is genuinely the user's and is refreshed rather than dropped).2.
inputsclobbersinputSpecson update. InvalidateUpdate(), each entry ofnewConfig.getInputs()is written intonewConfig.getInputSpecs()with a freshConsumerConfigbefore that map is iterated, so a topic listed in both would silently lose itsreceiverQueueSizeon every apply. (convert()appliesinputSpecslast on create, so the two paths disagree.) Topics carried by aninput_specsblock are therefore stripped frominputson the wire.3. The topic set and
isRegexPatternare immutable, everything else is not.validateUpdate()throws"Input Topics cannot be altered"and rejectsisRegexPatternchanges, but then doesmergedConfig.getInputSpecs().put(topicName, consumerConfig)— soreceiver_queue_sizegenuinely updates in place.CustomizeDiffmirrors exactly that rule, comparing the effective topic set (the union ofinputs,topics_patternandinput_specs). This matters for adoption: moving a topic thatinputsalready declares into aninput_specsblock leaves that set untouched and stays an in-place update, rather than destroying the user's function.Two SDK details are called out in comments because they are easy to reintroduce:
ForceNewis deliberately absent. On aTypeSet, changingreceiver_queue_sizerehashes the element, which the SDK reads as a removal plus an addition — a nestedForceNewwould replace the function on the very edit this feature exists to enable.diff.ForceNew()is not sufficient on its own.schemaMap.diffSetonly carries it into the diff through theinput_specs.#count attribute, which it emits solely when the element count changes; renaming a topic or flippingis_regex_patternkeeps the count identical and the replacement would be silently dropped. The changed nested attribute is flagged as well.Verifying this change
This change added tests and can be verified as follows:
inputsoverlap strip and empty-map omission), unmarshal (theinputs-covered skip, the both-places case, and the import case), and the effective-topic-set computation.TestFunctionInputSpecsForceNew— a table-driven test running the realschema.Resource.Diffover six scenarios: adoptinginput_specsfor a topic already ininputsand tuningreceiver_queue_sizemust not replace; adding, renaming, dropping a topic and flippingis_regex_patternmust. The two in-place cases additionally assert the planned value actually reaches the diff, so a change the SDK dropped entirely cannot pass as "no replacement needed".TestFunction(acceptance) — extended with a second step raisingreceiver_queue_sizefrom 100 to 250, asserting the broker reports the new value, thatschema_typesurvived (which fails if the topic was left ininputs, per finding 2), and that the resource ID is unchanged so the update was in place rather than a recreate. Run against Pulsar in Docker,-count 3.TestFunctionLegacyInputsOnly(acceptance) — a regression guard that a function configured only withinputsdoes not drift, sinceresource.Testfails a step whose post-apply plan is non-empty. This is the main risk to existing users, per finding 1.One side effect worth noting: import fidelity improves.
inputswas already never populated on import because the API does not return it, so imported functions now capture their input topics asinput_specsblocks instead of losing them entirely.Documentation
Check the box below.
Need to update docs?
doc-requiredno-need-docdocdocs/resources/function.mdregenerated withgo generate ./...; the diff there is purely additive.