From 145473acc5798a499a0f37e42df48a014a3955c1 Mon Sep 17 00:00:00 2001 From: Matt Stancliff Date: Fri, 9 May 2014 12:06:06 -0400 Subject: [PATCH 1/3] Convert check-dump to Redis check-rdb mode redis-check-dump is now named redis-check-rdb and it runs as a mode of redis-server instead of an independent binary. You can now use 'redis-server redis.conf --check-rdb' to check the RDB defined in redis.conf. Using argument --check-rdb checks the RDB and exits. We could potentially also allow the server to continue starting if the RDB check succeeds. This change also enables us to use RDB checking programatically from inside Redis for certain failure conditions. --- src/Makefile | 19 +++--- src/{redis-check-dump.c => redis-check-rdb.c} | 59 ++++++++----------- src/redis.c | 24 ++++++++ src/redis.h | 3 + 4 files changed, 60 insertions(+), 45 deletions(-) rename src/{redis-check-dump.c => redis-check-rdb.c} (94%) diff --git a/src/Makefile b/src/Makefile index 295600c4..271ab34d 100644 --- a/src/Makefile +++ b/src/Makefile @@ -117,17 +117,16 @@ endif REDIS_SERVER_NAME=redis-server REDIS_SENTINEL_NAME=redis-sentinel -REDIS_SERVER_OBJ=adlist.o quicklist.o ae.o anet.o dict.o redis.o sds.o zmalloc.o lzf_c.o lzf_d.o pqsort.o zipmap.o sha1.o ziplist.o release.o networking.o util.o object.o db.o replication.o rdb.o t_string.o t_list.o t_set.o t_zset.o t_hash.o config.o aof.o pubsub.o multi.o debug.o sort.o intset.o syncio.o cluster.o crc16.o endianconv.o slowlog.o scripting.o bio.o rio.o rand.o memtest.o crc64.o bitops.o sentinel.o notify.o setproctitle.o blocked.o hyperloglog.o latency.o sparkline.o +REDIS_SERVER_OBJ=adlist.o quicklist.o ae.o anet.o dict.o redis.o sds.o zmalloc.o lzf_c.o lzf_d.o pqsort.o zipmap.o sha1.o ziplist.o release.o networking.o util.o object.o db.o replication.o rdb.o t_string.o t_list.o t_set.o t_zset.o t_hash.o config.o aof.o pubsub.o multi.o debug.o sort.o intset.o syncio.o cluster.o crc16.o endianconv.o slowlog.o scripting.o bio.o rio.o rand.o memtest.o crc64.o bitops.o sentinel.o notify.o setproctitle.o blocked.o hyperloglog.o latency.o sparkline.o redis-check-rdb.o REDIS_CLI_NAME=redis-cli REDIS_CLI_OBJ=anet.o sds.o adlist.o redis-cli.o zmalloc.o release.o anet.o ae.o crc64.o REDIS_BENCHMARK_NAME=redis-benchmark REDIS_BENCHMARK_OBJ=ae.o anet.o redis-benchmark.o sds.o adlist.o zmalloc.o redis-benchmark.o -REDIS_CHECK_DUMP_NAME=redis-check-dump -REDIS_CHECK_DUMP_OBJ=redis-check-dump.o lzf_c.o lzf_d.o crc64.o +REDIS_CHECK_RDB_NAME=redis-check-rdb REDIS_CHECK_AOF_NAME=redis-check-aof REDIS_CHECK_AOF_OBJ=redis-check-aof.o -all: $(REDIS_SERVER_NAME) $(REDIS_SENTINEL_NAME) $(REDIS_CLI_NAME) $(REDIS_BENCHMARK_NAME) $(REDIS_CHECK_DUMP_NAME) $(REDIS_CHECK_AOF_NAME) +all: $(REDIS_SERVER_NAME) $(REDIS_SENTINEL_NAME) $(REDIS_CLI_NAME) $(REDIS_BENCHMARK_NAME) $(REDIS_CHECK_RDB_NAME) $(REDIS_CHECK_AOF_NAME) @echo "" @echo "Hint: It's a good idea to run 'make test' ;)" @echo "" @@ -178,6 +177,10 @@ $(REDIS_SERVER_NAME): $(REDIS_SERVER_OBJ) $(REDIS_SENTINEL_NAME): $(REDIS_SERVER_NAME) $(REDIS_INSTALL) $(REDIS_SERVER_NAME) $(REDIS_SENTINEL_NAME) +# redis-check-rdb +$(REDIS_CHECK_RDB_NAME): $(REDIS_SERVER_NAME) + $(REDIS_INSTALL) $(REDIS_SERVER_NAME) $(REDIS_CHECK_RDB_NAME) + # redis-cli $(REDIS_CLI_NAME): $(REDIS_CLI_OBJ) $(REDIS_LD) -o $@ $^ ../deps/hiredis/libhiredis.a ../deps/linenoise/linenoise.o $(FINAL_LIBS) @@ -186,10 +189,6 @@ $(REDIS_CLI_NAME): $(REDIS_CLI_OBJ) $(REDIS_BENCHMARK_NAME): $(REDIS_BENCHMARK_OBJ) $(REDIS_LD) -o $@ $^ ../deps/hiredis/libhiredis.a $(FINAL_LIBS) -# redis-check-dump -$(REDIS_CHECK_DUMP_NAME): $(REDIS_CHECK_DUMP_OBJ) - $(REDIS_LD) -o $@ $^ $(FINAL_LIBS) - # redis-check-aof $(REDIS_CHECK_AOF_NAME): $(REDIS_CHECK_AOF_OBJ) $(REDIS_LD) -o $@ $^ $(FINAL_LIBS) @@ -201,7 +200,7 @@ $(REDIS_CHECK_AOF_NAME): $(REDIS_CHECK_AOF_OBJ) $(REDIS_CC) -c $< clean: - rm -rf $(REDIS_SERVER_NAME) $(REDIS_SENTINEL_NAME) $(REDIS_CLI_NAME) $(REDIS_BENCHMARK_NAME) $(REDIS_CHECK_DUMP_NAME) $(REDIS_CHECK_AOF_NAME) *.o *.gcda *.gcno *.gcov redis.info lcov-html + rm -rf $(REDIS_SERVER_NAME) $(REDIS_SENTINEL_NAME) $(REDIS_CLI_NAME) $(REDIS_BENCHMARK_NAME) $(REDIS_CHECK_RDB_NAME) $(REDIS_CHECK_AOF_NAME) *.o *.gcda *.gcno *.gcov redis.info lcov-html .PHONY: clean @@ -257,6 +256,6 @@ install: all $(REDIS_INSTALL) $(REDIS_SERVER_NAME) $(INSTALL_BIN) $(REDIS_INSTALL) $(REDIS_BENCHMARK_NAME) $(INSTALL_BIN) $(REDIS_INSTALL) $(REDIS_CLI_NAME) $(INSTALL_BIN) - $(REDIS_INSTALL) $(REDIS_CHECK_DUMP_NAME) $(INSTALL_BIN) + $(REDIS_INSTALL) $(REDIS_CHECK_RDB_NAME) $(INSTALL_BIN) $(REDIS_INSTALL) $(REDIS_CHECK_AOF_NAME) $(INSTALL_BIN) @ln -sf $(REDIS_SERVER_NAME) $(INSTALL_BIN)/$(REDIS_SENTINEL_NAME) diff --git a/src/redis-check-dump.c b/src/redis-check-rdb.c similarity index 94% rename from src/redis-check-dump.c rename to src/redis-check-rdb.c index 54646200..893a8185 100644 --- a/src/redis-check-dump.c +++ b/src/redis-check-rdb.c @@ -133,18 +133,13 @@ typedef struct { char success; } entry; -/* Global vars that are actually used as constants. The following double - * values are used for double on-disk serialization, and are initialized - * at runtime to avoid strange compiler optimizations. */ -static double R_Zero, R_PosInf, R_NegInf, R_Nan; - #define MAX_TYPES_NUM 256 #define MAX_TYPE_NAME_LEN 16 /* store string types for output */ static char types[MAX_TYPES_NUM][MAX_TYPE_NAME_LEN]; /* Return true if 't' is a valid object type. */ -int checkType(unsigned char t) { +static int checkType(unsigned char t) { /* In case a new object type is added, update the following * condition as necessary. */ return @@ -154,7 +149,7 @@ int checkType(unsigned char t) { } /* when number of bytes to read is negative, do a peek */ -int readBytes(void *target, long num) { +static int readBytes(void *target, long num) { char peek = (num < 0) ? 1 : 0; num = (num < 0) ? -num : num; @@ -188,7 +183,7 @@ int processHeader(void) { return dump_version; } -int loadType(entry *e) { +static int loadType(entry *e) { uint32_t offset = CURR_OFFSET; /* this byte needs to qualify as type */ @@ -208,7 +203,7 @@ int loadType(entry *e) { return 0; } -int peekType() { +static int peekType() { unsigned char t; if (readBytes(&t, -1) && (checkType(t))) return t; @@ -216,7 +211,7 @@ int peekType() { } /* discard time, just consume the bytes */ -int processTime(int type) { +static int processTime(int type) { uint32_t offset = CURR_OFFSET; unsigned char t[8]; int timelen = (type == REDIS_EXPIRETIME_MS) ? 8 : 4; @@ -231,7 +226,7 @@ int processTime(int type) { return 0; } -uint32_t loadLength(int *isencoded) { +static uint32_t loadLength(int *isencoded) { unsigned char buf[2]; uint32_t len; int type; @@ -257,7 +252,7 @@ uint32_t loadLength(int *isencoded) { } } -char *loadIntegerObject(int enctype) { +static char *loadIntegerObject(int enctype) { uint32_t offset = CURR_OFFSET; unsigned char enc[4]; long long val; @@ -289,7 +284,7 @@ char *loadIntegerObject(int enctype) { return buf; } -char* loadLzfStringObject() { +static char* loadLzfStringObject() { unsigned int slen, clen; char *c, *s; @@ -313,7 +308,7 @@ char* loadLzfStringObject() { } /* returns NULL when not processable, char* when valid */ -char* loadStringObject() { +static char* loadStringObject() { uint32_t offset = CURR_OFFSET; int isencoded; uint32_t len; @@ -336,7 +331,7 @@ char* loadStringObject() { if (len == REDIS_RDB_LENERR) return NULL; - char *buf = malloc(sizeof(char) * (len+1)); + char *buf = zmalloc(sizeof(char) * (len+1)); if (buf == NULL) return NULL; buf[len] = '\0'; if (!readBytes(buf, len)) { @@ -346,7 +341,7 @@ char* loadStringObject() { return buf; } -int processStringObject(char** store) { +static int processStringObject(char** store) { unsigned long offset = CURR_OFFSET; char *key = loadStringObject(); if (key == NULL) { @@ -363,7 +358,7 @@ int processStringObject(char** store) { return 1; } -double* loadDoubleValue() { +static double* loadDoubleValue() { char buf[256]; unsigned char len; double* val; @@ -386,7 +381,7 @@ double* loadDoubleValue() { } } -int processDoubleValue(double** store) { +static int processDoubleValue(double** store) { unsigned long offset = CURR_OFFSET; double *val = loadDoubleValue(); if (val == NULL) { @@ -403,7 +398,7 @@ int processDoubleValue(double** store) { return 1; } -int loadPair(entry *e) { +static int loadPair(entry *e) { uint32_t offset = CURR_OFFSET; uint32_t i; @@ -486,7 +481,7 @@ int loadPair(entry *e) { return 1; } -entry loadEntry() { +static entry loadEntry() { entry e = { NULL, -1, 0 }; uint32_t length, offset[4]; @@ -544,7 +539,7 @@ entry loadEntry() { return e; } -void printCentered(int indent, int width, char* body) { +static void printCentered(int indent, int width, char* body) { char head[256], tail[256]; memset(head, '\0', 256); memset(tail, '\0', 256); @@ -554,21 +549,21 @@ void printCentered(int indent, int width, char* body) { printf("%s %s %s\n", head, body, tail); } -void printValid(uint64_t ops, uint64_t bytes) { +static void printValid(uint64_t ops, uint64_t bytes) { char body[80]; sprintf(body, "Processed %llu valid opcodes (in %llu bytes)", (unsigned long long) ops, (unsigned long long) bytes); printCentered(4, 80, body); } -void printSkipped(uint64_t bytes, uint64_t offset) { +static void printSkipped(uint64_t bytes, uint64_t offset) { char body[80]; sprintf(body, "Skipped %llu bytes (resuming at 0x%08llx)", (unsigned long long) bytes, (unsigned long long) offset); printCentered(4, 80, body); } -void printErrorStack(entry *e) { +static void printErrorStack(entry *e) { unsigned int i; char body[64]; @@ -708,24 +703,18 @@ void process(void) { } } -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); - } - +int redis_check_rdb(char *rdbfilename) { int fd; off_t size; struct stat stat; void *data; - fd = open(argv[1], O_RDONLY); + fd = open(rdbfilename, O_RDONLY); if (fd < 1) { - ERROR("Cannot open file: %s\n", argv[1]); + ERROR("Cannot open file: %s\n", rdbfilename); } if (fstat(fd, &stat) == -1) { - ERROR("Cannot stat: %s\n", argv[1]); + ERROR("Cannot stat: %s\n", rdbfilename); } else { size = stat.st_size; } @@ -736,7 +725,7 @@ int main(int argc, char **argv) { data = mmap(NULL, size, PROT_READ, MAP_SHARED, fd, 0); if (data == MAP_FAILED) { - ERROR("Cannot mmap: %s\n", argv[1]); + ERROR("Cannot mmap: %s\n", rdbfilename); } /* Initialize static vars */ diff --git a/src/redis.c b/src/redis.c index 13df8d28..24739cc3 100644 --- a/src/redis.c +++ b/src/redis.c @@ -3550,6 +3550,17 @@ int checkForSentinelMode(int argc, char **argv) { return 0; } +/* Returns 1 if there is --check-rdb among the arguments or if + * argv[0] is exactly "redis-check-rdb". */ +int checkForCheckRDBMode(int argc, char **argv) { + int j; + + if (strstr(argv[0],"redis-check-rdb") != NULL) return 1; + for (j = 1; j < argc; j++) + if (!strcmp(argv[j],"--check-rdb")) return 1; + return 0; +} + /* Function called at startup to load RDB or AOF file in memory. */ void loadDataFromDisk(void) { long long start = ustime(); @@ -3766,6 +3777,11 @@ int main(int argc, char **argv) { while(j != argc) { if (argv[j][0] == '-' && argv[j][1] == '-') { /* Option name */ + if (!strcmp(argv[j], "--check-rdb")) { + /* Argument has no options, need to skip for parsing. */ + j++; + continue; + } if (sdslen(options)) options = sdscat(options,"\n"); options = sdscat(options,argv[j]+2); options = sdscat(options," "); @@ -3791,9 +3807,17 @@ int main(int argc, char **argv) { redisLog(REDIS_WARNING, "Warning: no config file specified, using the default config. In order to specify a config file use %s /path/to/%s.conf", argv[0], server.sentinel_mode ? "sentinel" : "redis"); } + if (checkForCheckRDBMode(argc, argv)) { + redisLog(REDIS_WARNING, "Checking RDB file %s", server.rdb_filename); + redisLog(REDIS_WARNING, "To check different RDB file: " + "redis-check-rdb --dbfilename "); + exit(redis_check_rdb(server.rdb_filename)); + } + server.supervised = redisIsSupervised(server.supervised_mode); int background = server.daemonize && !server.supervised; if (background) daemonize(); + initServer(); if (background || server.pidfile) createPidFile(); redisSetProcTitle(argv[0]); diff --git a/src/redis.h b/src/redis.h index 0c191d06..87bb811b 100644 --- a/src/redis.h +++ b/src/redis.h @@ -1380,6 +1380,9 @@ void sentinelTimer(void); char *sentinelHandleConfiguration(char **argv, int argc); void sentinelIsRunning(void); +/* redis-check-rdb */ +int redis_check_rdb(char *rdbfilename); + /* Scripting */ void scriptingInit(void); From 764b000c3e7fcb88bdef51b4d3339fa9f620ee07 Mon Sep 17 00:00:00 2001 From: Matt Stancliff Date: Mon, 12 May 2014 12:12:48 -0400 Subject: [PATCH 2/3] Remove code duplication from check-rdb redis-check-rdb (previously redis-check-dump) had every RDB define copy/pasted from rdb.h and some defines copied from redis.h. Since the initial copy, some constants had changed in Redis headers and check-dump was using incorrect values. Since check-rdb is now a mode of Redis, the old check-dump code is cleaned up to: - replace all printf with redisLog (and remove \n from all strings) - remove all copy/pasted defines to use defines from rdb.h and redis.h - replace all malloc/free with zmalloc/zfree - remove unnecessary include headers --- src/redis-check-rdb.c | 188 ++++++++++++++---------------------------- 1 file changed, 63 insertions(+), 125 deletions(-) diff --git a/src/redis-check-rdb.c b/src/redis-check-rdb.c index 893a8185..c3420420 100644 --- a/src/redis-check-rdb.c +++ b/src/redis-check-rdb.c @@ -29,74 +29,19 @@ */ +#include "redis.h" +#include "rdb.h" #include #include #include #include #include #include -#include -#include -#include -#include #include "lzf.h" #include "crc64.h" -/* Object types */ -#define REDIS_STRING 0 -#define REDIS_LIST 1 -#define REDIS_SET 2 -#define REDIS_ZSET 3 -#define REDIS_HASH 4 -#define REDIS_HASH_ZIPMAP 9 -#define REDIS_LIST_ZIPLIST 10 -#define REDIS_SET_INTSET 11 -#define REDIS_ZSET_ZIPLIST 12 -#define REDIS_HASH_ZIPLIST 13 - -/* Objects encoding. Some kind of objects like Strings and Hashes can be - * internally represented in multiple ways. The 'encoding' field of the object - * is set to one of this fields for this object. */ -#define REDIS_ENCODING_RAW 0 /* Raw representation */ -#define REDIS_ENCODING_INT 1 /* Encoded as integer */ -#define REDIS_ENCODING_ZIPMAP 2 /* Encoded as zipmap */ -#define REDIS_ENCODING_HT 3 /* Encoded as a hash table */ - -/* Object types only used for dumping to disk */ -#define REDIS_EXPIRETIME_MS 252 -#define REDIS_EXPIRETIME 253 -#define REDIS_SELECTDB 254 -#define REDIS_EOF 255 - -/* Defines related to the dump file format. To store 32 bits lengths for short - * keys requires a lot of space, so we check the most significant 2 bits of - * the first byte to interpreter the length: - * - * 00|000000 => if the two MSB are 00 the len is the 6 bits of this byte - * 01|000000 00000000 => 01, the len is 14 byes, 6 bits + 8 bits of next byte - * 10|000000 [32 bit integer] => if it's 01, a full 32 bit len will follow - * 11|000000 this means: specially encoded object will follow. The six bits - * number specify the kind of object that follows. - * See the REDIS_RDB_ENC_* defines. - * - * Lengths up to 63 are stored using a single byte, most DB keys, and may - * values, will fit inside. */ -#define REDIS_RDB_6BITLEN 0 -#define REDIS_RDB_14BITLEN 1 -#define REDIS_RDB_32BITLEN 2 -#define REDIS_RDB_ENCVAL 3 -#define REDIS_RDB_LENERR UINT_MAX - -/* When a length of a string object stored on disk has the first two bits - * set, the remaining two bits specify a special encoding for the object - * accordingly to the following defines: */ -#define REDIS_RDB_ENC_INT8 0 /* 8 bit signed integer */ -#define REDIS_RDB_ENC_INT16 1 /* 16 bit signed integer */ -#define REDIS_RDB_ENC_INT32 2 /* 32 bit signed integer */ -#define REDIS_RDB_ENC_LZF 3 /* string compressed with FASTLZ */ - #define ERROR(...) { \ - printf(__VA_ARGS__); \ + redisLog(REDIS_WARNING, __VA_ARGS__); \ exit(1); \ } @@ -139,13 +84,13 @@ typedef struct { static char types[MAX_TYPES_NUM][MAX_TYPE_NAME_LEN]; /* Return true if 't' is a valid object type. */ -static int checkType(unsigned char t) { +static int rdbCheckType(unsigned char t) { /* In case a new object type is added, update the following * condition as necessary. */ return - (t >= REDIS_HASH_ZIPMAP && t <= REDIS_HASH_ZIPLIST) || - t <= REDIS_HASH || - t >= REDIS_EXPIRETIME_MS; + (t >= REDIS_RDB_TYPE_HASH_ZIPMAP && t <= REDIS_RDB_TYPE_HASH_ZIPLIST) || + t <= REDIS_RDB_TYPE_HASH || + t >= REDIS_RDB_OPCODE_EXPIRETIME_MS; } /* when number of bytes to read is negative, do a peek */ @@ -168,17 +113,17 @@ int processHeader(void) { int dump_version; if (!readBytes(buf, 9)) { - ERROR("Cannot read header\n"); + ERROR("Cannot read header"); } /* expect the first 5 bytes to equal REDIS */ if (memcmp(buf,"REDIS",5) != 0) { - ERROR("Wrong signature in header\n"); + ERROR("Wrong signature in header"); } dump_version = (int)strtol(buf + 5, NULL, 10); if (dump_version < 1 || dump_version > 6) { - ERROR("Unknown RDB format version: %d\n", dump_version); + ERROR("Unknown RDB format version: %d", dump_version); } return dump_version; } @@ -189,7 +134,7 @@ static int loadType(entry *e) { /* this byte needs to qualify as type */ unsigned char t; if (readBytes(&t, 1)) { - if (checkType(t)) { + if (rdbCheckType(t)) { e->type = t; return 1; } else { @@ -205,7 +150,7 @@ static int loadType(entry *e) { static int peekType() { unsigned char t; - if (readBytes(&t, -1) && (checkType(t))) + if (readBytes(&t, -1) && (rdbCheckType(t))) return t; return -1; } @@ -214,7 +159,7 @@ static int peekType() { static int processTime(int type) { uint32_t offset = CURR_OFFSET; unsigned char t[8]; - int timelen = (type == REDIS_EXPIRETIME_MS) ? 8 : 4; + int timelen = (type == REDIS_RDB_OPCODE_EXPIRETIME_MS) ? 8 : 4; if (readBytes(t,timelen)) { return 1; @@ -279,7 +224,7 @@ static char *loadIntegerObject(int enctype) { /* convert val into string */ char *buf; - buf = malloc(sizeof(char) * 128); + buf = zmalloc(sizeof(char) * 128); sprintf(buf, "%lld", val); return buf; } @@ -291,19 +236,19 @@ static char* loadLzfStringObject() { if ((clen = loadLength(NULL)) == REDIS_RDB_LENERR) return NULL; if ((slen = loadLength(NULL)) == REDIS_RDB_LENERR) return NULL; - c = malloc(clen); + c = zmalloc(clen); if (!readBytes(c, clen)) { - free(c); + zfree(c); return NULL; } - s = malloc(slen+1); + s = zmalloc(slen+1); if (lzf_decompress(c,clen,s,slen) == 0) { - free(c); free(s); + zfree(c); zfree(s); return NULL; } - free(c); + zfree(c); return s; } @@ -335,7 +280,7 @@ static char* loadStringObject() { if (buf == NULL) return NULL; buf[len] = '\0'; if (!readBytes(buf, len)) { - free(buf); + zfree(buf); return NULL; } return buf; @@ -346,14 +291,14 @@ static int processStringObject(char** store) { char *key = loadStringObject(); if (key == NULL) { SHIFT_ERROR(offset, "Error reading string object"); - free(key); + zfree(key); return 0; } if (store != NULL) { *store = key; } else { - free(key); + zfree(key); } return 1; } @@ -365,14 +310,14 @@ static double* loadDoubleValue() { if (!readBytes(&len,1)) return NULL; - val = malloc(sizeof(double)); + val = zmalloc(sizeof(double)); switch(len) { case 255: *val = R_NegInf; return val; case 254: *val = R_PosInf; return val; case 253: *val = R_Nan; return val; default: if (!readBytes(buf, len)) { - free(val); + zfree(val); return NULL; } buf[len] = '\0'; @@ -386,14 +331,14 @@ static int processDoubleValue(double** store) { double *val = loadDoubleValue(); if (val == NULL) { SHIFT_ERROR(offset, "Error reading double value"); - free(val); + zfree(val); return 0; } if (store != NULL) { *store = val; } else { - free(val); + zfree(val); } return 1; } @@ -412,10 +357,10 @@ static int loadPair(entry *e) { } uint32_t length = 0; - if (e->type == REDIS_LIST || - e->type == REDIS_SET || - e->type == REDIS_ZSET || - e->type == REDIS_HASH) { + if (e->type == REDIS_RDB_TYPE_LIST || + e->type == REDIS_RDB_TYPE_SET || + e->type == REDIS_RDB_TYPE_ZSET || + e->type == REDIS_RDB_TYPE_HASH) { if ((length = loadLength(NULL)) == REDIS_RDB_LENERR) { SHIFT_ERROR(offset, "Error reading %s length", types[e->type]); return 0; @@ -423,19 +368,19 @@ static int loadPair(entry *e) { } switch(e->type) { - case REDIS_STRING: - case REDIS_HASH_ZIPMAP: - case REDIS_LIST_ZIPLIST: - case REDIS_SET_INTSET: - case REDIS_ZSET_ZIPLIST: - case REDIS_HASH_ZIPLIST: + case REDIS_RDB_TYPE_STRING: + case REDIS_RDB_TYPE_HASH_ZIPMAP: + case REDIS_RDB_TYPE_LIST_ZIPLIST: + case REDIS_RDB_TYPE_SET_INTSET: + case REDIS_RDB_TYPE_ZSET_ZIPLIST: + case REDIS_RDB_TYPE_HASH_ZIPLIST: if (!processStringObject(NULL)) { SHIFT_ERROR(offset, "Error reading entry value"); return 0; } break; - case REDIS_LIST: - case REDIS_SET: + case REDIS_RDB_TYPE_LIST: + case REDIS_RDB_TYPE_SET: for (i = 0; i < length; i++) { offset = CURR_OFFSET; if (!processStringObject(NULL)) { @@ -444,7 +389,7 @@ static int loadPair(entry *e) { } } break; - case REDIS_ZSET: + case REDIS_RDB_TYPE_ZSET: for (i = 0; i < length; i++) { offset = CURR_OFFSET; if (!processStringObject(NULL)) { @@ -458,7 +403,7 @@ static int loadPair(entry *e) { } } break; - case REDIS_HASH: + case REDIS_RDB_TYPE_HASH: for (i = 0; i < length; i++) { offset = CURR_OFFSET; if (!processStringObject(NULL)) { @@ -494,7 +439,7 @@ static entry loadEntry() { } offset[1] = CURR_OFFSET; - if (e.type == REDIS_SELECTDB) { + if (e.type == REDIS_RDB_OPCODE_SELECTDB) { if ((length = loadLength(NULL)) == REDIS_RDB_LENERR) { SHIFT_ERROR(offset[1], "Error reading database number"); return e; @@ -503,7 +448,7 @@ static entry loadEntry() { SHIFT_ERROR(offset[1], "Database number out of range (%d)", length); return e; } - } else if (e.type == REDIS_EOF) { + } else if (e.type == REDIS_RDB_OPCODE_EOF) { if (positions[level].offset < positions[level].size) { SHIFT_ERROR(offset[0], "Unexpected EOF"); } else { @@ -512,8 +457,8 @@ static entry loadEntry() { return e; } else { /* optionally consume expire */ - if (e.type == REDIS_EXPIRETIME || - e.type == REDIS_EXPIRETIME_MS) { + if (e.type == REDIS_RDB_OPCODE_EXPIRETIME || + e.type == REDIS_RDB_OPCODE_EXPIRETIME_MS) { if (!processTime(e.type)) return e; if (!loadType(&e)) return e; } @@ -546,7 +491,7 @@ static void printCentered(int indent, int width, char* body) { memset(head, '=', indent); memset(tail, '=', width - 2 - indent - strlen(body)); - printf("%s %s %s\n", head, body, tail); + redisLog(REDIS_WARNING, "%s %s %s", head, body, tail); } static void printValid(uint64_t ops, uint64_t bytes) { @@ -593,7 +538,7 @@ static void printErrorStack(entry *e) { /* display error stack */ for (i = 0; i < errors.level; i++) { - printf("0x%08lx - %s\n", + redisLog(REDIS_WARNING, "0x%08lx - %s", (unsigned long) errors.offset[i], errors.error[i]); } } @@ -606,7 +551,7 @@ void process(void) { /* Exclude the final checksum for RDB >= 5. Will be checked at the end. */ if (dump_version >= 5) { if (positions[0].size < 8) { - printf("RDB version >= 5 but no room for checksum.\n"); + redisLog(REDIS_WARNING, "RDB version >= 5 but no room for checksum."); exit(1); } positions[0].size -= 8; @@ -655,7 +600,7 @@ void process(void) { /* advance position */ positions[0] = positions[1]; } - free(entry.key); + zfree(entry.key); } /* because there is another potential error, @@ -663,7 +608,7 @@ void process(void) { printValid(num_valid_ops, num_valid_bytes); /* expect an eof */ - if (entry.type != REDIS_EOF) { + if (entry.type != REDIS_RDB_OPCODE_EOF) { /* last byte should be EOF, add error */ errors.level = 0; SHIFT_ERROR(positions[0].offset, "Expected EOF, got %s", types[entry.type]); @@ -691,14 +636,13 @@ void process(void) { if (crc != crc2) { SHIFT_ERROR(positions[0].offset, "RDB CRC64 does not match."); } else { - printf("CRC64 checksum is OK\n"); + redisLog(REDIS_WARNING, "CRC64 checksum is OK"); } } /* print summary on errors */ if (num_errors) { - printf("\n"); - printf("Total unprocessable opcodes: %llu\n", + redisLog(REDIS_WARNING, "Total unprocessable opcodes: %llu", (unsigned long long) num_errors); } } @@ -711,21 +655,21 @@ int redis_check_rdb(char *rdbfilename) { fd = open(rdbfilename, O_RDONLY); if (fd < 1) { - ERROR("Cannot open file: %s\n", rdbfilename); + ERROR("Cannot open file: %s", rdbfilename); } if (fstat(fd, &stat) == -1) { - ERROR("Cannot stat: %s\n", rdbfilename); + ERROR("Cannot stat: %s", rdbfilename); } else { size = stat.st_size; } if (sizeof(size_t) == sizeof(int32_t) && size >= INT_MAX) { - ERROR("Cannot check dump files >2GB on a 32-bit platform\n"); + ERROR("Cannot check dump files >2GB on a 32-bit platform"); } data = mmap(NULL, size, PROT_READ, MAP_SHARED, fd, 0); if (data == MAP_FAILED) { - ERROR("Cannot mmap: %s\n", rdbfilename); + ERROR("Cannot mmap: %s", rdbfilename); } /* Initialize static vars */ @@ -735,22 +679,16 @@ int redis_check_rdb(char *rdbfilename) { errors.level = 0; /* Object types */ - sprintf(types[REDIS_STRING], "STRING"); - sprintf(types[REDIS_LIST], "LIST"); - sprintf(types[REDIS_SET], "SET"); - sprintf(types[REDIS_ZSET], "ZSET"); - sprintf(types[REDIS_HASH], "HASH"); + sprintf(types[REDIS_RDB_TYPE_STRING], "STRING"); + sprintf(types[REDIS_RDB_TYPE_LIST], "LIST"); + sprintf(types[REDIS_RDB_TYPE_SET], "SET"); + sprintf(types[REDIS_RDB_TYPE_ZSET], "ZSET"); + sprintf(types[REDIS_RDB_TYPE_HASH], "HASH"); /* Object types only used for dumping to disk */ - sprintf(types[REDIS_EXPIRETIME], "EXPIRETIME"); - sprintf(types[REDIS_SELECTDB], "SELECTDB"); - sprintf(types[REDIS_EOF], "EOF"); - - /* Double constants initialization */ - R_Zero = 0.0; - R_PosInf = 1.0/R_Zero; - R_NegInf = -1.0/R_Zero; - R_Nan = R_Zero/R_Zero; + sprintf(types[REDIS_RDB_OPCODE_EXPIRETIME], "EXPIRETIME"); + sprintf(types[REDIS_RDB_OPCODE_SELECTDB], "SELECTDB"); + sprintf(types[REDIS_RDB_OPCODE_EOF], "EOF"); process(); From d8c7db1bdba3938f31856a067b2966285acbf97f Mon Sep 17 00:00:00 2001 From: Matt Stancliff Date: Mon, 12 May 2014 11:44:37 -0400 Subject: [PATCH 3/3] Improve RDB error-on-load handling Previouly if we loaded a corrupt RDB, Redis printed an error report with a big "REPORT ON GITHUB" message at the bottom. But, we know RDB load failures are corrupt data, not corrupt code. Now when RDB failure is detected (duplicate keys or unknown data types in the file), we run check-rdb against the RDB then exit. The automatic check-rdb hopefully gives the user instant feedback about what is wrong instead of providing a mysterious stack trace. --- src/rdb.c | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/src/rdb.c b/src/rdb.c index d5e3a7f4..3cf8344a 100644 --- a/src/rdb.c +++ b/src/rdb.c @@ -44,6 +44,16 @@ #define RDB_LOAD_ENC (1<<0) #define RDB_LOAD_PLAIN (1<<1) +#define rdbExitReportCorruptRDB(reason) rdbCheckThenExit(reason, __LINE__); + +void rdbCheckThenExit(char *reason, int where) { + redisLog(REDIS_WARNING, "Corrupt RDB detected at rdb.c:%d (%s). " + "Running 'redis-check-rdb --dbfilename %s'", + where, reason, server.rdb_filename); + redis_check_rdb(server.rdb_filename); + exit(1); +} + static int rdbWriteRaw(rio *rdb, void *p, size_t len) { if (rdb && rioWrite(rdb,p,len) == 0) return -1; @@ -188,7 +198,7 @@ void *rdbLoadIntegerObject(rio *rdb, int enctype, int flags) { val = (int32_t)v; } else { val = 0; /* anti-warning */ - redisPanic("Unknown RDB integer encoding type"); + rdbExitReportCorruptRDB("Unknown RDB integer encoding type"); } if (plain) { char buf[REDIS_LONGSTR_SIZE], *p; @@ -394,7 +404,7 @@ void *rdbGenericLoadStringObject(rio *rdb, int flags) { case REDIS_RDB_ENC_LZF: return rdbLoadLzfStringObject(rdb,flags); default: - redisPanic("Unknown RDB encoding type"); + rdbExitReportCorruptRDB("Unknown RDB encoding type"); } } @@ -923,7 +933,7 @@ void rdbRemoveTempFile(pid_t childpid) { /* Load a Redis object of the specified type from the specified file. * On success a newly allocated object is returned, otherwise NULL. */ robj *rdbLoadObject(int rdbtype, rio *rdb) { - robj *o, *ele, *dec; + robj *o = NULL, *ele, *dec; size_t len; unsigned int i; @@ -1078,7 +1088,9 @@ robj *rdbLoadObject(int rdbtype, rio *rdb) { /* Add pair to hash table */ ret = dictAdd((dict*)o->ptr, field, value); - redisAssert(ret == DICT_OK); + if (ret == DICT_ERR) { + rdbExitReportCorruptRDB("Duplicate keys detected"); + } } /* All pairs should be read by now */ @@ -1164,11 +1176,11 @@ robj *rdbLoadObject(int rdbtype, rio *rdb) { hashTypeConvert(o, REDIS_ENCODING_HT); break; default: - redisPanic("Unknown encoding"); + rdbExitReportCorruptRDB("Unknown encoding"); break; } } else { - redisPanic("Unknown object type"); + rdbExitReportCorruptRDB("Unknown object type"); } return o; }