Conversation
…by sharedscripts). Use pkill command for postrotate script.
…ate config code more component-agnostic. Other adjustments.
internal/naming/names.go
Outdated
| // files have been rotated. The pgbouncer process is sent a sighup signal. | ||
| // TODO: Should this go here or in controller/postgrescluster/pgbouncer.go? | ||
| // Or somewhere in the internal/pgbouncer folder, for that matter? | ||
| PGBouncerPostRotateScript = "pkill -HUP --exact pgbouncer" |
There was a problem hiding this comment.
Kinda feel like this should be in the pgbouncer.go
There was a problem hiding this comment.
Agreed. Moving it.
internal/collector/config.go
Outdated
| if hours < 24 { | ||
| return int(hours), "hourly" | ||
| } | ||
| return int(math.Round(hours / 24)), "daily" |
There was a problem hiding this comment.
I want to flag this for a lot of documentation -- if I put in 3weeks of logs, I might expect 3 weeks of logs, not 21 days.
There was a problem hiding this comment.
Similarly, if I put in 36 hours, I might expect 36 hours, not 2 days.
There was a problem hiding this comment.
Well, 3 weeks of logs is 21 days of logs, we will just be keeping 21 log files where each is a day's worth of logs rather than 3 log files where each is a week's worth of logs... But yeah, we will definitely need plenty of emphasis on the fact that we will be rounding up to the nearest day for hour amounts that aren't multiples of 24.
There was a problem hiding this comment.
I don't think that needs to be documented. The user specifies how much to retain, not the method for retaining it.
There was a problem hiding this comment.
Good point. Should probably still make a note that we round up to the nearest day so users aren't confused at least.
…hen not set. Move PGBouncerPostRotateScript to collector package. Round daily retention count up to the nearest day.
internal/collector/instance.go
Outdated
|
|
||
| // If a retentionPeriod is set and this is a pod that uses logrotate for | ||
| // log rotation, add config volume and mount for logrotate config | ||
| if includeLogrotate && spec != nil && spec.Logs != nil && spec.Logs.RetentionPeriod != nil { |
There was a problem hiding this comment.
Do we require RetentionPeriod or are we allowing that to be blank and we'll default to a day?
There was a problem hiding this comment.
We allow it to be blank and default to a day.
There was a problem hiding this comment.
I'll remove the check for spec etc here
…rotate config. Round up (ceil) on retentionPeriod duration and add a test. Fix typo.
3 participants
Checklist:
Type of Changes:
What is the current behavior (link to any open issues here)?
No log rotation.
What is the new behavior (if this is a feature change)?
Allows the rotation of pgbouncer logs.
Other Information: