fluent-logger-java icon indicating copy to clipboard operation
fluent-logger-java copied to clipboard

Published 20 hours ago • verified publisher icon fluent

enable some asynchronous logging (revised)

Open cosmo0920 opened this issue 8 years ago • 10 comments

Revised #30.

And I modified setErrorHandler/removeErrorHandler in AsyncRawSocketSender.

Should I add tests for AsyncRawSocketSender?

cosmo0920 avatar Sep 07 '15 07:09 cosmo0920

Why does this feature make no progress anymore?

jondoe1337 avatar Mar 08 '16 12:03 jondoe1337

This feature is implemented in Fluency. I think that this library's owners do not want to add this feature because they want to keep simple implementation.

cosmo0920 avatar Mar 08 '16 15:03 cosmo0920

@cosmo0920 Oh really sorry, I failed to notice the notifications of this project. Let me see the changes.

komamitsu avatar Mar 09 '16 14:03 komamitsu

@cosmo0920 @mxk1235 Thanks for the PRs! I left some comments. Also, it needs unit tests in principle.

BTW, as I commented earlier, AsyncRawSocketSender creates a thread for each invoking emit() method and I'm concerned about the performance.

komamitsu avatar Mar 09 '16 15:03 komamitsu

Thanks for your review! I'll try to resolve issues which are pointed out.

cosmo0920 avatar Mar 09 '16 23:03 cosmo0920

I fixed some test failures at https://github.com/fluent/fluent-logger-java/commit/969b88d0d1ed9a5aa27e92ccc72621d524c76cdf.

Could you try git rebase master to it?

komamitsu avatar May 02 '16 08:05 komamitsu

Rebased!

cosmo0920 avatar May 02 '16 08:05 cosmo0920

@cosmo0920 Thanks! I left some comments on the diff. Especially, https://github.com/fluent/fluent-logger-java/pull/49/files#r61833474 is an issue, I think.

BTW, will you actually use AsyncRawSocketSender? If so, what do you think about creating an EmitRunnable for each emit() call in terms of performance. If it's just experimental, why don't you add a comment like this on the class? This feature is highly experimental

komamitsu avatar May 03 '16 06:05 komamitsu

BTW, will you actually use AsyncRawSocketSender?

No, I won't. It's just an experimental.

why don't you add a comment like this on the class? This feature is highly experimental

I've added this one in javadoc comment.

cosmo0920 avatar May 09 '16 08:05 cosmo0920

@cosmo0920 Sorry for late reply again. Could you check this comment? https://github.com/fluent/fluent-logger-java/pull/49/commits/1bf11d7bdbbbec13c84eea384acc3a2bf8488e3e#r65201214

komamitsu avatar May 31 '16 15:05 komamitsu