8000 docs(delivery): added first draft of e2e ack guide by barieom · Pull Request #11760 · vectordotdev/vector · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

docs(delivery): added first draft of e2e ack guide #11760

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

Draft
wants to merge 15 commits into
base: master
Choose a base branch
from
Draft

Conversation

barieom
Copy link
Contributor
@barieom barieom commented Mar 9, 2022

This is the first draft of the end-to-end acknowledgements guide. Let me know if I should expand on any talking points—currently only asking for preliminary review from @bruceg and @jszwedko

@bits-bot
Copy link
bits-bot commented Mar 9, 2022

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ jszwedko
❌ barieom
You have signed the CLA already but the status is still pending? Let us recheck it.

@netlify
Copy link
netlify bot commented Mar 9, 2022

Deploy Preview for vector-project ready!

Name Link
🔨 Latest commit 9940cc5
🔍 Latest deploy log https://app.netlify.com/sites/vector-project/deploys/624de76543c044000804ce12
😎 Deploy Preview https://deploy-preview-11760--vector-project.netlify.app/reports/lighthouse
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@barieom barieom requested review from bruceg and jszwedko March 9, 2022 23:04
Copy link
Member
@jszwedko jszwedko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @barieom !

I think this needs to be revised since the configuration was moved from sources to sinks, but otherwise seems good.

8000
barieom and others added 2 commits March 14, 2022 18:01
Co-authored-by: Jesse Szwedko <jesse@szwedko.me>
@barieom
Copy link
Contributor Author
barieom commented Mar 14, 2022

Thanks @barieom !

I think this needs to be revised since the configuration was moved from sources to sinks, but otherwise seems good.

Thanks for reviewing @jszwedko ! Addressed all your comments :)

Copy link
Member
@bruceg bruceg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good starting guide. I left some clarifications and comments below.

Do we want to say anything about error feedback (ie what happens if a sink rejects events)?

@jszwedko jszwedko added this to the Vector 0.21.0 milestone Mar 16, 2022
barieom and others added 8 commits March 18, 2022 11:57
Co-authored-by: Bruce Guenter <bruce.guenter@datadoghq.com>
Co-authored-by: Bruce Guenter <bruce.guenter@datadoghq.com>
Co-authored-by: Bruce Guenter <bruce.guenter@datadoghq.com>
Co-authored-by: Bruce Guenter <bruce.guenter@datadoghq.com>
Co-authored-by: Bruce Guenter <bruce.guenter@datadoghq.com>
@barieom
Copy link
Contributor Author
barieom commented Mar 18, 2022

This is a good starting guide. I left some clarifications and comments below.

Do we want to say anything about error feedback (ie what happens if a sink rejects events)?

Agree that's important. That's now in there.

@jszwedko jszwedko requested a review from bruceg March 23, 2022 22:45
bruceg
bruceg previously approved these changes Mar 23, 2022
Copy link
Member
@jszwedko jszwedko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work @barieom ! I left a few last comments, but overall this looks good.

You can set and control the acknowledgement feature either at the global level or
the sink level. Let's start with the global configuration option first, as it is
easy as flipping on a switch. As you would expect from a global configuration, the
configuration below turns end-to-end acknowledgement on for every source:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
configuration below turns end-to-end acknowledgement on for every source:
configuration below turns end-to-end acknowledgement on for every sink:

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, technically it is turning it on for every source, which is where the behavior change actually happens. This is a point of confusion.

Comment on lines +59 to +61
part of as _failed_, and the source will respond accordingly. For example, HTTP sources
will produce some 400 error code, while sources like Kafka that don't have protocol
support for rejecting messages behaves similarly to a positive acknowledgement.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
part of as _failed_, and the source will respond accordingly. For example, HTTP sources
will produce some 400 error code, while sources like Kafka that don't have protocol
support for rejecting messages behaves similarly to a positive acknowledgement.
part of as _failed_, and the source will respond accordingly. For example, HTTP sources
will produce a 400 or 500-level error code, while sources like Kafka that don't have protocol
support for rejecting messages will simply not ack the messages, allowing them to be reprocessed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kafka actually does acknowledge the messages on a failure, as there is nothing else it can do. In that way, it behaves similar to the file source.

Comment on lines +63 to +66
## Edge cases

Unsurprisingly, there are a few exceptions and edge cases for the end-to-end
acknowledgement feature.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think these are really "edge cases" except maybe the lua one. I might break this up into a few sections like:

## Source and sink support for acknowledgements

## Acknowledgements and disk buffers

## Acknowledgements and user-space transforms

It occurs to me that we should probably document these restrictions somewhere in addition to this guide, but this is a good start. Maybe a dedicated page for acknowledgements under "Concepts": https://vector.dev/docs/about/concepts/

@jszwedko jszwedko removed this from the Vector 0.21.0 milestone Apr 12, 2022
@jszwedko jszwedko added this to the Vector v0.22.0 milestone Apr 12, 2022
@jszwedko jszwedko removed this from the Vector v0.22.0 milestone May 18, 2022
@jszwedko jszwedko marked this pull request as draft March 24, 2023 13:43
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.

5 participants
0