Skip to content

Commit

Permalink
Reformat remaining Valkey source code and set up GitHub checker action
Browse files Browse the repository at this point in the history
Signed-off-by: Ping Xie <pingxie@google.com>
  • Loading branch information
PingXie committed May 27, 2024
1 parent 11eee55 commit 3399c93
Show file tree
Hide file tree
Showing 13 changed files with 275 additions and 256 deletions.
17 changes: 0 additions & 17 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,23 +7,6 @@ permissions:

jobs:

checkers:
name: Run static checkers
runs-on: ubuntu-latest
strategy:
matrix:
path:
- check: 'src'
exclude: 'src/(lzf.*|crccombine.*|crc16.c|crc16_slottable.h|crc64.c|crcspeed.*|fmtargs.h|mt19937-64.*|pqsort.*|setcpuaffinity.c|setproctitle.c|sha1.*|sha256.*|siphash.c|strl.c|unit/test_files.h)'
steps:
- uses: actions/checkout@44c2b7a8a4ea60a981eaca3cf939b5f4305c123b # v4.1.5
- name: Run clang-format style check (.c and .h)
uses: jidicula/clang-format-action@f62da5e3d3a2d88ff364771d9d938773a618ab5e # v4.11.0
with:
clang-format-version: '13'
check-path: ${{ matrix.path['check'] }}
exclude-regex: ${{ matrix.path['exclude'] }}

test-ubuntu-latest:
runs-on: ubuntu-latest
steps:
Expand Down
35 changes: 35 additions & 0 deletions .github/workflows/clang-format.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
name: Clang Format Check

on:
pull_request:
paths:
- 'src/**'

jobs:
clang-format-check:
runs-on: ubuntu-latest

steps:
- name: Checkout code
uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1

- name: Set up Clang
run: |
sudo apt-get update -y
sudo apt-get upgrade -y
sudo apt-get install -y clang-format
- name: Run clang-format
id: clang-format
run: |
# Run clang-format and capture the diff
cd src
clang-format -i **/*.c **/*.h
git diff --exit-code
shell: bash

