Skip to content

Commit 050268f

Browse files
committed
Remove malloc from entry appending
Added documentation to mention that user is responsible for managing memory.
1 parent 3b91d4b commit 050268f

2 files changed

Lines changed: 52 additions & 28 deletions

File tree

include/raft.h

Lines changed: 43 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,7 @@ typedef int (
206206
);
207207

208208
/** Callback for saving who we voted for to disk.
209-
* This callback MUST flush the change to disk.
209+
* For safety reasons this callback MUST flush the change to disk.
210210
* @param[in] raft The Raft server making this callback
211211
* @param[in] user_data User data that is passed from Raft server
212212
* @param[in] voted_for The node we voted for
@@ -220,16 +220,21 @@ typedef int (
220220
);
221221

222222
/** Callback for saving log entry changes.
223+
*
223224
* This callback is used for:
224225
* <ul>
225226
* <li>Adding entries to the log (ie. offer)
226227
* <li>Removing the first entry from the log (ie. polling)
227228
* <li>Removing the last entry from the log (ie. popping)
228229
* </ul>
229-
* This callback MUST flush the change to disk.
230+
*
231+
* For safety reasons this callback MUST flush the change to disk.
232+
*
230233
* @param[in] raft The Raft server making this callback
231234
* @param[in] user_data User data that is passed from Raft server
232-
* @param[in] entry The entry that the event is happening to
235+
* @param[in] entry The entry that the event is happening to.
236+
* The user is allowed to change the memory pointed to in the
237+
* raft_entry_data_t struct. This MUST be done if the memory is temporary.
233238
* @param[in] entry_idx The entries index in the log
234239
* @return 0 on success */
235240
typedef int (
@@ -253,23 +258,27 @@ typedef struct
253258
func_applylog_f applylog;
254259

255260
/** Callback for persisting vote data
256-
* This callback MUST flush the change to disk. */
261+
* For safety reasons this callback MUST flush the change to disk. */
257262
func_persist_int_f persist_vote;
258263

259264
/** Callback for persisting term data
260-
* This callback MUST flush the change to disk. */
265+
* For safety reasons this callback MUST flush the change to disk. */
261266
func_persist_int_f persist_term;
262267

263268
/** Callback for adding an entry to the log
264-
* This callback MUST flush the change to disk. */
269+
* For safety reasons this callback MUST flush the change to disk. */
265270
func_logentry_event_f log_offer;
266271

267272
/** Callback for removing the oldest entry from the log
268-
* This callback MUST flush the change to disk. */
273+
* For safety reasons this callback MUST flush the change to disk.
274+
* @note If memory was malloc'd in log_offer then this should be the right
275+
* time to free the memory. */
269276
func_logentry_event_f log_poll;
270277

271278
/** Callback for removing the youngest entry from the log
272-
* This callback MUST flush the change to disk. */
279+
* For safety reasons this callback MUST flush the change to disk.
280+
* @note If memory was malloc'd in log_offer then this should be the right
281+
* time to free the memory. */
273282
func_logentry_event_f log_pop;
274283

275284
/** Callback for catching debugging log messages
@@ -318,11 +327,11 @@ __attribute__ ((deprecated));
318327
/** Add node.
319328
*
320329
* @note This library does not yet support membership changes.
321-
* Once raft_periodic has been run this will fail.
330+
* Once raft_periodic has been run this will fail.
322331
*
323332
* @note The order this call is made is important.
324-
* This call MUST be made in the same order as the other raft nodes.
325-
* This is because the node ID is assigned depending on when this call is made
333+
* This call MUST be made in the same order as the other raft nodes.
334+
* This is because the node ID is assigned depending on when this call is made
326335
*
327336
* @param[in] user_data The user data for the node.
328337
* This is obtained using raft_node_get_udata.
@@ -351,7 +360,18 @@ void raft_set_request_timeout(raft_server_t* me, int msec);
351360
int raft_periodic(raft_server_t* me, int msec_elapsed);
352361

353362
/** Receive an appendentries message.
354-
* This function will block if it needs to append the message.
363+
*
364+
* Will block (ie. by syncing to disk) if we need to append a message.
365+
*
366+
* Might call malloc once to increase the log entry array size.
367+
*
368+
* The log_offer callback will be called.
369+
*
370+
* @note The memory pointer (ie. raft_entry_data_t) for each msg_entry_t is
371+
* copied directly. If the memory is temporary you MUST either make the
372+
* memory permanent (ie. via malloc) OR re-assign the memory within the
373+
* log_offer callback.
374+
*
355375
* @param[in] node Index of the node who sent us this message
356376
* @param[in] ae The appendentries message
357377
* @param[out] r The resulting response
@@ -387,11 +407,20 @@ int raft_recv_requestvote_response(raft_server_t* me,
387407
int node,
388408
msg_requestvote_response_t* r);
389409

390-
/** Receive an entry message from client.
410+
/** Receive an entry message from the client.
391411
*
392412
* Append the entry to the log and send appendentries to followers.
393413
*
394-
* This function will block if it needs to append the message.
414+
* Will block (ie. by syncing to disk) if we need to append a message.
415+
*
416+
* Might call malloc once to increase the log entry array size.
417+
*
418+
* The log_offer callback will be called.
419+
*
420+
* @note The memory pointer (ie. raft_entry_data_t) in msg_entry_t is
421+
* copied directly. If the memory is temporary you MUST either make the
422+
* memory permanent (ie. via malloc) OR re-assign the memory within the
423+
* log_offer callback.
395424
*
396425
* Will fail:
397426
* <ul>

src/raft_server.c

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -314,14 +314,11 @@ int raft_recv_appendentries(
314314
{
315315
msg_entry_t* cmd = &ae->entries[i];
316316

317-
/* TODO: replace malloc with mempoll/arena */
318-
raft_entry_t* c = (raft_entry_t*)malloc(sizeof(raft_entry_t));
319-
c->term = cmd->term;
320-
memcpy(&c->data, &cmd->data, sizeof(raft_entry_data_t));
321-
c->data.buf = (unsigned char*)malloc(cmd->data.len);
322-
memcpy(c->data.buf, cmd->data.buf, cmd->data.len);
323-
c->id = cmd->id;
324-
int e = raft_append_entry(me_, c);
317+
raft_entry_t ety;
318+
ety.term = cmd->term;
319+
ety.id = cmd->id;
320+
memcpy(&ety.data, &cmd->data, sizeof(raft_entry_data_t));
321+
int e = raft_append_entry(me_, &ety);
325322
if (-1 == e)
326323
{
327324
__log(me_, "AE failure; couldn't append entry");
@@ -423,19 +420,17 @@ int raft_recv_entry(raft_server_t* me_, int node, msg_entry_t* e,
423420
msg_entry_response_t *r)
424421
{
425422
raft_server_private_t* me = (raft_server_private_t*)me_;
426-
raft_entry_t ety;
427423
int i;
428424

429425
if (!raft_is_leader(me_))
430426
return -1;
431427

432428
__log(me_, "received entry from: %d", node);
433429

430+
raft_entry_t ety;
434431
ety.term = me->current_term;
435432
ety.id = e->id;
436-
ety.data.len = e->data.len;
437-
ety.data.buf = malloc(e->data.len);
438-
memcpy(ety.data.buf, e->data.buf, e->data.len);
433+
memcpy(&ety.data, &e->data, sizeof(raft_entry_data_t));
439434
raft_append_entry(me_, &ety);
440435
for (i = 0; i < me->num_nodes; i++)
441436
if (me->nodeid != i)
@@ -463,10 +458,10 @@ int raft_send_requestvote(raft_server_t* me_, int node)
463458
return 0;
464459
}
465460

466-
int raft_append_entry(raft_server_t* me_, raft_entry_t* c)
461+
int raft_append_entry(raft_server_t* me_, raft_entry_t* ety)
467462
{
468463
raft_server_private_t* me = (raft_server_private_t*)me_;
469-
return log_append_entry(me->log, c);
464+
return log_append_entry(me->log, ety);
470465
}
471466

472467
int raft_apply_entry(raft_server_t* me_)

0 commit comments

Comments
 (0)