-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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: flush idle dataobjects after X seconds #16348
Conversation
💻 Deploy preview deleted. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. I left a few minor suggestions but approving so you can merge once you're ready
cfg.IdleFlushTimeout = 60 * 60 * time.Second // default to 1 hour | ||
} | ||
|
||
f.DurationVar(&cfg.IdleFlushTimeout, prefix+"idle-flush-timeout", cfg.IdleFlushTimeout, "The maximum amount of time to wait in seconds before flushing an object that is no longer receiving new writes") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can set the default value as the 3rd argument in the flag definition, if you wish
@@ -39,6 +39,10 @@ type partitionProcessor struct { | |||
bucket objstore.Bucket | |||
bufPool *sync.Pool | |||
|
|||
// Idle stream handling | |||
idleFlushTimout time.Duration |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo: Timout -> Timeout
} | ||
|
||
now := time.Now() | ||
if now.Sub(p.lastFlush) < p.idleFlushTimout { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could also use time.Since here: time.Since(p.lastFlush) < p.idleFlushTimeout
In practice it makes very little difference but its slightly easier to read
What this PR does / why we need it:
We need an idle check for our objects to be flushed, since there are cases that objects are in memory until reach the configurable size.
If we stop receiving data the size won't increase and it is not flushed.
Which issue(s) this PR fixes:
Fixes https://github.com/grafana/loki-private/issues/1359
Special notes for your reviewer:
Checklist
CONTRIBUTING.md
guide (required)feat
PRs are unlikely to be accepted unless a case can be made for the feature actually being a bug fix to existing behavior.docs/sources/setup/upgrade/_index.md
deprecated-config.yaml
anddeleted-config.yaml
files respectively in thetools/deprecated-config-checker
directory. Example PR