log_get_from_idx not including entries that have wrapped around?

[freeekanayaka]

Suppose that the circular log buffer is of size 3 and has the following layout:

[<entry>, NULL, <entry>]
    ^              ^
   back          front

base index is 3 (so <entry> at front has index 3 and <entry> at back has index 4)

then it seems that a call to log_get_from_idx(3) instead of returning an array of 2 entries which includes entries 3 and 4, returns an array of just 1 entry which includes only entry 3.

This can be reproduced with the following (failing) test:

void TestLog_get_from_idx_with_wrapping(CuTest * tc)
{
    void* queue = llqueue_new();
    void *r = raft_new();
    raft_cbs_t funcs = {
        .log_pop = __log_pop,
        .log_get_node_id = __logentry_get_node_id
    };
    raft_set_callbacks(r, &funcs, queue);

    void *l;
    raft_entry_t e1, e2, e3, e4;

    memset(&e1, 0, sizeof(raft_entry_t));
    memset(&e2, 0, sizeof(raft_entry_t));
    memset(&e3, 0, sizeof(raft_entry_t));
    memset(&e4, 0, sizeof(raft_entry_t));

    e1.id = 1;
    e2.id = 2;
    e3.id = 3;
    e4.id = 4;

    l = log_alloc(3);
    log_set_callbacks(l, &funcs, r);

    raft_entry_t* ety;

    /* append append append */
    CuAssertIntEquals(tc, 0, log_append_entry(l, &e1));
    CuAssertIntEquals(tc, 0, log_append_entry(l, &e2));
    CuAssertIntEquals(tc, 0, log_append_entry(l, &e3));
    CuAssertIntEquals(tc, 3, log_count(l));

    /* poll poll */
    CuAssertIntEquals(tc, log_poll(l, (void*)&ety), 0);
    CuAssertIntEquals(tc, ety->id, 1);
    CuAssertIntEquals(tc, log_poll(l, (void*)&ety), 0);
    CuAssertIntEquals(tc, ety->id, 2);
    CuAssertIntEquals(tc, 1, log_count(l));

    /* append */
    CuAssertIntEquals(tc, 0, log_append_entry(l, &e4));
    CuAssertIntEquals(tc, 2, log_count(l));

    /* get from index 3 */
    int n_etys;
    raft_entry_t* etys;
    etys = log_get_from_idx(l, 3, &n_etys);

    CuAssertPtrNotNull(tc, etys);
    CuAssertIntEquals(tc, n_etys, 2);

    CuAssertIntEquals(tc, etys[0].id, 3);
    CuAssertIntEquals(tc, etys[1].id, 4);
}

It seems the bug is log_get_from_idx() when it calculates the length of the array to be returned:

    if (i < me->back)
        logs_till_end_of_log = me->back - i;
    else
        logs_till_end_of_log = me->size - i;

In the else case the length should actually be me->size - i + me->back, to include the most recent entries that have wrapped around. However it's not clear how to return a pointer without performing some allocation, because of course pointers don't wrap around.

Does it make sense? Is this a bug?

[skypexu]

Not only this question, my questions is: can log entry only be in memory? how about big log entry, such as 2G bytes, and you have mutliple log entries in memory ?
I suppose log interface should be virtual, and it does care where you put the log entry, may be in memory or on disk.

[freeekanayaka]

I personally thinking that the approach of keeping the whole log in memory is fine. Do you have a real world use case of entries sizing 2Gb each? It seems you're doing something really wrong in that case, as there are probably more effective alternatives (e.g. using raft as control layer for replication metadata, but replicate the data out of band).