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

chore: reorganise buffers behaviour #1755

Merged
merged 1 commit into from
Oct 16, 2023

Conversation

hauleth
Copy link
Contributor

@hauleth hauleth commented Oct 12, 2023

No description provided.

@hauleth hauleth force-pushed the chore/reorganise-buffers-behaviour branch from 9cf9dc5 to f8f3d2a Compare October 12, 2023 19:53
Copy link
Contributor

@Ziinc Ziinc left a comment

Choose a reason for hiding this comment

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

the MemoryBuffer actually needs to get rewritten to how the v1 pipeline works, such that we insert directly into Broadway's internal queue. Having the separate buffer leads to situation where we have 2 sequential queues.

@hauleth
Copy link
Contributor Author

hauleth commented Oct 13, 2023

Ok, so we can close it and I can checkout how to do that. The whole point of all these changes is to have behaviour callbacks calls in single place, so this will allow me to add telemetry in one place instead of tracing through whole codebase to see where I need add them, or to be wary of adding them to each implementation independently. So that is why you can see some of these appearing, as it is all preparation to Telemetry addition.

@Ziinc
Copy link
Contributor

Ziinc commented Oct 14, 2023

It's fine to merge this first, just wanted to give you more context. I'll have a final once over on Monday, but I like the api changes. We had the idea of a persistent disk-backed buffer previously to reduce risk of dropping events on node failure/shutdown, hence the pluggable design.

lib/logflare/buffers/memory_buffer.ex Outdated Show resolved Hide resolved
lib/logflare/buffers/memory_buffer.ex Outdated Show resolved Hide resolved
@hauleth hauleth force-pushed the chore/reorganise-buffers-behaviour branch 3 times, most recently from e465b9e to 0590365 Compare October 16, 2023 15:26
@hauleth hauleth force-pushed the chore/reorganise-buffers-behaviour branch from 0590365 to b4f7ff4 Compare October 16, 2023 15:36
@Ziinc Ziinc merged commit c6959c3 into Logflare:main Oct 16, 2023
3 checks passed
@hauleth hauleth deleted the chore/reorganise-buffers-behaviour branch October 16, 2023 19:04
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.

2 participants