In watchdog.module I propose changing the key used to sort logs chronologically. Instead of using the timestamp, which is accurate only to the nearest second, we could use the log entry id ("wid" in the database). As I understand it, the IDs are assigned sequentially, so this is a good approximation to the correct order in which the events are recorded. Also, I don't know of any events that are recorded after a delay.

The problem with the current sorting key arises when logged events come within 1 second of each other. The order of events is not always preserved when viewing the log because the timestamp is identical for several events.

In "modules/watchdog.module" neard line 91, the word "timestamp" is replaced with "wid":

$header = array(
    ' ',
    array('data' => t('Type'), 'field' => 'w.type'),
    array('data' => t('Date'), 'field' => 'w.wid', 'sort' => 'desc'),
    array('data' => t('Message'), 'field' => 'w.message'),
    array('data' => t('User'), 'field' => 'u.name'),
    array('data' => t('Operations'), 'colspan' => '2')
  );

Preliminary testing suggests this is a good tweak.

CommentFileSizeAuthor
#3 wid.patch782 byteskilles@www.drop.org

Comments

Steven’s picture

Steven commented

Do auto_increment id's never go back to fill in deleted rows? We need to make sure on pgsql and mysql.

Another option might be to sort by timestamp first, then by wid. I'm not sure if the current tablesort supports that though.

dries’s picture

dries commented

Also, I think auto_increment IDs rotate/overflow. This isn't very likely though. Currently, drupal.org's largest wid value is appr. 1,500,000.

killes@www.drop.org’s picture

killes@www.drop.org commented
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new wid.patch782 bytes

According to http://dev.mysql.com/doc/mysql/en/example-auto-increment.html
auto_increments ususally do not get re-used.

The patch is attached and tested.

jeremy’s picture

jeremy commented

Does the tablesort function allow using two keys to sort results? If you first sort on timestamp, then on wid, you wouldn't have to worry about id's being reused or wrapping.

moshe weitzman’s picture

moshe weitzman commented
Status: Reviewed & tested by the community » Needs review

we made a decision a long time ago to support DBMS which don't have autoincrement fields. As a result, we implemented our own sequences table and almost all modules use this. Any module which relies on autoincrement instead of sequences is buggy (according to current policy).

if we change our policy on this, then we should rip out all the sequences stuff. Until we do that, I can't support relying on autoincrement. I'd love for others to chime in here - marking this as 'needs review'

killes@www.drop.org’s picture

killes@www.drop.org commented
Status: Needs review » Reviewed & tested by the community

@Jeremy: Sorting by two fields does not seem to work.

@Moshe: This code does not rely on the fact that wid is an auto_increment field in any way. Just some concerns did.

Crell’s picture

Crell commented

Current policy or no, isn't part of the point of unique IDs in SQL that you are guaranteed that they are unique, not that they are sequential? Relying on them to be sequential is a problem, because you can't always guarantee that they will be.

If you want greater than one second resolution for timestamps, I'd suggest replacing the SQL timestamp filed with a microsecond-sensitive numeric field and storing the timestamp that way. It can then be converted back to a YYYY-MM-DD (or something else) timestamp on the fly, after it's already been sorted by the SQL query. PHP's microtime() function would be useful here.

That should be database-agnostic, too.

Steven’s picture

Steven commented
Status: Reviewed & tested by the community » Fixed

Committed to head/4.6.

Anonymous’s picture

(not verified) commented
Anonymous’s picture

(not verified) commented
Anonymous’s picture

(not verified) commented
Anonymous’s picture

(not verified) commented
Status: Fixed » Closed (fixed)