add parameter for limiting maximum AOF files size on disk #1630
Conversation
Signed-off-by: kronwerk <kronwerk@users.noreply.github.com> improved aof-max-size tests Signed-off-by: kronwerk <kronwerk@users.noreply.github.com>
Signed-off-by: kronwerk <kronwerk@users.noreply.github.com>
Signed-off-by: kronwerk <kronwerk@users.noreply.github.com>
kronwerk
force-pushed
the
aof_size_limit
branch
from
d6cafa1 to
2954eb0
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## unstable #1630 +/- ##
============================================
+ Coverage 70.84% 70.99% +0.14%
============================================
Files 121 121
Lines 65169 65180 +11
============================================
+ Hits 46172 46274 +102
+ Misses 18997 18906 -91
|
|
@xbasel @soloestoy pls take a look, I've dumped my previous similar PR where you were reviewers |
| * there is an actual error condition we'll get it at the next try. | ||
| * We also check for aof-max-size limit here returning custom error on exceed. */ | ||
| ssize_t aofWrite(int fd, const char *buf, size_t len, off_t aof_current_size, unsigned long long aof_max_size) { | ||
| ssize_t nwritten = 0, totwritten = 0, nonewritten = -1; |
There was a problem hiding this comment.
Seems like nonewritten should be a constant/literal.
Perhaps directly returning -1/C_ERR instead of using an extra variable.
| ssize_t nwritten = 0, totwritten = 0; | ||
| * there is an actual error condition we'll get it at the next try. | ||
| * We also check for aof-max-size limit here returning custom error on exceed. */ | ||
| ssize_t aofWrite(int fd, const char *buf, size_t len, off_t aof_current_size, unsigned long long aof_max_size) { |
| ssize_t nwritten = 0, totwritten = 0; | ||
| * there is an actual error condition we'll get it at the next try. | ||
| * We also check for aof-max-size limit here returning custom error on exceed. */ | ||
| ssize_t aofWrite(int fd, const char *buf, size_t len, off_t aof_current_size, unsigned long long aof_max_size) { |
There was a problem hiding this comment.
Not related to this change, but do you have any idea why aof_current_size (and some other aof-related variables) are already defined as off_t instead of size_t?
| @@ -4627,13 +4627,17 @@ int writeCommandsDeniedByDiskError(void) { | |||
| return DISK_ERROR_TYPE_NONE; | |||
| } | |||
|
|
|||
| char *getAofWriteErrStr(int error_code) { | |||
There was a problem hiding this comment.
If this function is strictly for AOF-related and always uses server.aof_last_write_errno, passing error_code is misleading. I'd remove the parameter and read server.aof_last_write_errno directly to avoid confusion and unintended wider use.
[duplicate of this]
one can optionally want to limit AOF disk usage not with disk real size, but with some tailored value. this PR makes it possible.
possible use cases:
while on 100% filled disk (like nowadays) that automatic solution is impossible due to insufficient space for tmp AOF rewrite files (0 space, actually) - one should size up disk under running database which makes some people a bit nervous, as I noticed, and in some cases that's not always convenient or even possible.
Fixes #540