You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by "abhilashmandaliya (via GitHub)" <gi...@apache.org> on 2023/03/22 07:14:18 UTC
[GitHub] [pulsar] abhilashmandaliya opened a new issue, #19894: [Bug] Pulsar shutdowns Sink connector on exception
abhilashmandaliya opened a new issue, #19894:
URL: https://github.com/apache/pulsar/issues/19894
### Search before asking
- [X] I searched in the [issues](https://github.com/apache/pulsar/issues) and found nothing similar.
### Version
* Pulsar 2.11.0
### Minimal reproduce step
* Change any sink implementation for test and call `record.fail()` from the write method. The processing guarantee for the sink should be set to `EFFECTIVELY_ONCE`.
### What did you expect to see?
For the sinks having `EFFECTIVE_ONCE` guarantee, prior to pulsar v2.7.3 , if there was any exception in record processing, the `CompletableFuture` [callback](https://github.com/apache/pulsar/blob/v2.7.2/pulsar-functions/instance/src/main/java/org/apache/pulsar/functions/instance/JavaInstanceRunnable.java#L345) was swallowing the [RuntimeException](https://github.com/apache/pulsar/blob/v2.7.2/pulsar-functions/instance/src/main/java/org/apache/pulsar/functions/instance/JavaInstanceRunnable.java#L376). From v2.7.3, it is [not processed in the callback](https://github.com/apache/pulsar/blob/v2.7.3/pulsar-functions/instance/src/main/java/org/apache/pulsar/functions/instance/JavaInstanceRunnable.java#L349) hence [caught](https://github.com/apache/pulsar/blob/v2.7.3/pulsar-functions/instance/src/main/java/org/apache/pulsar/functions/instance/JavaInstanceRunnable.java#L277) by the `run` method of `JavaInstanceRunnable` which shutdowns the sink. Is this expected behavior? In this case, on
failure, no further processing will happen as the sink is shutdown. Shouldn't this exception be handled and the record be retried and allow other future message processing as well? At least, there should be a config option that would decide to shutdown the sink or keep it running.
### What did you see instead?
Pulsar shutdowns the sink service hence that particular record and any further records are processed by the sink.
### 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
[GitHub] [pulsar] cbornet commented on issue #19894: [Bug] Pulsar shutdowns Sink connector on exception
Posted by "cbornet (via GitHub)" <gi...@apache.org>.
cbornet commented on issue #19894:
URL: https://github.com/apache/pulsar/issues/19894#issuecomment-1482629555
I'm not an expert but it seems to be the desired behavior for `EFFECTIVELY_ONCE` indeed.
--
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
[GitHub] [pulsar] abhilashmandaliya commented on issue #19894: [Bug] Pulsar shutdowns Sink connector on exception
Posted by "abhilashmandaliya (via GitHub)" <gi...@apache.org>.
abhilashmandaliya commented on issue #19894:
URL: https://github.com/apache/pulsar/issues/19894#issuecomment-1483735947
> while this PR #14589 was not merged (cherry-picks w/protocol changes), it outlines why we want the sink to crash on unhandled exceptions.
Thanks for the reply @dlg99. I went through PR #14589 and got your point for k8s use case. But let's consider a case where some sink parses some field of the received record and that field is malformed and the sink can't parse it and wants to throw an exception. In this case, it will never come out of a poison message. Shouldn't a config option exist to ignore exception, log it and send an `ack` to the broker? If we send a `nack`, this malformed message will never succeed and it will never come out of a poison message.
--
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
[GitHub] [pulsar] abhilashmandaliya commented on issue #19894: [Bug] Pulsar shutdowns Sink connector on exception
Posted by "abhilashmandaliya (via GitHub)" <gi...@apache.org>.
abhilashmandaliya commented on issue #19894:
URL: https://github.com/apache/pulsar/issues/19894#issuecomment-1482581818
Thanks for the update @cbornet.
> We could catch the exception and nack. The advantage is that you can configure retries and DLQ if the use-case allows it.
I think this will not work directly in the current code. If we nack by calling the `fail` method on the record, the failure handler will again throw a [RuntimeException](https://github.com/apache/pulsar/blob/v2.11.0/pulsar-functions/instance/src/main/java/org/apache/pulsar/functions/source/PulsarSource.java#L155) which will be [caught](https://github.com/apache/pulsar/blob/v2.11.0/pulsar-functions/instance/src/main/java/org/apache/pulsar/functions/instance/JavaInstanceRunnable.java#L304) by the JavaInstanceRunnable and this loop will continue. It needs some more changes or please correct me if I am wrong.
--
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
[GitHub] [pulsar] eolivelli commented on issue #19894: [Bug] Pulsar shutdowns Sink connector on exception
Posted by "eolivelli (via GitHub)" <gi...@apache.org>.
eolivelli commented on issue #19894:
URL: https://github.com/apache/pulsar/issues/19894#issuecomment-1479384496
The behavior you want makes sense when auto acknowledge is turned on, in that case the Sink should acknowledge the message in case of success and nack the message otherwise.
--
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
[GitHub] [pulsar] abhilashmandaliya commented on issue #19894: [Bug] Pulsar shutdowns Sink connector on exception
Posted by "abhilashmandaliya (via GitHub)" <gi...@apache.org>.
abhilashmandaliya commented on issue #19894:
URL: https://github.com/apache/pulsar/issues/19894#issuecomment-1480661046
> > For the sinks having EFFECTIVE_ONCE guarantee, prior to pulsar v2.7.3 , if there was any exception in record processing, the CompletableFuture [callback](https://github.com/apache/pulsar/blob/v2.7.2/pulsar-functions/instance/src/main/java/org/apache/pulsar/functions/instance/JavaInstanceRunnable.java#L345) was swallowing the [RuntimeException](https://github.com/apache/pulsar/blob/v2.7.2/pulsar-functions/instance/src/main/java/org/apache/pulsar/functions/instance/JavaInstanceRunnable.java#L376). From v2.7.3, it is [not processed in the callback](https://github.com/apache/pulsar/blob/v2.7.3/pulsar-functions/instance/src/main/java/org/apache/pulsar/functions/instance/JavaInstanceRunnable.java#L349) hence [caught](https://github.com/apache/pulsar/blob/v2.7.3/pulsar-functions/instance/src/main/java/org/apache/pulsar/functions/instance/JavaInstanceRunnable.java#L277) by the run method of JavaInstanceRunnable which shutdowns the sink. Is this expected behavior? In this case, on fail
ure, no further processing will happen as the sink is shutdown.
>
> @abhilashmandaliya Yes, it seems surprising if the sink is shutdown. Is the sink left in the shutdown state or does it restart automatically?
@lhotari it gets stuck into shutdown/restart loop as Pulsar will retry the same message again and again.
--
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
[GitHub] [pulsar] lhotari commented on issue #19894: [Bug] Pulsar shutdowns Sink connector on exception
Posted by "lhotari (via GitHub)" <gi...@apache.org>.
lhotari commented on issue #19894:
URL: https://github.com/apache/pulsar/issues/19894#issuecomment-1480057491
> For the sinks having EFFECTIVE_ONCE guarantee, prior to pulsar v2.7.3 , if there was any exception in record processing, the CompletableFuture [callback](https://github.com/apache/pulsar/blob/v2.7.2/pulsar-functions/instance/src/main/java/org/apache/pulsar/functions/instance/JavaInstanceRunnable.java#L345) was swallowing the [RuntimeException](https://github.com/apache/pulsar/blob/v2.7.2/pulsar-functions/instance/src/main/java/org/apache/pulsar/functions/instance/JavaInstanceRunnable.java#L376). From v2.7.3, it is [not processed in the callback](https://github.com/apache/pulsar/blob/v2.7.3/pulsar-functions/instance/src/main/java/org/apache/pulsar/functions/instance/JavaInstanceRunnable.java#L349) hence [caught](https://github.com/apache/pulsar/blob/v2.7.3/pulsar-functions/instance/src/main/java/org/apache/pulsar/functions/instance/JavaInstanceRunnable.java#L277) by the run method of JavaInstanceRunnable which shutdowns the sink. Is this expected behavior? In this case, on failur
e, no further processing will happen as the sink is shutdown.
@abhilashmandaliya Yes, it seems surprising if the sink is shutdown. Is the sink left in the shutdown state or does it restart automatically?
> Shouldn't this exception be handled and the record be retried and allow other future message processing as well? At least, there should be a config option that would decide to shutdown the sink or keep it running.
seems reasonable
@abhilashmandaliya
--
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
[GitHub] [pulsar] abhilashmandaliya commented on issue #19894: [Bug] Pulsar shutdowns Sink connector on exception
Posted by "abhilashmandaliya (via GitHub)" <gi...@apache.org>.
abhilashmandaliya commented on issue #19894:
URL: https://github.com/apache/pulsar/issues/19894#issuecomment-1480769277
> > it gets stuck into shutdown/restart loop as Pulsar will retry the same message again and again.
>
> @abhilashmandaliya Would this problem of being stuck on a "poison message" get resolved if the Sink connector didn't get shutdown?
@lhotari it was working that way in the previous version. In the case of the exception, the `JavaInstanceRunnable` was swallowing it and hence it was neither sending `ack` nor a `nack`. By default, the ack timeout is disabled hence the [broker will not re-deliver the message unless a consumer crashes](https://pulsar.apache.org/docs/2.11.x/concepts-messaging/#acknowledgment-timeout). Now we hit this 'poison message' loop because of the consumer shutdowns and restarts.
--
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
[GitHub] [pulsar] abhilashmandaliya commented on issue #19894: [Bug] Pulsar shutdowns Sink connector on exception
Posted by "abhilashmandaliya (via GitHub)" <gi...@apache.org>.
abhilashmandaliya commented on issue #19894:
URL: https://github.com/apache/pulsar/issues/19894#issuecomment-1480705073
> The behavior you want makes sense when auto acknowledge is turned on, in that case the Sink should acknowledge the message in case of success and nack the message otherwise.
@eolivelli In my case, autoack is set to false.
--
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
[GitHub] [pulsar] eolivelli commented on issue #19894: [Bug] Pulsar shutdowns Sink connector on exception
Posted by "eolivelli (via GitHub)" <gi...@apache.org>.
eolivelli commented on issue #19894:
URL: https://github.com/apache/pulsar/issues/19894#issuecomment-1479383270
Runtime exceptions are totally out of control, like Out of memory errors and they can leave the JVM in a unstable status
--
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
[GitHub] [pulsar] abhilashmandaliya commented on issue #19894: [Bug] Pulsar shutdowns Sink connector on exception
Posted by "abhilashmandaliya (via GitHub)" <gi...@apache.org>.
abhilashmandaliya commented on issue #19894:
URL: https://github.com/apache/pulsar/issues/19894#issuecomment-1484693573
Thanks for all your review and feedback @eolivelli @lhotari @cbornet @dlg99
What should be the steps further? I am ready to contribute a fix given some guidance :)
--
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
[GitHub] [pulsar] eolivelli commented on issue #19894: [Bug] Pulsar shutdowns Sink connector on exception
Posted by "eolivelli (via GitHub)" <gi...@apache.org>.
eolivelli commented on issue #19894:
URL: https://github.com/apache/pulsar/issues/19894#issuecomment-1479388732
@dlg99 @nicoloboschi @cbornet may be interested in taking a look
--
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
[GitHub] [pulsar] eolivelli commented on issue #19894: [Bug] Pulsar shutdowns Sink connector on exception
Posted by "eolivelli (via GitHub)" <gi...@apache.org>.
eolivelli commented on issue #19894:
URL: https://github.com/apache/pulsar/issues/19894#issuecomment-1479388083
I can't check the code deeply, I will take a look later
--
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
[GitHub] [pulsar] abhilashmandaliya commented on issue #19894: [Bug] Pulsar shutdowns Sink connector on exception
Posted by "abhilashmandaliya (via GitHub)" <gi...@apache.org>.
abhilashmandaliya commented on issue #19894:
URL: https://github.com/apache/pulsar/issues/19894#issuecomment-1479026929
@sijie @lhotari @eolivelli would be great if you guys can clarify. I can see that this behavior has changed from PR #10618
--
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
[GitHub] [pulsar] cbornet commented on issue #19894: [Bug] Pulsar shutdowns Sink connector on exception
Posted by "cbornet (via GitHub)" <gi...@apache.org>.
cbornet commented on issue #19894:
URL: https://github.com/apache/pulsar/issues/19894#issuecomment-1482516951
It seems good behavior to me that a poison pill is handled and not silently ignored. The previous behavior of neither acking nor nacking and just forget about it until there's a restart (consuming some broker and client resources for the message reference) doesn't seem right.
We could catch the exception and nack. The advantage is that you can configure retries and DLQ if the use-case allows it.
Now in some situations such as OOME, it's probably better to let the connector crash and be restarted.
Note that crash loops can have some bad consequences such as spurious bandwidth consumption in k8s/cloud environments (downloading the NAR from BK at each restart).
--
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
[GitHub] [pulsar] github-actions[bot] commented on issue #19894: [Bug] Pulsar shutdowns Sink connector on exception
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on issue #19894:
URL: https://github.com/apache/pulsar/issues/19894#issuecomment-1524434788
The issue had no activity for 30 days, mark with Stale label.
--
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
[GitHub] [pulsar] lhotari commented on issue #19894: [Bug] Pulsar shutdowns Sink connector on exception
Posted by "lhotari (via GitHub)" <gi...@apache.org>.
lhotari commented on issue #19894:
URL: https://github.com/apache/pulsar/issues/19894#issuecomment-1480052675
> Runtime exceptions are totally out of control, like Out of memory errors and they can leave the JVM in a unstable status
@eolivelliWouldn't it be fine to catch all other throwables which aren't java.lang.Errors?
--
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
[GitHub] [pulsar] lhotari commented on issue #19894: [Bug] Pulsar shutdowns Sink connector on exception
Posted by "lhotari (via GitHub)" <gi...@apache.org>.
lhotari commented on issue #19894:
URL: https://github.com/apache/pulsar/issues/19894#issuecomment-1480720099
> it gets stuck into shutdown/restart loop as Pulsar will retry the same message again and again.
@abhilashmandaliya Would this problem of being stuck on a "poison message" get resolved if the Sink connector didn't get shutdown?
--
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
[GitHub] [pulsar] dlg99 commented on issue #19894: [Bug] Pulsar shutdowns Sink connector on exception
Posted by "dlg99 (via GitHub)" <gi...@apache.org>.
dlg99 commented on issue #19894:
URL: https://github.com/apache/pulsar/issues/19894#issuecomment-1483609877
while this PR https://github.com/apache/pulsar/pull/14589 was not merged (cherry-picks w/protocol changes), it outlines why we want the sink to crash on unhandled exceptions.
--
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