You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2022/09/05 08:52:19 UTC
[GitHub] [pulsar] cbornet opened a new pull request, #17471: Fix flaky testJavaLoggingFunction
cbornet opened a new pull request, #17471:
URL: https://github.com/apache/pulsar/pull/17471
Fixes #15392
### Motivation
When using a `log-topic` in Functions, an appender is added to all loggers to send the logs with the Pulsar client.
To prevent sending logs from the framework, the appender is added before the function processing (JavaInstance::handleMessage) and removed just after.
This is a problem because since #15188, the logs are sent asynchronously and sometimes we remove the appender before the log is processed by the LogAppender.
And this is the reason why we see flakyness in testJavaLoggingFunction because me miss some logs.
This is also a problem when using an async Function that returns CompletableFuture where the execution of the Function will be done async and probably when the appender is removed.
I had a look and the framework only sends error logs. It may even be interesting to get these logs to debug Functions.
So this change is about removing this dynamic add/delete of the appender (which probably also has some effect on perf) and add it once and for all.
### Modifications
Remove calls to `addLogTopicHandler ` and `removeLogTopicHandler` and call `setupLogTopicAppender` during the setup.
### Verifying this change
- [ ] Make sure that the change passes the CI checks.
This change is already covered by existing tests, such as *testJavaLoggingFunction*.
### Does this pull request potentially affect one of the following parts:
no
*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)
- The rest endpoints: (yes / no)
- The admin cli options: (yes / no)
- Anything that affects deployment: (yes / no / don't know)
### Documentation
Check the box below or label this PR directly.
Need to update docs?
- [ ] `doc-required`
(Your PR needs to update docs and you will update later)
- [x] `doc-not-needed`
fix
- [ ] `doc`
(Your PR contains doc changes)
- [ ] `doc-complete`
(Docs have been already added)
--
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 pull request #17471: [fix][functions] Fix flaky testJavaLoggingFunction
Posted by GitBox <gi...@apache.org>.
eolivelli commented on PR #17471:
URL: https://github.com/apache/pulsar/pull/17471#issuecomment-1236767851
@merlimat @codelipenghui I think that it is safer to revert https://github.com/apache/pulsar/pull/15188 at least for Pulsar 2.11
I agree with this patch but we have to fix 2.11 as priority
--
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] codelipenghui merged pull request #17471: [fix][functions] Fix Java Function instance LogAppender missing logs
Posted by GitBox <gi...@apache.org>.
codelipenghui merged PR #17471:
URL: https://github.com/apache/pulsar/pull/17471
--
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] tisonkun commented on pull request #17471: [fix][functions] Fix Java Function instance LogAppender missing logs
Posted by GitBox <gi...@apache.org>.
tisonkun commented on PR #17471:
URL: https://github.com/apache/pulsar/pull/17471#issuecomment-1240464952
Cool!
--
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 pull request #17471: [fix][functions] Fix Java Function instance LogAppender missing logs
Posted by "cbornet (via GitHub)" <gi...@apache.org>.
cbornet commented on PR #17471:
URL: https://github.com/apache/pulsar/pull/17471#issuecomment-1582107051
Yes, I think we can add it 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] merlimat commented on pull request #17471: [fix][functions] Fix Java Function instance LogAppender missing logs
Posted by "merlimat (via GitHub)" <gi...@apache.org>.
merlimat commented on PR #17471:
URL: https://github.com/apache/pulsar/pull/17471#issuecomment-1577112922
@cbornet I'm just realizing that the async-loggers PR was reverted. After this change, could #15188 be added 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