From b4bd05241da2772f457d0552d807e7cfd4dc8f33 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Wed, 5 May 2010 13:36:29 +0200 Subject: [PATCH 1/7] tool to check if AOF is valid --- Makefile | 9 ++- redis-check-aof.c | 161 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 168 insertions(+), 2 deletions(-) create mode 100644 redis-check-aof.c diff --git a/Makefile b/Makefile index 13b6bf45..0d7cd28f 100644 --- a/Makefile +++ b/Makefile @@ -20,13 +20,15 @@ OBJ = adlist.o ae.o anet.o dict.o redis.o sds.o zmalloc.o lzf_c.o lzf_d.o pqsort BENCHOBJ = ae.o anet.o redis-benchmark.o sds.o adlist.o zmalloc.o CLIOBJ = anet.o sds.o adlist.o redis-cli.o zmalloc.o linenoise.o CHECKDUMPOBJ = redis-check-dump.o lzf_c.o lzf_d.o +CHECKAOFOBJ = redis-check-aof.o PRGNAME = redis-server BENCHPRGNAME = redis-benchmark CLIPRGNAME = redis-cli CHECKDUMPPRGNAME = redis-check-dump +CHECKAOFPRGNAME = redis-check-aof -all: redis-server redis-benchmark redis-cli redis-check-dump +all: redis-server redis-benchmark redis-cli redis-check-dump redis-check-aof # Deps (use make dep to generate this) adlist.o: adlist.c adlist.h zmalloc.h @@ -68,11 +70,14 @@ redis-cli: $(CLIOBJ) redis-check-dump: $(CHECKDUMPOBJ) $(CC) -o $(CHECKDUMPPRGNAME) $(CCOPT) $(DEBUG) $(CHECKDUMPOBJ) +redis-check-aof: $(CHECKAOFOBJ) + $(CC) -o $(CHECKAOFPRGNAME) $(CCOPT) $(DEBUG) $(CHECKAOFOBJ) + .c.o: $(CC) -c $(CFLAGS) $(DEBUG) $(COMPILE_TIME) $< clean: - rm -rf $(PRGNAME) $(BENCHPRGNAME) $(CLIPRGNAME) $(CHECKDUMPPRGNAME) *.o *.gcda *.gcno *.gcov + rm -rf $(PRGNAME) $(BENCHPRGNAME) $(CLIPRGNAME) $(CHECKDUMPPRGNAME) $(CHECKAOFPRGNAME) *.o *.gcda *.gcno *.gcov dep: $(CC) -MM *.c diff --git a/redis-check-aof.c b/redis-check-aof.c new file mode 100644 index 00000000..3be60781 --- /dev/null +++ b/redis-check-aof.c @@ -0,0 +1,161 @@ +#include +#include +#include +#include +#include "config.h" + +#define ERROR(...) { \ + char __buf[1024]; \ + sprintf(__buf, __VA_ARGS__); \ + sprintf(error, "0x%08lx: %s", epos, __buf); \ +} + +static char error[1024]; +static long epos; + +int consumeNewline(char *buf) { + if (strncmp(buf,"\r\n",2) != 0) { + ERROR("Expected \\r\\n, got: %02x%02x",buf[0],buf[1]); + return 0; + } + return 1; +} + +int readLong(FILE *fp, char prefix, long *target) { + char buf[128], *eptr; + epos = ftell(fp); + if (fgets(buf,sizeof(buf),fp) == NULL) { + return 0; + } + if (buf[0] != prefix) { + ERROR("Expected prefix '%c', got: '%c'",buf[0],prefix); + return 0; + } + *target = strtol(buf+1,&eptr,10); + return consumeNewline(eptr); +} + +int readBytes(FILE *fp, char *target, long length) { + long real; + epos = ftell(fp); + real = fread(target,1,length,fp); + if (real != length) { + ERROR("Expected to read %ld bytes, got %ld bytes",length,real); + return 0; + } + return 1; +} + +int readString(FILE *fp, char** target) { + long len; + *target = NULL; + if (!readLong(fp,'$',&len)) { + return 0; + } + + /* Increase length to also consume \r\n */ + len += 2; + *target = (char*)malloc(len); + if (!readBytes(fp,*target,len)) { + free(*target); + *target = NULL; + return 0; + } + if (!consumeNewline(*target+len-2)) { + free(*target); + *target = NULL; + return 0; + } + (*target)[len-2] = '\0'; + return 1; +} + +int readArgc(FILE *fp, long *target) { + return readLong(fp,'*',target); +} + +long process(FILE *fp) { + long argc, pos = 0; + int i, multi = 0; + char *str; + + while(1) { + if (!multi) pos = ftell(fp); + if (!readArgc(fp, &argc)) { + break; + } + + for (i = 0; i < argc; i++) { + if (!readString(fp,&str)) { + break; + } + if (i == 0) { + if (strcasecmp(str, "multi") == 0) { + if (multi++) { + ERROR("Unexpected MULTI"); + break; + } + } else if (strcasecmp(str, "exec") == 0) { + if (--multi) { + ERROR("Unexpected EXEC"); + break; + } + } + } + free(str); + } + + /* Check if loop was finished */ + if (i < argc) { + if (str) free(str); + break; + } + } + + if (feof(fp) && multi && strlen(error) == 0) { + ERROR("Reached EOF before reading EXEC for MULTI"); + } + + if (strlen(error) > 0) { + printf("%s\n", error); + } + + return pos; +} + +int main(int argc, char **argv) { + /* expect the first argument to be the dump file */ + if (argc <= 1) { + printf("Usage: %s \n", argv[0]); + exit(0); + } + + FILE *fp = fopen(argv[1],"r"); + if (fp == NULL) { + printf("Cannot open file: %s\n", argv[1]); + exit(1); + } + + struct redis_stat sb; + if (redis_fstat(fileno(fp),&sb) == -1) { + printf("Cannot stat file: %s\n", argv[1]); + exit(1); + } + + long size = sb.st_size; + if (size == 0) { + printf("Empty file: %s\n", argv[1]); + exit(1); + } + + long pos = process(fp); + if (pos < size) { + printf("First invalid operation at offset %ld.\n", pos); + exit(1); + } else { + printf("AOF is valid.\n"); + } + + fclose(fp); + return 0; +} From cb8ae3c88908a8a4b9c88d56713ae2aea472cbd8 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Wed, 5 May 2010 13:46:37 +0200 Subject: [PATCH 2/7] allow AOF to be fixed by truncating to the portion of the file that is valid --- redis-check-aof.c | 41 +++++++++++++++++++++++++++++++++-------- 1 file changed, 33 insertions(+), 8 deletions(-) diff --git a/redis-check-aof.c b/redis-check-aof.c index 3be60781..31abe1be 100644 --- a/redis-check-aof.c +++ b/redis-check-aof.c @@ -1,6 +1,7 @@ #include #include #include +#include #include #include "config.h" @@ -126,34 +127,58 @@ long process(FILE *fp) { int main(int argc, char **argv) { /* expect the first argument to be the dump file */ if (argc <= 1) { - printf("Usage: %s \n", argv[0]); + printf("Usage: %s [--fix] \n", argv[0]); exit(0); } - FILE *fp = fopen(argv[1],"r"); + char *filename; + int fix = 0; + if (argc == 3) { + if (strcmp(argv[1],"--fix") != 0) { + printf("Invalid argument: %s\n", argv[1]); + exit(1); + } + fix = 1; + filename = argv[2]; + } else if (argc == 2) { + filename = argv[1]; + } else { + printf("Invalid argument"); + exit(1); + } + + FILE *fp = fopen(filename,"r+"); if (fp == NULL) { - printf("Cannot open file: %s\n", argv[1]); + printf("Cannot open file: %s\n", filename); exit(1); } struct redis_stat sb; if (redis_fstat(fileno(fp),&sb) == -1) { - printf("Cannot stat file: %s\n", argv[1]); + printf("Cannot stat file: %s\n", filename); exit(1); } long size = sb.st_size; if (size == 0) { - printf("Empty file: %s\n", argv[1]); + printf("Empty file: %s\n", filename); exit(1); } long pos = process(fp); if (pos < size) { - printf("First invalid operation at offset %ld.\n", pos); - exit(1); + if (fix) { + if (ftruncate(fileno(fp), pos) == -1) { + printf("Could not truncate AOF to size %ld\n", pos); + exit(1); + } else { + printf("AOF succesfully truncated to %ld bytes\n", pos); + } + } else { + printf("First invalid operation at offset %ld\n", pos); + } } else { - printf("AOF is valid.\n"); + printf("AOF is valid\n"); } fclose(fp); From e795c7588875c47908c06a5fac0f869e23854218 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Wed, 5 May 2010 13:47:17 +0200 Subject: [PATCH 3/7] ignore redis-check-aof binary --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 1b0b8578..5463a140 100644 --- a/.gitignore +++ b/.gitignore @@ -6,6 +6,7 @@ redis-cli redis-server redis-benchmark redis-check-dump +redis-check-aof doc-tools mkrelease.sh release From 57ca68acc16cedb39409ce5e513f9c2dd6f29182 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Wed, 5 May 2010 14:02:04 +0200 Subject: [PATCH 4/7] moved argument parsing around --- redis-check-aof.c | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/redis-check-aof.c b/redis-check-aof.c index 31abe1be..53472032 100644 --- a/redis-check-aof.c +++ b/redis-check-aof.c @@ -125,25 +125,23 @@ long process(FILE *fp) { } int main(int argc, char **argv) { - /* expect the first argument to be the dump file */ - if (argc <= 1) { - printf("Usage: %s [--fix] \n", argv[0]); - exit(0); - } - char *filename; int fix = 0; - if (argc == 3) { + + if (argc < 2) { + printf("Usage: %s [--fix] \n", argv[0]); + exit(1); + } else if (argc == 2) { + filename = argv[1]; + } else if (argc == 3) { if (strcmp(argv[1],"--fix") != 0) { printf("Invalid argument: %s\n", argv[1]); exit(1); } - fix = 1; filename = argv[2]; - } else if (argc == 2) { - filename = argv[1]; + fix = 1; } else { - printf("Invalid argument"); + printf("Invalid arguments\n"); exit(1); } From e51fa063db5fbb53c754738d87146551aebfd8e2 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Wed, 5 May 2010 14:06:55 +0200 Subject: [PATCH 5/7] str can be free'd outside readString --- redis-check-aof.c | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/redis-check-aof.c b/redis-check-aof.c index 53472032..8d72e6b2 100644 --- a/redis-check-aof.c +++ b/redis-check-aof.c @@ -58,13 +58,9 @@ int readString(FILE *fp, char** target) { len += 2; *target = (char*)malloc(len); if (!readBytes(fp,*target,len)) { - free(*target); - *target = NULL; return 0; } if (!consumeNewline(*target+len-2)) { - free(*target); - *target = NULL; return 0; } (*target)[len-2] = '\0'; @@ -82,14 +78,10 @@ long process(FILE *fp) { while(1) { if (!multi) pos = ftell(fp); - if (!readArgc(fp, &argc)) { - break; - } + if (!readArgc(fp, &argc)) break; for (i = 0; i < argc; i++) { - if (!readString(fp,&str)) { - break; - } + if (!readString(fp,&str)) break; if (i == 0) { if (strcasecmp(str, "multi") == 0) { if (multi++) { @@ -106,7 +98,7 @@ long process(FILE *fp) { free(str); } - /* Check if loop was finished */ + /* Stop if the loop did not finish */ if (i < argc) { if (str) free(str); break; @@ -116,11 +108,9 @@ long process(FILE *fp) { if (feof(fp) && multi && strlen(error) == 0) { ERROR("Reached EOF before reading EXEC for MULTI"); } - if (strlen(error) > 0) { printf("%s\n", error); } - return pos; } From 81330149f84f26c3e0492a296acfce7bd55d783f Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Wed, 5 May 2010 16:38:50 +0200 Subject: [PATCH 6/7] ask for confirmation before AOF is truncated --- redis-check-aof.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/redis-check-aof.c b/redis-check-aof.c index 8d72e6b2..050bb562 100644 --- a/redis-check-aof.c +++ b/redis-check-aof.c @@ -154,16 +154,26 @@ int main(int argc, char **argv) { } long pos = process(fp); - if (pos < size) { + long diff = size-pos; + if (diff > 0) { if (fix) { + char buf[2]; + printf("This will shrink the AOF from %ld bytes, with %ld bytes, to %ld bytes\n",size,diff,pos); + printf("Continue? [y/N]: "); + if (fgets(buf,sizeof(buf),stdin) == NULL || + strncasecmp(buf,"y",1) != 0) { + printf("Aborting...\n"); + exit(1); + } if (ftruncate(fileno(fp), pos) == -1) { - printf("Could not truncate AOF to size %ld\n", pos); + printf("Failed to truncate AOF\n"); exit(1); } else { - printf("AOF succesfully truncated to %ld bytes\n", pos); + printf("Successfully truncated AOF\n"); } } else { - printf("First invalid operation at offset %ld\n", pos); + printf("AOF is not valid\n"); + exit(1); } } else { printf("AOF is valid\n"); From 8063b99da677afc470342a3f75552950b6a487db Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Thu, 6 May 2010 22:00:04 +0200 Subject: [PATCH 7/7] log error and quit when the AOF contains an unfinished MULTI --- redis.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/redis.c b/redis.c index 465398ad..d8d024e7 100644 --- a/redis.c +++ b/redis.c @@ -8111,12 +8111,14 @@ static struct redisClient *createFakeClient(void) { c->reply = listCreate(); listSetFreeMethod(c->reply,decrRefCount); listSetDupMethod(c->reply,dupClientReplyValue); + initClientMultiState(c); return c; } static void freeFakeClient(struct redisClient *c) { sdsfree(c->querybuf); listRelease(c->reply); + freeClientMultiState(c); zfree(c); } @@ -8128,6 +8130,7 @@ int loadAppendOnlyFile(char *filename) { FILE *fp = fopen(filename,"r"); struct redis_stat sb; unsigned long long loadedkeys = 0; + int appendonly = server.appendonly; if (redis_fstat(fileno(fp),&sb) != -1 && sb.st_size == 0) return REDIS_ERR; @@ -8137,6 +8140,10 @@ int loadAppendOnlyFile(char *filename) { exit(1); } + /* Temporarily disable AOF, to prevent EXEC from feeding a MULTI + * to the same file we're about to read. */ + server.appendonly = 0; + fakeClient = createFakeClient(); while(1) { int argc, j; @@ -8192,8 +8199,14 @@ int loadAppendOnlyFile(char *filename) { } } } + + /* This point can only be reached when EOF is reached without errors. + * If the client is in the middle of a MULTI/EXEC, log error and quit. */ + if (fakeClient->flags & REDIS_MULTI) goto readerr; + fclose(fp); freeFakeClient(fakeClient); + server.appendonly = appendonly; return REDIS_OK; readerr: