Bug 64616

Right, it turned out that some of the files in /var/log/journal.cursor-bug are corrupted.  Some entry objects in them are zeroed-out.  I am quite brutal with this machine (a VM), so this is not that much of a surprise.

Unfortunately, the journal is not just shorter than expected, it really is corrupted and becomes inconsistent as shown in the bug description.

_Fortunately_, binary search doesn't really seem to be necessary for finding entries by seqnum.  A entry with seqnum S should always be at index S-1 in the entry array of its file, no?

This patch works for my scenario:

--- a/src/journal/journal-file.c
+++ b/src/journal/journal-file.c
@@ -1767,6 +1767,21 @@ int journal_file_move_to_entry_by_seqnum(
                 Object **ret,
                 uint64_t *offset) {
 
+        int r;
+
+        if (seqnum >= 1 && seqnum <= f->header->n_entries) {
+                Object *o;
+                r = generic_array_get(f,
+                                      le64toh(f->header->entry_array_offset),
+                                      seqnum-1,
+                                      &o, offset);
+                if (r >= 0 && le64toh(o->entry.seqnum) == seqnum) {
+                        if (ret)
+                                *ret = o;
+                        return 1;
+                }
+        }
+
         return generic_array_bisect(f,
                                     le64toh(f->header->entry_array_offset),
                                     le64toh(f->header->n_entries),

(It is quite conservative, and falls back to the existing code if my assumption is wrong.)

I haven't yet looked at journal_file_move_to_entry_by_seqnum_for_data.  Something similar will probably be appropriate there as well.

Even without considering corruption, using generic_array_get with seqnums should be a small performance improvement, no?


In general, we might try to detect corruption during a binary search, issue a warning, and try to repair the damage.  For example, we might do a linear search for a non-corrupted entry and continue the bisecting from there.

I'll look at the _data case and then make a complete patch.

The _data case needs binary search, and seqnums are probably not guaranteed to strictly correspond to array indices.  Correct?

I am happy for now with the attached patch, but that doesn't do much about robustness against file corruption.

Created attachment 79382 [details] [review]
Ttry to avoid binary search when looking up by seqnum.