Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

await_next_record seems to return old records while triggering on new records. #66

Open
bestouff opened this issue May 24, 2019 · 5 comments

Comments

@bestouff
Copy link

In my executable one my my threads basically does:

    let mut log = Journal::open(systemd::journal::JournalFiles::All, true, true)
        .expect("unable to open systemd journal");
    loop {
        if let Ok(Some(record)) = log.await_next_record(None) {
            ... send the record somewhere
        }
    }

and apparently this triggers on new log entries, but returns one old new entry each time (i.e. I see it displaying kernel boot entries, one line at a time, instead of what is happening now).

@bestouff
Copy link
Author

Here's how I had to fix it:

    let mut log = Journal::open(systemd::journal::JournalFiles::All, true, true)
        .expect("unable to open systemd journal");
    loop {
        while let Ok(Some(record)) = log.next_record() {
            ... send the existing records somewhere
        }
        if let Ok(Some(record)) = log.await_next_record(None) {
            ... send the next records somewhere
        }
    }

@jlebon
Copy link
Contributor

jlebon commented May 24, 2019

Hmm, yeah, await_next_record() should probably implicitly do a seek(Tail)? Otherwise, maybe try doing the seek() yourself before entering the loop?

@bestouff
Copy link
Author

There will always be a race condition if I do that: what if a log record appears between the ‘seek‘ and the ‘await_next_record‘ ?

@codyps codyps added the docs label Sep 7, 2020
@codyps
Copy link
Owner

codyps commented Sep 7, 2020

Internally, await_next_record() is essentially calling:

sd_journal_wait();
have_entry = sd_journal_next();
if have_entry {
    while sd_journal_enumerate_data(...) {

    }
    return Some(Map)
}
return None

The issue with calling that repeatedly is that we wait() might have gotten multiple entries, but won't return again until another entry is appended. As a result, we end up permenantly behind processing the journal (by some amount of entries that grows faster when the journal is getting entries written to it faster).

The right way to do this (in sd-journal apis) is something like:

loop {
    sd_journal_wait();
    loop {
         have_entry = sd_journal_next()
         if !have_entry {
            break
         }

         // process entry
    }
}

so:

  • await_next_record is probably not the construction we really want to use, and potentially should be removed.
  • barring that, it's docs need fixing

@codyps
Copy link
Owner

codyps commented Sep 7, 2020

#124 helps this. I've added an example of using the low-level interfaces and made wait() public. Still probably should consider removing await_next_record entirely. It seems like removing all the functions that are using get_entry (next_entry, prev_entry) etc would be a good idea, and that sweeps up await_next_record as well.

@codyps codyps added the bug label Oct 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants