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

Add diskbuffer segment. #78

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

Conversation

JustKiddingCode
Copy link

Allows to buffer flows in memory and (additionally) on disk in zstd compressed files.

Add a small segment (filegate) for testing purposes to pause the pipeline when a file exists.

Allows to buffer flows in memory and (additionally) on disk in zstd
compressed files.

Add a small segment (filegate) for testing purposes to pause the pipeline when a file
exists.
@georg-e
Copy link
Contributor

georg-e commented Aug 7, 2023

Hi, thank you for the PR. I took a first look on it and have some comments and remarks to be adressed. Maybe you have already identified some of my points and maybe we need some more detailed testing and validation of this feature.

  • General structure and categorization:

    • i think the filegate segments suits better to the already existing testing directory/category instead of a new category like dev.
    • the new diskbuffer segment will be better categorized within the controlflow category. i think its rather an offloading and manipulation of the control-flow in the current flowpipeline instance
    • if possible, it would be nice to add some test cases to the segment. To start maybe a simple "passthrough" test for knowing, that the segment works and passes through flows. For an example please find pass_test.go
  • Diskbuffer segment

    • diskbuffer.go line 89 is probably reading the wrong configuration variable
    • it looks like data is still written to disk after filesize is reached. (after debug log in line 298 was triggered)

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