iodone commented on pull request #1853:
URL:
https://github.com/apache/incubator-kyuubi/pull/1853#issuecomment-1079592789
> LGTM, in addition, I have some comments:
>
> 1. Why don't we register handlers with `EventBus.registerAsync`?
> 2. `SparkJsonLoggingEventHandler` and `ServerJsonLoggingEventHandler`
don't seem to do anything, they seem unnecessary.
> 3. Do we need to use a separate `ExecutionContext` for EventBus instead of
`Implicits.global`?
@wForget, thank you for your advice.
For points 1, 3:
Currently, since the Async pattern has not been validated in real scenarios:
1. Just the same implementation of the previous LoggerService is done, not
upgraded to asynchronous.
2. Scala global `executioncontext` uses the `ForkJoin` settings related to
the system, which can ensure optimal performance. We can tuning it when using
async EventHandler in the future like the implement of
`AsyncElasticSearchLoggingHandler`
For points 2:
For decoupling and extensibility, while also facilitating independent unit
testing
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
