Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

enable some asynchronous logging #30

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

mxk1235
Copy link

@mxk1235 mxk1235 commented Sep 4, 2014

AsyncRawSocketSender is just a wrapper around RawSocketSender that does its work in a separate thread.

also added a factory method to create a logger with its own sender, and moved from using a synchronized method to using a synchronizedMap for the loggers.

@komamitsu
Copy link
Member

Thanks for the PR and sorry for my late reply.

I looked at the PR and I found that AsyncRawSocketSender.emit() and flush() create anonymous Runnable every time. It can cause a poor performance.

So I think we'd better use consumer-producer model to implement async sender. What do you think?

@mxk1235
Copy link
Author

mxk1235 commented Sep 22, 2014

sorry for the late reply. yes, performance might be an issue. but i think i'd rather let someone else do that refactoring. i am currently in the middle of a different task at work, which no longer has anything to do with AsyncRawSocketSender.

Mike Kobyakov added 2 commits November 25, 2014 21:04
Conflicts:
	src/main/java/org/fluentd/logger/sender/AsyncRawSocketSender.java
@mxk1235
Copy link
Author

mxk1235 commented Nov 26, 2014

changed to nested classes.

@anusricorp
Copy link

@komamitsu @mxk1235 can this PR be revived?
Could we handle the concerns Mitsunori Komatsu had

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants