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 support for data persistence #351

Merged
merged 13 commits into from
Jan 29, 2020
Merged

Add support for data persistence #351

merged 13 commits into from
Jan 29, 2020

Conversation

maimaisie
Copy link
Contributor

@maimaisie maimaisie commented Jan 7, 2020

Description

This PR adds support for data persistence to a persistent volume per fluentd pod. After discussing with the team, we decided to switch from using deployments to statefulsets because statefulsets allows easier setup of persistent volume.

Testing performed
  • ci/build.sh
  • Redeploy fluentd and fluentd-events pods
  • Confirm events, logs, and metrics are coming in
  • Throttle the account, confirm data is written to buffer and flushed after unthrottling

@maimaisie maimaisie requested review from samjsong, rvmiller89 and vsinghal13 and removed request for samjsong and rvmiller89 January 7, 2020 22:10
Copy link
Contributor

@vsinghal13 vsinghal13 left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -1,5 +1,4 @@
compress gzip
flush_interval "#{ENV['FLUSH_INTERVAL']}"
flush_thread_count "#{ENV['NUM_THREADS']}"
chunk_limit_size "#{ENV['CHUNK_LIMIT_SIZE']}"
Copy link
Contributor

Choose a reason for hiding this comment

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

should we increase chunk_limit_size to 1mb knowing that the chunks will no longer be compressed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we do still do compression for memory buffer and file buffer without persistence, it might be really tricky to have two defaults depending on the buffer type..

Copy link
Contributor

Choose a reason for hiding this comment

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

OK I'm fine with keeping this for now. Ideally we figure out how to continue to use gzip since these metrics will compress heavily (repeated dimensions + metadata)

Copy link
Contributor

Choose a reason for hiding this comment

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

Could this be the issue we're seeing? fluent/fluentd#2619

Have we tried upgrading our fluentd image from v1.6.3 to v1.8.1 and see if it resolves the issue?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will try that

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep, v1.8.1 fixed the issue! will revert the compression changes

deploy/docker/Dockerfile Outdated Show resolved Hide resolved
@maimaisie maimaisie merged commit 78f550b into master Jan 29, 2020
@maimaisie maimaisie deleted the maisie-persistence-2 branch January 29, 2020 19:12
@maimaisie maimaisie mentioned this pull request Feb 19, 2020
4 tasks
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