Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

a bug about aofrewrite_scheduled #161

Closed
hoterran opened this Issue Oct 26, 2011 · 5 comments

Comments

Projects
None yet
2 participants

i has post in google group.

when bgsave is doing, if user trigger bgrewriteaof, then
aofrewrite_scheduled = 1
but there not other set aofrewrite_scheduled back to 0, possible
bug???
==============all code about aofrewrite_scheduled ====
./src/redis.h: int aofrewrite_scheduled; /* Rewrite once
BGSAVE terminates. */
./src/redis.c: server.aofrewrite_scheduled)
./src/redis.c: server.aofrewrite_scheduled = 0; #just
init
./src/redis.c: server.aofrewrite_scheduled);
./src/aof.c: server.aofrewrite_scheduled = 1;
where server.aofrewrite_scheduled = 0;
==============use gdb test log =============
[15108] 26 Oct 15:46:52 * Background append only file rewriting
started by pid 15259
[15259] 26 Oct 15:46:52 * SYNC append only file rewrite performed
[15108] 26 Oct 15:46:52 * Background AOF rewrite terminated with
success
[15108] 26 Oct 15:46:52 * Parent diff successfully flushed to the
rewritten AOF (0 bytes)
[15108] 26 Oct 15:46:52 * Background AOF rewrite successful
[15108] 26 Oct 15:46:52 - Background AOF rewrite signal handler took
277us
[15108] 26 Oct 15:46:52 * Background append only file rewriting
started by pid 15260
[15260] 26 Oct 15:46:52 * SYNC append only file rewrite performed
[15108] 26 Oct 15:46:53 * Background AOF rewrite terminated with
success
[15108] 26 Oct 15:46:53 * Parent diff successfully flushed to the
rewritten AOF (0 bytes)
[15108] 26 Oct 15:46:53 * Background AOF rewrite successful
[15108] 26 Oct 15:46:53 - Background AOF rewrite signal handler took
207us
[15108] 26 Oct 15:46:53 * Background append only file rewriting
started by pid 15261
[15261] 26 Oct 15:46:53 * SYNC append only file rewrite performed
[15108] 26 Oct 15:46:53 * Background AOF rewrite terminated with
success
[15108] 26 Oct 15:46:53 * Parent diff successfully flushed to the
rewritten AOF (0 bytes)
[15108] 26 Oct 15:46:53 * Background AOF rewrite successful
[15108] 26 Oct 15:46:53 - Background AOF rewrite signal handler took
239us
[15108] 26 Oct 15:46:53 * Background append only file rewriting
started by pid 15262
[15262] 26 Oct 15:46:53 * SYNC append only file rewrite performed
[15108] 26 Oct 15:46:53 * Background AOF rewrite terminated with
success
[15108] 26 Oct 15:46:53 * Parent diff successfully flushed to the
rewritten AOF (0 bytes)
[15108] 26 Oct 15:46:53 * Background AOF rewrite successful
[15108] 26 Oct 15:46:53 - Background AOF rewrite signal handler took
212us
............................. repeat bgrewriteaof ............................

================my fix ==========================
598 if (server.bgsavechildpid == -1 && server.bgrewritechildpid
== -1 &&
599 server.aofrewrite_scheduled)
600 {
601 rewriteAppendOnlyFileBackground();
server.aofrewrite_scheduled = 0 // my add
602 }

Code i tested, is ok

antirez added a commit that referenced this issue Oct 26, 2011

Clear the AOF rewrite scheduled flag once an AOF rewrite is triggered…
…. Fix for issue #161, probably fixing 159 as well.

antirez added a commit that referenced this issue Oct 26, 2011

Clear the AOF rewrite scheduled flag once an AOF rewrite is triggered…
…. Fix for issue #161, probably fixing 159 as well.
Owner

antirez commented Oct 26, 2011

Hello, thanks, the bug exists indeed. My fix is marginally different compared to your fix since I clear the flag every time a background rewrite actually happens, this has two advantages mainly:

  • If the user asks BGREWRITEAOF again before the scheduler is albe to launch the scheduled one, the scheduled one is cancelled. Otherwise it would be repeated again.
  • If the scheduled rewrite fails for some reason, like fork(), it remains scheduled.

Taking this issue open as I'm still testing that it works well.

Owner

antirez commented Oct 26, 2011

Verified that the bug is fixed now. Closing, thank you.

@antirez antirez closed this Oct 26, 2011

ok, nice.

Owner

antirez commented Oct 26, 2011

@hoterran: thank you again, great bug report without frills but with what was needed to get it fixed ASAP.

@antirez yeah bug report is important.
i see 2.4.2 release has included this bug fix, you are so quickly ~~~~

JackieXie168 pushed a commit to JackieXie168/redis that referenced this issue Aug 29, 2016

Clear the AOF rewrite scheduled flag once an AOF rewrite is triggered…
…. Fix for issue #161, probably fixing 159 as well.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment