gvsafronov/futriix
1
0
Fork 0
Code Issues Pull Requests Actions 8 Packages Projects Releases Wiki Activity

cleanup around loadAppendOnlyFile (#9012)

Browse Source 
Today when we load the AOF on startup, the loadAppendOnlyFile checks if
the file is openning for reading.
This check is redundent (dead code) as we open the AOF file for writing at initServer,
and the file will always be existing for the loadAppendOnlyFile.

In this commit:
- remove all the exit(1) from loadAppendOnlyFile, as it is the caller
  responsibility to decide what to do in case of failure.
- move the opening of the AOF file for writing, to be after we loading it.
- avoid return -ERR in DEBUG LOADAOF, when the AOF is existing but empty
This commit is contained in:
YaacovHazan 2021-06-14 10:38:08 +03:00 committed by GitHub
parent 2727a2cec0
commit 1677efb9da
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 81 additions and 35 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

51
src/aof.c
View File

@ -700,9 +700,12 @@ void freeFakeClient(struct client *c) {
zfree(c);
}
/* Replay the append log file. On success C_OK is returned. On non fatal
* error (the append only file is zero-length) C_ERR is returned. On
* fatal error an error message is logged and the program exists. */
/* Replay the append log file. On success AOF_OK is returned,
* otherwise, one of the following is returned:
* AOF_OPEN_ERR: Failed to open the AOF file.
* AOF_NOT_EXIST: AOF file doesn't exist.
* AOF_EMPTY: The AOF file is empty (nothing to load).
* AOF_FAILED: Failed to load the AOF file. */
int loadAppendOnlyFile(char *filename) {
struct client *fakeClient;
FILE *fp = fopen(filename,"r");
@ -711,10 +714,17 @@ int loadAppendOnlyFile(char *filename) {
long loops = 0;
off_t valid_up_to = 0; /* Offset of latest well-formed command loaded. */
off_t valid_before_multi = 0; /* Offset before MULTI command loaded. */
int ret;
if (fp == NULL) {
serverLog(LL_WARNING,"Fatal error: can't open the append log file for reading: %s",strerror(errno));
exit(1);
int en = errno;
if (redis_stat(filename, &sb) == 0) {
serverLog(LL_WARNING,"Fatal error: can't open the append log file for reading: %s",strerror(en));
return AOF_OPEN_ERR;
} else {
serverLog(LL_WARNING,"The append log file doesn't exist: %s",strerror(errno));
return AOF_NOT_EXIST;
}
}
/* Handle a zero-length AOF file as a special case. An empty AOF file
@ -725,7 +735,7 @@ int loadAppendOnlyFile(char *filename) {
server.aof_current_size = 0;
server.aof_fsync_offset = server.aof_current_size;
fclose(fp);
return C_ERR;
return AOF_EMPTY;
}
/* Temporarily disable AOF, to prevent EXEC from feeding a MULTI
@ -826,7 +836,9 @@ int loadAppendOnlyFile(char *filename) {
serverLog(LL_WARNING,
"Unknown command '%s' reading the append only file",
(char*)argv[0]->ptr);
exit(1);
freeFakeClientArgv(fakeClient);
ret = AOF_FAILED;
goto cleanup;
}
if (cmd == server.multiCommand) valid_before_multi = valid_up_to;
@ -869,21 +881,18 @@ int loadAppendOnlyFile(char *filename) {
}
loaded_ok: /* DB loaded, cleanup and return C_OK to the caller. */
fclose(fp);
freeFakeClient(fakeClient);
server.aof_state = old_aof_state;
stopLoading(1);
aofUpdateCurrentSize();
server.aof_rewrite_base_size = server.aof_current_size;
server.aof_fsync_offset = server.aof_current_size;
return C_OK;
ret = AOF_OK;
goto cleanup;
readerr: /* Read error. If feof(fp) is true, fall through to unexpected EOF. */
if (!feof(fp)) {
if (fakeClient) freeFakeClient(fakeClient); /* avoid valgrind warning */
fclose(fp);
serverLog(LL_WARNING,"Unrecoverable error reading the append only file: %s", strerror(errno));
exit(1);
ret = AOF_FAILED;
goto cleanup;
}
uxeof: /* Unexpected AOF end of file. */
@ -911,16 +920,20 @@ uxeof: /* Unexpected AOF end of file. */
}
}
}
if (fakeClient) freeFakeClient(fakeClient); /* avoid valgrind warning */
fclose(fp);
serverLog(LL_WARNING,"Unexpected end of file reading the append only file. You can: 1) Make a backup of your AOF file, then use ./redis-check-aof --fix <filename>. 2) Alternatively you can set the 'aof-load-truncated' configuration option to yes and restart the server.");
exit(1);
ret = AOF_FAILED;
goto cleanup;
fmterr: /* Format error. */
serverLog(LL_WARNING,"Bad file format reading the append only file: make a backup of your AOF file, then use ./redis-check-aof --fix <filename>");
ret = AOF_FAILED;
/* fall through to cleanup. */
cleanup:
if (fakeClient) freeFakeClient(fakeClient); /* avoid valgrind warning */
fclose(fp);
serverLog(LL_WARNING,"Bad file format reading the append only file: make a backup of your AOF file, then use ./redis-check-aof --fix <filename>");
exit(1);
stopLoading(ret == AOF_OK);
return ret;
}
/* ----------------------------------------------------------------------------

6
src/debug.c
View File

@ -551,11 +551,9 @@ NULL
emptyDb(-1,EMPTYDB_NO_FLAGS,NULL);
protectClient(c);
int ret = loadAppendOnlyFile(server.aof_filename);
if (ret != AOF_OK && ret != AOF_EMPTY)
exit(1);
unprotectClient(c);
if (ret != C_OK) {
addReplyErrorObject(c,shared.err);
return;
}
server.dirty = 0; /* Prevent AOF / replication */
serverLog(LL_WARNING,"Append Only File loaded by DEBUG LOADAOF");
addReply(c,shared.ok);

27
src/server.c
View File

@ -3342,17 +3342,6 @@ void initServer(void) {
aeSetBeforeSleepProc(server.el,beforeSleep);
aeSetAfterSleepProc(server.el,afterSleep);
/* Open the AOF file if needed. */
if (server.aof_state == AOF_ON) {
server.aof_fd = open(server.aof_filename,
O_WRONLY|O_APPEND|O_CREAT,0644);
if (server.aof_fd == -1) {
serverLog(LL_WARNING, "Can't open the append-only file: %s",
strerror(errno));
exit(1);
}
}
/* 32 bit instances are limited to 4GB of address space, so if there is
* no explicit limit in the user provided configuration we set a limit
* at 3 GB using maxmemory with 'noeviction' policy'. This avoids
@ -5936,7 +5925,11 @@ int checkForSentinelMode(int argc, char **argv) {
void loadDataFromDisk(void) {
long long start = ustime();
if (server.aof_state == AOF_ON) {
if (loadAppendOnlyFile(server.aof_filename) == C_OK)
/* It's not a failure if the file is empty or doesn't exist (later we will create it) */
int ret = loadAppendOnlyFile(server.aof_filename);
if (ret == AOF_FAILED || ret == AOF_OPEN_ERR)
exit(1);
if (ret == AOF_OK)
serverLog(LL_NOTICE,"DB loaded from append only file: %.3f seconds",(float)(ustime()-start)/1000000);
} else {
rdbSaveInfo rsi = RDB_SAVE_INFO_INIT;
@ -6370,6 +6363,16 @@ int main(int argc, char **argv) {
ACLLoadUsersAtStartup();
InitServerLast();
loadDataFromDisk();
/* Open the AOF file if needed. */
if (server.aof_state == AOF_ON) {
server.aof_fd = open(server.aof_filename,
O_WRONLY|O_APPEND|O_CREAT,0644);
if (server.aof_fd == -1) {
serverLog(LL_WARNING, "Can't open the append-only file: %s",
strerror(errno));
exit(1);
}
}
if (server.cluster_enabled) {
if (verifyClusterConfigWithData() == C_ERR) {
serverLog(LL_WARNING,

7
src/server.h
View File

@ -222,6 +222,13 @@ extern int configOOMScoreAdjValuesDefaults[CONFIG_OOM_COUNT];
#define AOF_ON 1 /* AOF is on */
#define AOF_WAIT_REWRITE 2 /* AOF waits rewrite to start appending */
/* AOF return values for loadAppendOnlyFile() */
#define AOF_OK 0
#define AOF_NOT_EXIST 1
#define AOF_EMPTY 2
#define AOF_OPEN_ERR 3
#define AOF_FAILED 4
/* Client flags */
#define CLIENT_SLAVE (1<<0) /* This client is a replica */
#define CLIENT_MASTER (1<<1) /* This client is a master */

25
tests/integration/aof.tcl
View File

@ -295,4 +295,29 @@ tags {"aof external:skip"} {
assert_equal $before $after
}
}
## Test that the server exits when the AOF contains a unknown command
create_aof {
append_to_aof [formatCommand set foo hello]
append_to_aof [formatCommand bla foo hello]
append_to_aof [formatCommand set foo hello]
}
start_server_aof [list dir $server_path aof-load-truncated yes] {
test "Unknown command: Server should have logged an error" {
set pattern "*Unknown command 'bla' reading the append only file*"
set retry 10
while {$retry} {
set result [exec tail -1 < [dict get $srv stdout]]
if {[string match $pattern $result]} {
break
}
incr retry -1
after 1000
}
if {$retry == 0} {
error "assertion:expected error not found on config file"
}
}
}
}