Hi,
This seems to have fixed "test-journal-stream" but
"test-journal-interleaving" is still broken for me, it fails with:
NUMBER=1
NUMBER=2
Assertion 'r == 1' failed at
src/journal/test-journal-interleaving.c:101, function
test_check_numbers_down(). Aborting.
Aborted (core dumped)
Can you please double check?
Can you please make sure that "make check" passes before pushing the commits?
Thanks!
Filipe
On Fri, Dec 19, 2014 at 6:38 AM, Michal Schmidt
<mich...@kemper.freedesktop.org> wrote:
> src/journal/sd-journal.c | 66
> +++++++++++++++++++++--------------------------
> 1 file changed, 30 insertions(+), 36 deletions(-)
>
> New commits:
> commit 487d37209b30a536636c95479cfeba931fea25c5
> Author: Michal Schmidt <mschm...@redhat.com>
> Date: Fri Dec 19 15:05:30 2014 +0100
>
> journal: fix skipping of duplicate entries in iteration
>
> I accidentally broke the detection of duplicate entries in 7943f42275
> "journal: optimize iteration by returning previously found candidate
> entry".
>
> When we have a known location of a candidate entry, we must not return
> from next_beyond_location() immediately. We must go through the
> duplicates detection to make sure the candidate differs from the
> already iterated entry.
>
> This fix slows down iteration a bit, but it's still faster than it
> was before the rework.
>
> $ time ./journalctl --since=2014-06-01 --until=2014-07-01 > /dev/null
>
> real 0m4.448s
> user 0m4.298s
> sys 0m0.149s
>
> (Compare with results from commit 7943f42275, where real was 5.3s before
> the rework.)
>
> diff --git a/src/journal/sd-journal.c b/src/journal/sd-journal.c
> index 285e5e3..173f948 100644
> --- a/src/journal/sd-journal.c
> +++ b/src/journal/sd-journal.c
> @@ -412,60 +412,51 @@ _public_ void sd_journal_flush_matches(sd_journal *j) {
> detach_location(j);
> }
>
> -_pure_ static int compare_with_location(JournalFile *af, Object *ao,
> Location *l) {
> - uint64_t a;
> -
> - assert(af);
> - assert(ao);
> +_pure_ static int compare_with_location(JournalFile *f, Location *l) {
> + assert(f);
> assert(l);
> + assert(f->location_type == LOCATION_SEEK);
> assert(l->type == LOCATION_DISCRETE || l->type == LOCATION_SEEK);
>
> if (l->monotonic_set &&
> - sd_id128_equal(ao->entry.boot_id, l->boot_id) &&
> + sd_id128_equal(f->current_boot_id, l->boot_id) &&
> l->realtime_set &&
> - le64toh(ao->entry.realtime) == l->realtime &&
> + f->current_realtime == l->realtime &&
> l->xor_hash_set &&
> - le64toh(ao->entry.xor_hash) == l->xor_hash)
> + f->current_xor_hash == l->xor_hash)
> return 0;
>
> if (l->seqnum_set &&
> - sd_id128_equal(af->header->seqnum_id, l->seqnum_id)) {
> + sd_id128_equal(f->header->seqnum_id, l->seqnum_id)) {
>
> - a = le64toh(ao->entry.seqnum);
> -
> - if (a < l->seqnum)
> + if (f->current_seqnum < l->seqnum)
> return -1;
> - if (a > l->seqnum)
> + if (f->current_seqnum > l->seqnum)
> return 1;
> }
>
> if (l->monotonic_set &&
> - sd_id128_equal(ao->entry.boot_id, l->boot_id)) {
> -
> - a = le64toh(ao->entry.monotonic);
> + sd_id128_equal(f->current_boot_id, l->boot_id)) {
>
> - if (a < l->monotonic)
> + if (f->current_monotonic < l->monotonic)
> return -1;
> - if (a > l->monotonic)
> + if (f->current_monotonic > l->monotonic)
> return 1;
> }
>
> if (l->realtime_set) {
>
> - a = le64toh(ao->entry.realtime);
> -
> - if (a < l->realtime)
> + if (f->current_realtime < l->realtime)
> return -1;
> - if (a > l->realtime)
> + if (f->current_realtime > l->realtime)
> return 1;
> }
>
> if (l->xor_hash_set) {
> - a = le64toh(ao->entry.xor_hash);
>
> - if (a < l->xor_hash)
> + if (f->current_xor_hash < l->xor_hash)
> return -1;
> - if (a > l->xor_hash)
> + if (f->current_xor_hash > l->xor_hash)
> return 1;
> }
>
> @@ -740,21 +731,24 @@ static int next_beyond_location(sd_journal *j,
> JournalFile *f, direction_t direc
> f->last_n_entries = n_entries;
>
> if (f->last_direction == direction && f->current_offset > 0) {
> + cp = f->current_offset;
> +
> /* LOCATION_SEEK here means we did the work in a previous
> * iteration and the current location already points to a
> * candidate entry. */
> - if (f->location_type == LOCATION_SEEK)
> - return 1;
> -
> - cp = f->current_offset;
> + if (f->location_type != LOCATION_SEEK) {
> + r = next_with_matches(j, f, direction, &c, &cp);
> + if (r <= 0)
> + return r;
>
> - r = next_with_matches(j, f, direction, &c, &cp);
> - if (r <= 0)
> - return r;
> + journal_file_save_location(f, direction, c, cp);
> + }
> } else {
> r = find_location_with_matches(j, f, direction, &c, &cp);
> if (r <= 0)
> return r;
> +
> + journal_file_save_location(f, direction, c, cp);
> }
>
> /* OK, we found the spot, now let's advance until an entry
> @@ -769,20 +763,20 @@ static int next_beyond_location(sd_journal *j,
> JournalFile *f, direction_t direc
> if (j->current_location.type == LOCATION_DISCRETE) {
> int k;
>
> - k = compare_with_location(f, c,
> &j->current_location);
> + k = compare_with_location(f, &j->current_location);
>
> found = direction == DIRECTION_DOWN ? k > 0 : k < 0;
> } else
> found = true;
>
> - if (found) {
> - journal_file_save_location(f, direction, c, cp);
> + if (found)
> return 1;
> - }
>
> r = next_with_matches(j, f, direction, &c, &cp);
> if (r <= 0)
> return r;
> +
> + journal_file_save_location(f, direction, c, cp);
> }
> }
>
>
> _______________________________________________
> systemd-commits mailing list
> systemd-comm...@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-commits
_______________________________________________
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
