fix(flatfile): drop cursor optim + guard n_entries==0
Some checks failed
CI Code / Check spelling (pull_request) Failing after 22s
CI Code / Check coding style (pull_request) Successful in 34s
CI Code / Linux (debian) (pull_request) Successful in 5m23s
CI Code / Linux (arch) (pull_request) Successful in 6m1s
CI Code / Linux (ubuntu) (pull_request) Successful in 7m18s
CI Code / Code Coverage (pull_request) Successful in 7m32s
Some checks failed
CI Code / Check spelling (pull_request) Failing after 22s
CI Code / Check coding style (pull_request) Successful in 34s
CI Code / Linux (debian) (pull_request) Successful in 5m23s
CI Code / Linux (arch) (pull_request) Successful in 6m1s
CI Code / Linux (ubuntu) (pull_request) Successful in 7m18s
CI Code / Code Coverage (pull_request) Successful in 7m32s
Cursor was set to oldest of read window, not oldest of returned batch — each page-up jumped past entire ranges (F16: 600/2000 msgs). Always bisect now; correct + simpler. Cost: +5-10 ms per page-up at 1M msgs, well below UI threshold. Also guard state->entries NULL when total_lines>0 && n_entries==0 (every line failed _ff_maybe_index_line). Backup would segfault. F16 tightened to received == total_msgs. Catches both stuck-on- entries[0] and cursor-jumping regressions.
This commit is contained in:
@@ -719,6 +719,12 @@ _flatfile_get_previous_chat(const gchar* const contact_barejid, const gchar* sta
|
|||||||
return DB_RESPONSE_EMPTY;
|
return DB_RESPONSE_EMPTY;
|
||||||
if (state->total_lines == 0)
|
if (state->total_lines == 0)
|
||||||
return DB_RESPONSE_EMPTY;
|
return DB_RESPONSE_EMPTY;
|
||||||
|
// n_entries can be 0 even when total_lines > 0 if every line in the
|
||||||
|
// file failed _ff_maybe_index_line (e.g. unparseable timestamps).
|
||||||
|
// Subsequent backup logic dereferences entries[0] unconditionally,
|
||||||
|
// which would segfault on a NULL entries array.
|
||||||
|
if (state->n_entries == 0)
|
||||||
|
return DB_RESPONSE_EMPTY;
|
||||||
|
|
||||||
// Reset cursor for filtered (non-Page-Up) queries
|
// Reset cursor for filtered (non-Page-Up) queries
|
||||||
if (start_time)
|
if (start_time)
|
||||||
@@ -728,16 +734,17 @@ _flatfile_get_previous_chat(const gchar* const contact_barejid, const gchar* sta
|
|||||||
off_t read_from = state->bom_len;
|
off_t read_from = state->bom_len;
|
||||||
off_t read_to = state->stamp.size;
|
off_t read_to = state->stamp.size;
|
||||||
|
|
||||||
// Use stored cursor for sequential Page Up (skip bisect).
|
// Upper-bound via bisect on end_time. The previous cursor-based
|
||||||
// Guard: when cursor lands on entries[0].byte_offset, the !from_start
|
// shortcut (use last batch's oldest file_offset as read_to) was
|
||||||
// backup below produces an empty [X, X) range; falling back to the
|
// both buggy and unnecessary: cursor was set to the oldest line in
|
||||||
// end_time bisect avoids permanently sticking page-up at the top.
|
// the read window, but pagination dropped the older portion of that
|
||||||
if (!start_time && end_time && state->cursor_offset >= 0
|
// window before returning to the caller. The next page-up therefore
|
||||||
&& state->n_entries > 0
|
// started reading from a point much earlier than what the user had
|
||||||
&& state->cursor_offset != state->entries[0].byte_offset) {
|
// actually seen, "jumping" past entire ranges of messages. Bisect
|
||||||
read_to = state->cursor_offset;
|
// on the index is O(log n_entries) — for typical 200-entry indexes
|
||||||
} else if (end_time) {
|
// it's a handful of comparisons, well below the per-line parse
|
||||||
// Upper-bound: find first index entry AFTER end_time
|
// cost — so we now run it unconditionally.
|
||||||
|
if (end_time) {
|
||||||
GDateTime* edt = g_date_time_new_from_iso8601(end_time, NULL);
|
GDateTime* edt = g_date_time_new_from_iso8601(end_time, NULL);
|
||||||
if (edt) {
|
if (edt) {
|
||||||
gint64 end_epoch = g_date_time_to_unix(edt);
|
gint64 end_epoch = g_date_time_to_unix(edt);
|
||||||
@@ -750,8 +757,8 @@ _flatfile_get_previous_chat(const gchar* const contact_barejid, const gchar* sta
|
|||||||
else
|
else
|
||||||
hi = mid;
|
hi = mid;
|
||||||
}
|
}
|
||||||
// lo = first entry with timestamp > end_epoch
|
// lo = first entry with timestamp > end_epoch.
|
||||||
// Use next entry's offset as read_to (+ 1 entry margin)
|
// Use next entry's offset as read_to (+ 1 entry margin).
|
||||||
if (lo + 1 < state->n_entries)
|
if (lo + 1 < state->n_entries)
|
||||||
read_to = state->entries[lo + 1].byte_offset;
|
read_to = state->entries[lo + 1].byte_offset;
|
||||||
// else read_to stays at EOF
|
// else read_to stays at EOF
|
||||||
|
|||||||
@@ -672,21 +672,21 @@ test_F16(fail_ctx_t* ctx)
|
|||||||
|
|
||||||
double dt = bench_now_ms() - t0;
|
double dt = bench_now_ms() - t0;
|
||||||
|
|
||||||
// Pass criterion focuses on the reviewer-reported symptom: without the
|
// Two regression classes are caught here:
|
||||||
// guard, the cursor lands on entries[0] within the first few iterations
|
// 1. The original "cursor stuck on entries[0]" symptom (early EMPTY
|
||||||
// (file has 4 index entries, after ~2 page-ups the cursor drops to msg
|
// after 2-3 iterations, ~200 received) — capped page-up at the top.
|
||||||
// 0 due to the read-window-vs-returned-batch divergence), and the next
|
// 2. The deeper "cursor jumping" bug — cursor was set to the oldest
|
||||||
// call returns DB_RESPONSE_EMPTY immediately, capping total_received at
|
// line of the read window rather than the oldest line returned to
|
||||||
// ~200 messages. With the guard the bisect fallback engages and pulls
|
// the caller after pagination, so each subsequent page-up skipped
|
||||||
// additional batches before genuinely reaching the top, lifting total
|
// past entire ranges of messages. With both fixed, every written
|
||||||
// well above the bug-induced ceiling.
|
// message must be reachable through sequential page-up.
|
||||||
gboolean ok = (iterations >= 5)
|
gboolean ok = (total_received == total_msgs)
|
||||||
&& (total_received >= 500)
|
|
||||||
&& (empty_responses == 1)
|
&& (empty_responses == 1)
|
||||||
&& (iterations < 200);
|
&& (iterations < 200);
|
||||||
|
|
||||||
auto_gchar gchar* note = g_strdup_printf(
|
auto_gchar gchar* note = g_strdup_printf(
|
||||||
"wrote=%d received=%d iters=%d empty=%d (without guard: <=200 received in <=3 iters)",
|
"wrote=%d received=%d iters=%d empty=%d "
|
||||||
|
"(without fix: ~200 received in ~3 iters; cursor-jumping mode: ~600 in ~7)",
|
||||||
total_msgs, total_received, iterations, empty_responses);
|
total_msgs, total_received, iterations, empty_responses);
|
||||||
report(ctx, "F16", ok, dt, path, note);
|
report(ctx, "F16", ok, dt, path, note);
|
||||||
be->close();
|
be->close();
|
||||||
|
|||||||
Reference in New Issue
Block a user