From 077efa0cc707c59b4c0885b077ba61f186ad2b61 Mon Sep 17 00:00:00 2001 From: Willem Thiart Date: Thu, 17 May 2018 13:00:39 +1200 Subject: [PATCH] Remove user data passing. This is an experimental branch to investigate removing user data passing. The objective is to see how the API for version 1.0 could look like. This is the recommended method for passing user data: https://github.com/clibs/container_of --- include/raft.h | 41 ++++------------------------------------- include/raft_private.h | 3 +-- src/raft_log.c | 9 +++------ src/raft_node.c | 17 +---------------- src/raft_server.c | 33 ++++++++++++++++----------------- 5 files changed, 25 insertions(+), 78 deletions(-) diff --git a/include/raft.h b/include/raft.h index d45f0878..ff48afd4 100644 --- a/include/raft.h +++ b/include/raft.h @@ -196,7 +196,6 @@ typedef void* raft_node_t; /** Callback for sending request vote messages. * @param[in] raft The Raft server making this callback - * @param[in] user_data User data that is passed from Raft server * @param[in] node The node's ID that we are sending this message to * @param[in] msg The request vote message to be sent * @return 0 on success */ @@ -204,14 +203,12 @@ typedef int ( *func_send_requestvote_f ) ( raft_server_t* raft, - void *user_data, raft_node_t* node, msg_requestvote_t* msg ); /** Callback for sending append entries messages. * @param[in] raft The Raft server making this callback - * @param[in] user_data User data that is passed from Raft server * @param[in] node The node's ID that we are sending this message to * @param[in] msg The appendentries message to be sent * @return 0 on success */ @@ -219,7 +216,6 @@ typedef int ( *func_send_appendentries_f ) ( raft_server_t* raft, - void *user_data, raft_node_t* node, msg_appendentries_t* msg ); @@ -229,28 +225,24 @@ typedef int ( * Callback for telling the user to send a snapshot. * * @param[in] raft Raft server making this callback - * @param[in] user_data User data that is passed from Raft server * @param[in] node Node's ID that needs a snapshot sent to **/ typedef int ( *func_send_snapshot_f ) ( raft_server_t* raft, - void *user_data, raft_node_t* node ); /** Callback for detecting when non-voting nodes have obtained enough logs. * This triggers only when there are no pending configuration changes. * @param[in] raft The Raft server making this callback - * @param[in] user_data User data that is passed from Raft server * @param[in] node The node * @return 0 does not want to be notified again; otherwise -1 */ typedef int ( *func_node_has_sufficient_logs_f ) ( raft_server_t* raft, - void *user_data, raft_node_t* node ); @@ -260,14 +252,12 @@ typedef int ( * This callback is optional * @param[in] raft The Raft server making this callback * @param[in] node The node that is the subject of this log. Could be NULL. - * @param[in] user_data User data that is passed from Raft server * @param[in] buf The buffer that was logged */ typedef void ( *func_log_f ) ( raft_server_t* raft, raft_node_t* node, - void *user_data, const char *buf ); #endif @@ -275,14 +265,12 @@ typedef void ( /** Callback for saving who we voted for to disk. * For safety reasons this callback MUST flush the change to disk. * @param[in] raft The Raft server making this callback - * @param[in] user_data User data that is passed from Raft server * @param[in] vote The node we voted for * @return 0 on success */ typedef int ( *func_persist_vote_f ) ( raft_server_t* raft, - void *user_data, int vote ); @@ -290,7 +278,6 @@ typedef int ( * For safety reasons this callback MUST flush the term and vote changes to * disk atomically. * @param[in] raft The Raft server making this callback - * @param[in] user_data User data that is passed from Raft server * @param[in] term Current term * @param[in] vote The node value dicating we haven't voted for anybody * @return 0 on success */ @@ -298,7 +285,6 @@ typedef int ( *func_persist_term_f ) ( raft_server_t* raft, - void *user_data, int term, int vote ); @@ -316,7 +302,6 @@ typedef int ( * For safety reasons this callback MUST flush the change to disk. * * @param[in] raft The Raft server making this callback - * @param[in] user_data User data that is passed from Raft server * @param[in] entry The entry that the event is happening to. * For offering, polling, and popping, the user is allowed to change the * memory pointed to in the raft_entry_data_t struct. This MUST be done if @@ -327,7 +312,6 @@ typedef int ( *func_logentry_event_f ) ( raft_server_t* raft, - void *user_data, raft_entry_t *entry, int entry_idx ); @@ -415,8 +399,8 @@ void raft_clear(raft_server_t* me); /** Set callbacks and user data. * * @param[in] funcs Callbacks - * @param[in] user_data "User data" - user's context that's included in a callback */ -void raft_set_callbacks(raft_server_t* me, raft_cbs_t* funcs, void* user_data); + */ +void raft_set_callbacks(raft_server_t* me, raft_cbs_t* funcs); /** Add node. * @@ -426,18 +410,13 @@ void raft_set_callbacks(raft_server_t* me, raft_cbs_t* funcs, void* user_data); * This call MUST be made in the same order as the other raft nodes. * This is because the node ID is assigned depending on when this call is made * - * @param[in] user_data The user data for the node. - * This is obtained using raft_node_get_udata. - * Examples of what this could be: - * - void* pointing to implementor's networking data - * - a (IP,Port) tuple * @param[in] id The integer ID of this node * This is used for identifying clients across sessions. * @param[in] is_self Set to 1 if this "node" is this server * @return * node if it was successfully added; * NULL if the node already exists */ -raft_node_t* raft_add_node(raft_server_t* me, void* user_data, int id, int is_self); +raft_node_t* raft_add_node(raft_server_t* me, int id, int is_self); #define raft_add_peer raft_add_node @@ -447,7 +426,7 @@ raft_node_t* raft_add_node(raft_server_t* me, void* user_data, int id, int is_se * @return * node if it was successfully added; * NULL if the node already exists */ -raft_node_t* raft_add_non_voting_node(raft_server_t* me_, void* udata, int id, int is_self); +raft_node_t* raft_add_non_voting_node(raft_server_t* me_, int id, int is_self); /** Remove node. * @param node The node to be removed. */ @@ -629,14 +608,6 @@ int raft_node_get_next_idx(raft_node_t* node); * @return this node's user data */ int raft_node_get_match_idx(raft_node_t* me); -/** - * @return this node's user data */ -void* raft_node_get_udata(raft_node_t* me); - -/** - * Set this node's user data */ -void raft_node_set_udata(raft_node_t* me, void* user_data); - /** * @param[in] idx The entry's index * @return entry from index */ @@ -671,10 +642,6 @@ int raft_get_current_leader(raft_server_t* me); * NULL if the leader is unknown */ raft_node_t* raft_get_current_leader_node(raft_server_t* me); -/** - * @return callback user data */ -void* raft_get_udata(raft_server_t* me); - /** Vote for a server. * This should be used to reload persistent state, ie. the voted-for field. * @param[in] node The server to vote for diff --git a/include/raft_private.h b/include/raft_private.h index 9179120b..7b911f98 100644 --- a/include/raft_private.h +++ b/include/raft_private.h @@ -58,7 +58,6 @@ typedef struct { /* callbacks */ raft_cbs_t cb; - void* udata; /* my node ID */ raft_node_t* node; @@ -108,7 +107,7 @@ void raft_set_state(raft_server_t* me_, int state); int raft_get_state(raft_server_t* me_); -raft_node_t* raft_node_new(void* udata, int id); +raft_node_t* raft_node_new(int id); void raft_node_free(raft_node_t* me_); diff --git a/src/raft_log.c b/src/raft_log.c index 7e74fbd4..a3257d24 100644 --- a/src/raft_log.c +++ b/src/raft_log.c @@ -151,8 +151,7 @@ int log_append_entry(log_t* me_, raft_entry_t* ety) if (me->cb && me->cb->log_offer) { - void* ud = raft_get_udata(me->raft); - e = me->cb->log_offer(me->raft, ud, &me->entries[me->back], idx); + e = me->cb->log_offer(me->raft, &me->entries[me->back], idx); if (0 != e) return e; raft_offer_log(me->raft, &me->entries[me->back], idx); @@ -237,8 +236,7 @@ int log_delete(log_t* me_, int idx) if (me->cb && me->cb->log_pop) { - int e = me->cb->log_pop(me->raft, raft_get_udata(me->raft), - &me->entries[back], idx_tmp); + int e = me->cb->log_pop(me->raft, &me->entries[back], idx_tmp); if (0 != e) return e; } @@ -260,8 +258,7 @@ int log_poll(log_t * me_, void** etyp) const void *elem = &me->entries[me->front]; if (me->cb && me->cb->log_poll) { - int e = me->cb->log_poll(me->raft, raft_get_udata(me->raft), - &me->entries[me->front], idx); + int e = me->cb->log_poll(me->raft, &me->entries[me->front], idx); if (0 != e) return e; } diff --git a/src/raft_node.c b/src/raft_node.c index 17ebc35f..19c2305b 100644 --- a/src/raft_node.c +++ b/src/raft_node.c @@ -26,8 +26,6 @@ typedef struct { - void* udata; - int next_idx; int match_idx; @@ -36,13 +34,12 @@ typedef struct int id; } raft_node_private_t; -raft_node_t* raft_node_new(void* udata, int id) +raft_node_t* raft_node_new(int id) { raft_node_private_t* me; me = (raft_node_private_t*)__raft_calloc(1, sizeof(raft_node_private_t)); if (!me) return NULL; - me->udata = udata; me->next_idx = 1; me->match_idx = 0; me->id = id; @@ -80,18 +77,6 @@ void raft_node_set_match_idx(raft_node_t* me_, int matchIdx) me->match_idx = matchIdx; } -void* raft_node_get_udata(raft_node_t* me_) -{ - raft_node_private_t* me = (raft_node_private_t*)me_; - return me->udata; -} - -void raft_node_set_udata(raft_node_t* me_, void* udata) -{ - raft_node_private_t* me = (raft_node_private_t*)me_; - me->udata = udata; -} - void raft_node_vote_for_me(raft_node_t* me_, const int vote) { raft_node_private_t* me = (raft_node_private_t*)me_; diff --git a/src/raft_server.c b/src/raft_server.c index fdfb6298..af8c352a 100644 --- a/src/raft_server.c +++ b/src/raft_server.c @@ -54,7 +54,7 @@ static void __log(raft_server_t *me_, raft_node_t* node, const char *fmt, ...) va_start(args, fmt); vsprintf(buf, fmt, args); - me->cb.log(me_, node, me->udata, buf); + me->cb.log(me_, node, buf); } void raft_randomize_election_timeout(raft_server_t* me_) @@ -93,12 +93,11 @@ raft_server_t* raft_new() return (raft_server_t*)me; } -void raft_set_callbacks(raft_server_t* me_, raft_cbs_t* funcs, void* udata) +void raft_set_callbacks(raft_server_t* me_, raft_cbs_t* funcs) { raft_server_private_t* me = (raft_server_private_t*)me_; memcpy(&me->cb, funcs, sizeof(raft_cbs_t)); - me->udata = udata; log_set_callbacks(me->log, &me->cb, me_); } @@ -341,7 +340,7 @@ int raft_recv_appendentries_response(raft_server_t* me_, 0 == raft_node_has_sufficient_logs(node) ) { - int e = me->cb.node_has_sufficient_logs(me_, me->udata, node); + int e = me->cb.node_has_sufficient_logs(me_, node); if (0 == e) raft_node_set_has_sufficient_logs(node); } @@ -768,7 +767,7 @@ int raft_send_requestvote(raft_server_t* me_, raft_node_t* node) rv.last_log_term = raft_get_last_log_term(me_); rv.candidate_id = raft_get_nodeid(me_); if (me->cb.send_requestvote) - e = me->cb.send_requestvote(me_, me->udata, node, &rv); + e = me->cb.send_requestvote(me_, node, &rv); return e; } @@ -805,7 +804,7 @@ int raft_apply_entry(raft_server_t* me_) me->last_applied_idx++; if (me->cb.applylog) { - int e = me->cb.applylog(me_, me->udata, ety, me->last_applied_idx - 1); + int e = me->cb.applylog(me_, ety, me->last_applied_idx - 1); if (RAFT_ERR_SHUTDOWN == e) return RAFT_ERR_SHUTDOWN; } @@ -817,7 +816,7 @@ int raft_apply_entry(raft_server_t* me_) if (!raft_entry_is_cfg_change(ety)) return 0; - int node_id = me->cb.log_get_node_id(me_, raft_get_udata(me_), ety, log_idx); + int node_id = me->cb.log_get_node_id(me_, ety, log_idx); raft_node_t* node = raft_get_node(me_, node_id); switch (ety->type) { @@ -875,7 +874,7 @@ int raft_send_appendentries(raft_server_t* me_, raft_node_t* node) if (0 < me->snapshot_last_idx && next_idx < me->snapshot_last_idx) { if (me->cb.send_snapshot) - me->cb.send_snapshot(me_, me->udata, node); + me->cb.send_snapshot(me_, node); return RAFT_ERR_NEEDS_SNAPSHOT; } @@ -902,7 +901,7 @@ int raft_send_appendentries(raft_server_t* me_, raft_node_t* node) ae.prev_log_idx, ae.prev_log_term); - return me->cb.send_appendentries(me_, me->udata, node, &ae); + return me->cb.send_appendentries(me_, node, &ae); } int raft_send_appendentries_all(raft_server_t* me_) @@ -924,7 +923,7 @@ int raft_send_appendentries_all(raft_server_t* me_) return 0; } -raft_node_t* raft_add_node(raft_server_t* me_, void* udata, int id, int is_self) +raft_node_t* raft_add_node(raft_server_t* me_, int id, int is_self) { raft_server_private_t* me = (raft_server_private_t*)me_; @@ -942,7 +941,7 @@ raft_node_t* raft_add_node(raft_server_t* me_, void* udata, int id, int is_self) return NULL; } - node = raft_node_new(udata, id); + node = raft_node_new(id); if (!node) return NULL; void* p = __raft_realloc(me->nodes, sizeof(void*) * (me->num_nodes + 1)); @@ -959,12 +958,12 @@ raft_node_t* raft_add_node(raft_server_t* me_, void* udata, int id, int is_self) return me->nodes[me->num_nodes - 1]; } -raft_node_t* raft_add_non_voting_node(raft_server_t* me_, void* udata, int id, int is_self) +raft_node_t* raft_add_non_voting_node(raft_server_t* me_, int id, int is_self) { if (raft_get_node(me_, id)) return NULL; - raft_node_t* node = raft_add_node(me_, udata, id, is_self); + raft_node_t* node = raft_add_node(me_, id, is_self); if (!node) return NULL; @@ -1026,7 +1025,7 @@ int raft_vote_for_nodeid(raft_server_t* me_, const int nodeid) raft_server_private_t* me = (raft_server_private_t*)me_; if (me->cb.persist_vote) { - int e = me->cb.persist_vote(me_, me->udata, nodeid); + int e = me->cb.persist_vote(me_, nodeid); if (0 != e) return e; } @@ -1084,7 +1083,7 @@ void raft_offer_log(raft_server_t* me_, raft_entry_t* ety, const int idx) if (!raft_entry_is_cfg_change(ety)) return; - int node_id = me->cb.log_get_node_id(me_, raft_get_udata(me_), ety, idx); + int node_id = me->cb.log_get_node_id(me_, ety, idx); raft_node_t* node = raft_get_node(me_, node_id); int is_self = node_id == raft_get_nodeid(me_); @@ -1133,7 +1132,7 @@ void raft_pop_log(raft_server_t* me_, raft_entry_t* ety, const int idx) if (!raft_entry_is_cfg_change(ety)) return; - int node_id = me->cb.log_get_node_id(me_, raft_get_udata(me_), ety, idx); + int node_id = me->cb.log_get_node_id(me_, ety, idx); switch (ety->type) { @@ -1275,7 +1274,7 @@ int raft_end_snapshot(raft_server_t *me_) if (0 < me->snapshot_last_idx && next_idx < me->snapshot_last_idx) { if (me->cb.send_snapshot) - me->cb.send_snapshot(me_, me->udata, node); + me->cb.send_snapshot(me_, node); } }