Skip to content

Commit 810bfee

Browse files
[fix][fn] Honour negativeAckRedeliveryDelayMs in the Python function runtime
The Python runtime negatively acknowledges on failure but never configured the redelivery delay, so the client default of 60 seconds applied regardless of what SourceSpec.negativeAckRedeliveryDelayMs carried. A function configured for fast retry, or for a long back-off from a struggling downstream, silently got neither. Add get_negative_ack_args() and splat its result into all three subscribe() call sites. Two details drive the shape: - The field is a proto3 scalar with no presence, so an unset value reads as 0. Only a positive value is forwarded, leaving the client default in place otherwise - the same guard JavaInstanceRunnable applies. Sending 0 through would mean immediate redelivery rather than the default. - The argument is omitted rather than passed as None. subscribe() validates it with _check_type(int, ...) and not _check_type_or_none, so None would raise for every function that does not configure it, unlike the neighbouring unacked_messages_timeout_ms which does accept None. Returning a dict to splat rather than a value keeps that omission at one site, since the first call site passes explicit keywords while the other two build a consumer_args dict. Fixes #26411 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 3be684a commit 810bfee

2 files changed

Lines changed: 61 additions & 1 deletion

File tree

pulsar-functions/instance/src/main/python/python_instance.py

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,8 @@ def run(self):
150150
elif self.instance_config.function_details.retainKeyOrdering:
151151
mode = pulsar._pulsar.ConsumerType.KeyShared
152152

153+
nack_args = self.get_negative_ack_args()
154+
153155
position = pulsar._pulsar.InitialPosition.Latest
154156
if self.instance_config.function_details.source.subscriptionPosition == Function_pb2.SubscriptionPosition.Value("EARLIEST"):
155157
position = pulsar._pulsar.InitialPosition.Earliest
@@ -181,7 +183,8 @@ def run(self):
181183
message_listener=partial(self.message_listener, self.input_serdes[topic], DEFAULT_SCHEMA),
182184
unacked_messages_timeout_ms=int(self.timeout_ms) if self.timeout_ms else None,
183185
initial_position=position,
184-
properties=properties
186+
properties=properties,
187+
**nack_args
185188
)
186189

187190
for topic, consumer_conf in self.instance_config.function_details.source.inputSpecs.items():
@@ -207,6 +210,7 @@ def run(self):
207210
"properties": properties,
208211
"crypto_key_reader": crypto_key_reader
209212
}
213+
consumer_args.update(nack_args)
210214
if consumer_conf.HasField("receiverQueueSize"):
211215
consumer_args["receiver_queue_size"] = consumer_conf.receiverQueueSize.value
212216

@@ -584,6 +588,26 @@ def get_record_class(self, class_name):
584588
except:
585589
pass
586590
return record_kclass
591+
def get_negative_ack_args(self):
592+
"""Build the negative-ack redelivery delay argument for Client.subscribe().
593+
594+
Returns a dict to splat into the subscribe() call: either empty, or carrying
595+
negative_ack_redelivery_delay_ms.
596+
597+
SourceSpec.negativeAckRedeliveryDelayMs is a proto3 scalar with no presence, so an unset field
598+
reads as 0. Only a positive value is forwarded, leaving the client default (60s) in place
599+
otherwise - the same guard the Java runtime applies in JavaInstanceRunnable.
600+
601+
The argument is omitted rather than passed as None because subscribe() validates it with
602+
_check_type(int, ...) rather than _check_type_or_none, so None would fail for every function
603+
that does not configure it.
604+
"""
605+
delay_ms = self.instance_config.function_details.source.negativeAckRedeliveryDelayMs
606+
if delay_ms <= 0:
607+
return {}
608+
609+
return {"negative_ack_redelivery_delay_ms": delay_ms}
610+
587611
def get_crypto_reader(self, crypto_spec):
588612
crypto_key_reader = None
589613
if crypto_spec is not None:

pulsar-functions/instance/src/test/python/test_python_instance.py

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,3 +149,39 @@ def test_do_not_forward_properties(self):
149149
self.assertNotIn("custom-key", kwargs['properties'])
150150
self.assertIn("__pfn_input_topic__", kwargs['properties'])
151151

152+
153+
class TestNegativeAckRedeliveryDelay(unittest.TestCase):
154+
"""Covers SourceSpec.negativeAckRedeliveryDelayMs reaching the consumer.
155+
156+
The runtime negatively acknowledges on failure but never configured the delay, so the client
157+
default of 60s always applied. The Java runtime guards on > 0 in JavaInstanceRunnable.
158+
"""
159+
160+
def _instance(self, delay_ms=None):
161+
function_details = Function_pb2.FunctionDetails()
162+
function_details.sink.topic = "test_sink_topic"
163+
if delay_ms is not None:
164+
function_details.source.negativeAckRedeliveryDelayMs = delay_ms
165+
166+
return PythonInstance('test_instance', 'test_func', '1.0', function_details, 100, 30,
167+
'user_code', Mock(), Mock(), 'test_cluster', 'test_url', None)
168+
169+
def test_positive_delay_is_forwarded(self):
170+
args = self._instance(delay_ms=5000).get_negative_ack_args()
171+
self.assertEqual({"negative_ack_redelivery_delay_ms": 5000}, args)
172+
173+
def test_unset_delay_is_omitted(self):
174+
# proto3 scalar with no presence: unset reads as 0. The argument must be omitted rather than
175+
# sent - subscribe() validates it with _check_type(int), so None would fail for every function
176+
# that does not set it, and 0 would mean immediate redelivery instead of the 60s default.
177+
self.assertEqual({}, self._instance().get_negative_ack_args())
178+
179+
def test_explicit_zero_is_omitted(self):
180+
self.assertEqual({}, self._instance(delay_ms=0).get_negative_ack_args())
181+
182+
def test_result_is_splattable_into_subscribe_kwargs(self):
183+
# The value is consumed via **nack_args and consumer_args.update(...), so it must be a dict
184+
# with exactly the keyword subscribe() expects.
185+
args = self._instance(delay_ms=250).get_negative_ack_args()
186+
self.assertIsInstance(args, dict)
187+
self.assertEqual(["negative_ack_redelivery_delay_ms"], list(args.keys()))

0 commit comments

Comments
 (0)