- name: Check for formatting changes
if: ${{ failure() }}
run: |
echo "Code is not formatted correctly."
exit 1
9 changes: 4 additions & 5 deletions src/aof.c
Original file line number Diff line number Diff line change
Expand Up @@ -987,8 +987,7 @@ int startAppendOnly(void) {
/* If AOF fsync error in bio job, we just ignore it and log the event. */
int aof_bio_fsync_status = atomic_load_explicit(&server.aof_bio_fsync_status, memory_order_relaxed);
if (aof_bio_fsync_status == C_ERR) {
serverLog(LL_WARNING,
"AOF reopen, just ignore the AOF fsync error in bio job");
serverLog(LL_WARNING, "AOF reopen, just ignore the AOF fsync error in bio job");
atomic_store_explicit(&server.aof_bio_fsync_status, C_OK, memory_order_relaxed);
}

Expand Down Expand Up @@ -1245,8 +1244,7 @@ void flushAppendOnlyFile(int force) {
server.aof_last_incr_fsync_offset = server.aof_last_incr_size;
server.aof_last_fsync = server.mstime;
atomic_store_explicit(&server.fsynced_reploff_pending, server.master_repl_offset, memory_order_relaxed);
} else if (server.aof_fsync == AOF_FSYNC_EVERYSEC &&
server.mstime - server.aof_last_fsync >= 1000) {
} else if (server.aof_fsync == AOF_FSYNC_EVERYSEC && server.mstime - server.aof_last_fsync >= 1000) {
if (!sync_in_progress) {
aof_background_fsync(server.aof_fd);
server.aof_last_incr_fsync_offset = server.aof_last_incr_size;
Expand Down Expand Up @@ -2648,7 +2646,8 @@ void backgroundRewriteDoneHandler(int exitcode, int bysignal) {
/* Update the fsynced replication offset that just now become valid.
* This could either be the one we took in startAppendOnly, or a
* newer one set by the bio thread. */
long long fsynced_reploff_pending = atomic_load_explicit(&server.fsynced_reploff_pending, memory_order_relaxed);
long long fsynced_reploff_pending =
atomic_load_explicit(&server.fsynced_reploff_pending, memory_order_relaxed);
server.fsynced_reploff = fsynced_reploff_pending;
}

Expand Down
4 changes: 1 addition & 3 deletions src/bio.c
Original file line number Diff line number Diff line change
Expand Up @@ -257,9 +257,7 @@ void *bioProcessBackgroundJobs(void *arg) {
/* The fd may be closed by main thread and reused for another
* socket, pipe, or file. We just ignore these errno because
* aof fsync did not really fail. */
if (valkey_fsync(job->fd_args.fd) == -1 &&
errno != EBADF && errno != EINVAL)
{
if (valkey_fsync(job->fd_args.fd) == -1 && errno != EBADF && errno != EINVAL) {
int last_status = atomic_load_explicit(&server.aof_bio_fsync_status, memory_order_relaxed);

atomic_store_explicit(&server.aof_bio_fsync_errno, errno, memory_order_relaxed);
Expand Down
63 changes: 31 additions & 32 deletions src/lazyfree.c
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ static _Atomic size_t lazyfreed_objects = 0;
void lazyfreeFreeObject(void *args[]) {
robj *o = (robj *)args[0];
decrRefCount(o);
atomic_fetch_sub_explicit(&lazyfree_objects,1,memory_order_relaxed);
atomic_fetch_add_explicit(&lazyfreed_objects,1,memory_order_relaxed);
atomic_fetch_sub_explicit(&lazyfree_objects, 1, memory_order_relaxed);
atomic_fetch_add_explicit(&lazyfreed_objects, 1, memory_order_relaxed);
}

/* Release a database from the lazyfree thread. The 'db' pointer is the
Expand All @@ -27,26 +27,26 @@ void lazyfreeFreeDatabase(void *args[]) {
size_t numkeys = kvstoreSize(da1);
kvstoreRelease(da1);
kvstoreRelease(da2);
atomic_fetch_sub_explicit(&lazyfree_objects,numkeys,memory_order_relaxed);
atomic_fetch_add_explicit(&lazyfreed_objects,numkeys,memory_order_relaxed);
atomic_fetch_sub_explicit(&lazyfree_objects, numkeys, memory_order_relaxed);
atomic_fetch_add_explicit(&lazyfreed_objects, numkeys, memory_order_relaxed);
}

/* Release the key tracking table. */
void lazyFreeTrackingTable(void *args[]) {
rax *rt = args[0];
size_t len = rt->numele;
freeTrackingRadixTree(rt);
atomic_fetch_sub_explicit(&lazyfree_objects,len,memory_order_relaxed);
atomic_fetch_add_explicit(&lazyfreed_objects,len,memory_order_relaxed);
atomic_fetch_sub_explicit(&lazyfree_objects, len, memory_order_relaxed);
atomic_fetch_add_explicit(&lazyfreed_objects, len, memory_order_relaxed);
}

/* Release the error stats rax tree. */
void lazyFreeErrors(void *args[]) {
rax *errors = args[0];
size_t len = errors->numele;
raxFreeWithCallback(errors, zfree);
atomic_fetch_sub_explicit(&lazyfree_objects,len,memory_order_relaxed);
atomic_fetch_add_explicit(&lazyfreed_objects,len,memory_order_relaxed);
atomic_fetch_sub_explicit(&lazyfree_objects, len, memory_order_relaxed);
atomic_fetch_add_explicit(&lazyfreed_objects, len, memory_order_relaxed);
}

/* Release the lua_scripts dict. */
Expand All @@ -56,17 +56,17 @@ void lazyFreeLuaScripts(void *args[]) {
lua_State *lua = args[2];
long long len = dictSize(lua_scripts);
freeLuaScriptsSync(lua_scripts, lua_scripts_lru_list, lua);
atomic_fetch_sub_explicit(&lazyfree_objects,len,memory_order_relaxed);
atomic_fetch_add_explicit(&lazyfreed_objects,len,memory_order_relaxed);
atomic_fetch_sub_explicit(&lazyfree_objects, len, memory_order_relaxed);
atomic_fetch_add_explicit(&lazyfreed_objects, len, memory_order_relaxed);
}

/* Release the functions ctx. */
void lazyFreeFunctionsCtx(void *args[]) {
functionsLibCtx *functions_lib_ctx = args[0];
size_t len = functionsLibCtxFunctionsLen(functions_lib_ctx);
functionsLibCtxFree(functions_lib_ctx);
atomic_fetch_sub_explicit(&lazyfree_objects,len,memory_order_relaxed);
atomic_fetch_add_explicit(&lazyfreed_objects,len,memory_order_relaxed);
atomic_fetch_sub_explicit(&lazyfree_objects, len, memory_order_relaxed);
atomic_fetch_add_explicit(&lazyfreed_objects, len, memory_order_relaxed);
}

/* Release replication backlog referencing memory. */
Expand All @@ -77,24 +77,24 @@ void lazyFreeReplicationBacklogRefMem(void *args[]) {
len += raxSize(index);
listRelease(blocks);
raxFree(index);
atomic_fetch_sub_explicit(&lazyfree_objects,len,memory_order_relaxed);
atomic_fetch_add_explicit(&lazyfreed_objects,len,memory_order_relaxed);
atomic_fetch_sub_explicit(&lazyfree_objects, len, memory_order_relaxed);
atomic_fetch_add_explicit(&lazyfreed_objects, len, memory_order_relaxed);
}

/* Return the number of currently pending objects to free. */
size_t lazyfreeGetPendingObjectsCount(void) {
size_t aux = atomic_load_explicit(&lazyfree_objects,memory_order_relaxed);
size_t aux = atomic_load_explicit(&lazyfree_objects, memory_order_relaxed);
return aux;
}

/* Return the number of objects that have been freed. */
size_t lazyfreeGetFreedObjectsCount(void) {
size_t aux = atomic_load_explicit(&lazyfreed_objects,memory_order_relaxed);
size_t aux = atomic_load_explicit(&lazyfreed_objects, memory_order_relaxed);
return aux;
}

void lazyfreeResetStats(void) {
atomic_store_explicit(&lazyfreed_objects,0,memory_order_relaxed);
atomic_store_explicit(&lazyfreed_objects, 0, memory_order_relaxed);
}

/* Return the amount of work needed in order to free an object.
Expand Down Expand Up @@ -174,8 +174,8 @@ void freeObjAsync(robj *key, robj *obj, int dbid) {
* of parts of the server core may call incrRefCount() to protect
* objects, and then call dbDelete(). */
if (free_effort > LAZYFREE_THRESHOLD && obj->refcount == 1) {
atomic_fetch_add_explicit(&lazyfree_objects,1,memory_order_relaxed);
bioCreateLazyFreeJob(lazyfreeFreeObject,1,obj);
atomic_fetch_add_explicit(&lazyfree_objects, 1, memory_order_relaxed);
bioCreateLazyFreeJob(lazyfreeFreeObject, 1, obj);
} else {
decrRefCount(obj);
}
Expand Down Expand Up @@ -203,8 +203,8 @@ void emptyDbAsync(serverDb *db) {
void freeTrackingRadixTreeAsync(rax *tracking) {
/* Because this rax has only keys and no values so we use numnodes. */
if (tracking->numnodes > LAZYFREE_THRESHOLD) {
atomic_fetch_add_explicit(&lazyfree_objects,tracking->numele,memory_order_relaxed);
bioCreateLazyFreeJob(lazyFreeTrackingTable,1,tracking);
atomic_fetch_add_explicit(&lazyfree_objects, tracking->numele, memory_order_relaxed);
bioCreateLazyFreeJob(lazyFreeTrackingTable, 1, tracking);
} else {
freeTrackingRadixTree(tracking);
}
Expand All @@ -215,8 +215,8 @@ void freeTrackingRadixTreeAsync(rax *tracking) {
void freeErrorsRadixTreeAsync(rax *errors) {
/* Because this rax has only keys and no values so we use numnodes. */
if (errors->numnodes > LAZYFREE_THRESHOLD) {
atomic_fetch_add_explicit(&lazyfree_objects,errors->numele,memory_order_relaxed);
bioCreateLazyFreeJob(lazyFreeErrors,1,errors);
atomic_fetch_add_explicit(&lazyfree_objects, errors->numele, memory_order_relaxed);
bioCreateLazyFreeJob(lazyFreeErrors, 1, errors);
} else {
raxFreeWithCallback(errors, zfree);
}
Expand All @@ -226,8 +226,8 @@ void freeErrorsRadixTreeAsync(rax *errors) {
* Close lua interpreter, if there are a lot of lua scripts, close it in async way. */
void freeLuaScriptsAsync(dict *lua_scripts, list *lua_scripts_lru_list, lua_State *lua) {
if (dictSize(lua_scripts) > LAZYFREE_THRESHOLD) {
atomic_fetch_add_explicit(&lazyfree_objects,dictSize(lua_scripts),memory_order_relaxed);
bioCreateLazyFreeJob(lazyFreeLuaScripts,3,lua_scripts,lua_scripts_lru_list,lua);
atomic_fetch_add_explicit(&lazyfree_objects, dictSize(lua_scripts), memory_order_relaxed);
bioCreateLazyFreeJob(lazyFreeLuaScripts, 3, lua_scripts, lua_scripts_lru_list, lua);
} else {
freeLuaScriptsSync(lua_scripts, lua_scripts_lru_list, lua);
}
Expand All @@ -236,20 +236,19 @@ void freeLuaScriptsAsync(dict *lua_scripts, list *lua_scripts_lru_list, lua_Stat
/* Free functions ctx, if the functions ctx contains enough functions, free it in async way. */
void freeFunctionsAsync(functionsLibCtx *functions_lib_ctx) {
if (functionsLibCtxFunctionsLen(functions_lib_ctx) > LAZYFREE_THRESHOLD) {
atomic_fetch_add_explicit(&lazyfree_objects,functionsLibCtxFunctionsLen(functions_lib_ctx),memory_order_relaxed);
bioCreateLazyFreeJob(lazyFreeFunctionsCtx,1,functions_lib_ctx);
atomic_fetch_add_explicit(&lazyfree_objects, functionsLibCtxFunctionsLen(functions_lib_ctx),
memory_order_relaxed);
bioCreateLazyFreeJob(lazyFreeFunctionsCtx, 1, functions_lib_ctx);
} else {
functionsLibCtxFree(functions_lib_ctx);
}
}

/* Free replication backlog referencing buffer blocks and rax index. */
void freeReplicationBacklogRefMemAsync(list *blocks, rax *index) {
if (listLength(blocks) > LAZYFREE_THRESHOLD ||
raxSize(index) > LAZYFREE_THRESHOLD)
{
atomic_fetch_add_explicit(&lazyfree_objects,listLength(blocks)+raxSize(index),memory_order_relaxed);
bioCreateLazyFreeJob(lazyFreeReplicationBacklogRefMem,2,blocks,index);
if (listLength(blocks) > LAZYFREE_THRESHOLD || raxSize(index) > LAZYFREE_THRESHOLD) {
atomic_fetch_add_explicit(&lazyfree_objects, listLength(blocks) + raxSize(index), memory_order_relaxed);
bioCreateLazyFreeJob(lazyFreeReplicationBacklogRefMem, 2, blocks, index);
} else {
listRelease(blocks);
raxFree(index);
Expand Down
14 changes: 7 additions & 7 deletions src/networking.c
Original file line number Diff line number Diff line change
Expand Up @@ -128,8 +128,8 @@ client *createClient(connection *conn) {
connSetPrivateData(conn, c);
}
c->buf = zmalloc_usable(PROTO_REPLY_CHUNK_BYTES, &c->buf_usable_size);
selectDb(c,0);
uint64_t client_id = atomic_fetch_add_explicit(&server.next_client_id,1,memory_order_relaxed);
selectDb(c, 0);
uint64_t client_id = atomic_fetch_add_explicit(&server.next_client_id, 1, memory_order_relaxed);
c->id = client_id;
#ifdef LOG_REQ_RES
reqresReset(c, 0);
Expand Down Expand Up @@ -1942,7 +1942,7 @@ int _writeToClient(client *c, ssize_t *nwritten) {
* thread safe. */
int writeToClient(client *c, int handler_installed) {
/* Update total number of writes on server */
atomic_fetch_add_explicit(&server.stat_total_writes_processed,1, memory_order_relaxed);
atomic_fetch_add_explicit(&server.stat_total_writes_processed, 1, memory_order_relaxed);

ssize_t nwritten = 0, totwritten = 0;

Expand Down Expand Up @@ -2610,7 +2610,7 @@ void readQueryFromClient(connection *conn) {
if (postponeClientRead(c)) return;

/* Update total number of reads on server */
atomic_fetch_add_explicit(&server.stat_total_reads_processed,1,memory_order_relaxed);
atomic_fetch_add_explicit(&server.stat_total_reads_processed, 1, memory_order_relaxed);

readlen = PROTO_IOBUF_LEN;
/* If this is a multi bulk request, and we are processing a bulk reply
Expand Down Expand Up @@ -2676,9 +2676,9 @@ void readQueryFromClient(connection *conn) {
c->lastinteraction = server.unixtime;
if (c->flags & CLIENT_MASTER) {
c->read_reploff += nread;
atomic_fetch_add_explicit(&server.stat_net_repl_input_bytes,nread,memory_order_relaxed);
atomic_fetch_add_explicit(&server.stat_net_repl_input_bytes, nread, memory_order_relaxed);
} else {
atomic_fetch_add_explicit(&server.stat_net_input_bytes,nread,memory_order_relaxed);
atomic_fetch_add_explicit(&server.stat_net_input_bytes, nread, memory_order_relaxed);
}
c->net_input_bytes += nread;

Expand All @@ -2697,7 +2697,7 @@ void readQueryFromClient(connection *conn) {
sdsfree(ci);
sdsfree(bytes);
freeClientAsync(c);
atomic_fetch_add_explicit(&server.stat_client_qbuf_limit_disconnections,1,memory_order_relaxed);
atomic_fetch_add_explicit(&server.stat_client_qbuf_limit_disconnections, 1, memory_order_relaxed);
goto done;
}

Expand Down
2 changes: 1 addition & 1 deletion src/rdb.c
Original file line number Diff line number Diff line change
Expand Up @@ -2889,7 +2889,7 @@ void rdbLoadProgressCallback(rio *r, const void *buf, size_t len) {
processModuleLoadingProgressEvent(0);
}
if (server.repl_state == REPL_STATE_TRANSFER && rioCheckType(r) == RIO_TYPE_CONN) {
atomic_fetch_add_explicit(&server.stat_net_repl_input_bytes,len,memory_order_relaxed);
atomic_fetch_add_explicit(&server.stat_net_repl_input_bytes, len, memory_order_relaxed);
}
}

Expand Down
14 changes: 7 additions & 7 deletions src/replication.c
Original file line number Diff line number Diff line change
Expand Up @@ -1364,8 +1364,8 @@ void sendBulkToSlave(connection *conn) {
freeClient(slave);
return;
}
atomic_fetch_add_explicit(&server.stat_net_repl_output_bytes,nwritten,memory_order_relaxed);
sdsrange(slave->replpreamble,nwritten,-1);
atomic_fetch_add_explicit(&server.stat_net_repl_output_bytes, nwritten, memory_order_relaxed);
sdsrange(slave->replpreamble, nwritten, -1);
if (sdslen(slave->replpreamble) == 0) {
sdsfree(slave->replpreamble);
slave->replpreamble = NULL;
Expand All @@ -1392,7 +1392,7 @@ void sendBulkToSlave(connection *conn) {
return;
}
slave->repldboff += nwritten;
atomic_fetch_add_explicit(&server.stat_net_repl_output_bytes,nwritten,memory_order_relaxed);
atomic_fetch_add_explicit(&server.stat_net_repl_output_bytes, nwritten, memory_order_relaxed);
if (slave->repldboff == slave->repldbsize) {
closeRepldbfd(slave);
connSetWriteHandler(slave->conn, NULL);
Expand Down Expand Up @@ -1434,7 +1434,7 @@ void rdbPipeWriteHandler(struct connection *conn) {
return;
} else {
slave->repldboff += nwritten;
atomic_fetch_add_explicit(&server.stat_net_repl_output_bytes,nwritten,memory_order_relaxed);
atomic_fetch_add_explicit(&server.stat_net_repl_output_bytes, nwritten, memory_order_relaxed);
if (slave->repldboff < server.rdb_pipe_bufflen) {
slave->repl_last_partial_write = server.unixtime;
return; /* more data to write.. */
Expand Down Expand Up @@ -1507,7 +1507,7 @@ void rdbPipeReadHandler(struct aeEventLoop *eventLoop, int fd, void *clientData,
/* Note: when use diskless replication, 'repldboff' is the offset
* of 'rdb_pipe_buff' sent rather than the offset of entire RDB. */
slave->repldboff = nwritten;
atomic_fetch_add_explicit(&server.stat_net_repl_output_bytes,nwritten,memory_order_relaxed);
atomic_fetch_add_explicit(&server.stat_net_repl_output_bytes, nwritten, memory_order_relaxed);
}
/* If we were unable to write all the data to one of the replicas,
* setup write handler (and disable pipe read handler, below) */
Expand Down Expand Up @@ -1815,7 +1815,7 @@ void readSyncBulkPayload(connection *conn) {
} else {
/* nread here is returned by connSyncReadLine(), which calls syncReadLine() and
* convert "\r\n" to '\0' so 1 byte is lost. */
atomic_fetch_add_explicit(&server.stat_net_repl_input_bytes,nread+1,memory_order_relaxed);
atomic_fetch_add_explicit(&server.stat_net_repl_input_bytes, nread + 1, memory_order_relaxed);
}

if (buf[0] == '-') {
Expand Down Expand Up @@ -1884,7 +1884,7 @@ void readSyncBulkPayload(connection *conn) {
cancelReplicationHandshake(1);
return;
}
atomic_fetch_add_explicit(&server.stat_net_repl_input_bytes,nread,memory_order_relaxed);
atomic_fetch_add_explicit(&server.stat_net_repl_input_bytes, nread, memory_order_relaxed);

/* When a mark is used, we want to detect EOF asap in order to avoid
* writing the EOF mark into the file... */
Expand Down
Loading

0 comments on commit 3399c93

Please sign in to comment.