[systemd-devel] Journal tests broken by commit 6573ef05a3cbe1 ("journal: keep per-JournalFile location info during iteration")

[Filipe Brandenburger]

I took a closer look at "test-journal-stream". It's setting up 3
journal files and writing entries to the three of them with some
interleaving, then expecting to read them in order.

After commit 6573ef05a3cbe1, what happens is that a single call to
sd_journal_next() ends up calling next_beyond_location() on the three
files which advances the f->current_offset from the three of them, so
the first access looks right, you'll be comparing the first entry of
each of the three files, but the second access is bad because you're
comparing the second entry of each of the three files, instead of the
second entry of the one that was picked before with the first entry of
the other two files.

Not sure what's the correct solution, maybe journal_file_save_location
needs to happen only in real_journal_next() outside the
ORDERED_HASHMAP_FOREACH loop? I'll try that and report if I find
something that seems to solve this problem...

Cheers,
Filipe


On Thu, Dec 18, 2014 at 2:34 PM, Filipe Brandenburger
<filbranden at google.com> wrote:
> Hi,
>
> Commit 6573ef05a3cbe1 ("journal: keep per-JournalFile location info
> during iteration") breaks tests "test-journal-stream" and
> "test-journal-interleaving".
>
> It seems that the logic of overriding f->current_offset in
> journal_file_save_location has other unintended side effects, checking
> out that commit and commenting out that line seems to have
> "test-journal-stream" working back again, but not
> "test-journal-interleaving". The same no longer has effect in trunk
> head anymore since I believe the follow up patches rely on that saved
> information which is no longer there.
>
> Not sure if the problem is in the code or in the tests, but looking at
> the tests the logic looks right to me, so I'm leaning towards the
> code...
>
> Cheers,
> Filipe