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

Fix ack handling #73

Closed
wants to merge 5 commits into from
Closed

Conversation

notslang
Copy link
Contributor

@notslang notslang commented Jul 24, 2017

So far I've got this running at 250 msg/sec without issues. If I go faster than that I start missing acks, even though it seems that the messages got accepted. Not sure if that's an issue in the fluentd daemon, or something I've done wrong, but I'll keep polishing this up.

Previously, receiving multiple acks in a 100ms window cause all but the last one to be dropped. This fixes the issue and can even handle unordered acks.

Closes #67

You only need to do that when you're passing the value of this to
another function. Also remove wrapper function from `FluentSender.emit`
Previously, receiving multiple acks in a 100ms window cause all but the
last one to be dropped. This fixes the issue and can even handle
unordered acks.
@okkez
Copy link
Contributor

okkez commented Jul 26, 2017

We must follow Fluentd forward protocol specification and we should follow Fluentd out_forward implementation and other logger implemantations.

Fluentd out_forward and other loggers block after send require-ack-response request. So we must block until receive ack response.

@notslang
Copy link
Contributor Author

Where does the spec say we need to block until an ack is received?

@okkez
Copy link
Contributor

okkez commented Jul 26, 2017

Spec does not say we need to block until an ack is received.
But Fluentd out_forward implementation and other logger implementation do block until an ack is received.
Other loggers I've checked:

BTW, fluent-logger-python, fluent-logger-java don't support requrie-ack-response. fluent-logger-ruby don't support yet, too.

requireAckResponse provides at-least-once semantics.

If you need more performance, we can support PackedForward or CompressedPackedForward to send a lot of events at once.

@notslang
Copy link
Contributor Author

Over a high latency connection, not waiting for an ack should have a significant performance advantage. And yeah, I'm working on packed forward support right now, but that also benefits from asynchronous sending.

@okkez
Copy link
Contributor

okkez commented Jul 26, 2017

Could you take a benchmark to compare packed forward mode and forward mode?

@notslang
Copy link
Contributor Author

It depends a lot on how you're doing packed forward mode. If you're trying to send messages as soon as possible then you're not going to be building up a backlog until you get well over 250 msg/sec (even if you limit to 1 outstanding ack at any given time), so messages will be sent individually up until that point. You could wait until you have a backlog of n messages before you pack them all together, but then if the process crashes up to n - 1 messages would be lost... And those messages would be from right before the crash, so they're probably important. You could pack and send a backlog of messages on an interval, but that creates the same type of problem n seconds of data could be lost in a crash.

@notslang
Copy link
Contributor Author

But Fluentd out_forward implementation and other logger implementation do block until an ack is received.

Are you sure that's a good thing? You don't need to look any further than the buffer compression and compressed forward implementation to see why that codebase shouldn't be treated as gospel.

Plus, the fluentd daemon has on-disk storage, meaning that in the event of a crash the backlog can be recovered, unlike this implementation where the backlog is lost. They can afford delays that cause logs to build up because they're dealing with a fundamentally different problem.

@mururu
Copy link
Member

mururu commented Jul 26, 2017

I think out_forward of fluentd v0.14 doesn't block sending data to wait for ack. It creates another thread to handle it: https://github.com/fluent/fluentd/blob/master/lib/fluent/plugin/out_forward.rb#L254

@okkez
Copy link
Contributor

okkez commented Jul 27, 2017

I've created new issues #74 and #75.
Please move to these issues.

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.

'ack in response and chunk id in sent data are different' if logs are submitted too quickly
3 participants