This setting gets set on the config page, but it's never referenced in any other part of the code.

There's a TODO about this in recently_read_entity_view()

CommentFileSizeAuthor
#3 recently_read-max_record_implementation-2778623-2.patch2.01 KBaaronbauman
#2 recently_read-max_record_implementation-2778623.patch1.85 KBaaronbauman

Comments

aaronbauman created an issue. See original summary.

aaronbauman’s picture

aaronbauman
he/him
commented
Status: Active » Needs review
StatusFileSize
new recently_read-max_record_implementation-2778623.patch1.85 KB

After digging into this, it's going to be very expensive to accomplish during cron.

Better to clean up just-in-time, inside recently_read_entity_view()

aaronbauman’s picture

aaronbauman
he/him
commented
StatusFileSize
new recently_read-max_record_implementation-2778623-2.patch2.01 KB

derp. typo.

zterry95’s picture

zterry95 commented

After digging into this, it's going to be very expensive to accomplish during cron.

Better to clean up just-in-time, inside recently_read_entity_view()

Why it's expensive in cron? I would prefer the clean up the records in cron. like what the watchdog module do.

http://cgit.drupalcode.org/drupal/tree/modules/dblog/dblog.module?h=7.x

aaronbauman’s picture

aaronbauman
he/him
commented

Watchdog can do a very simple query to clean up during cron:
"delete from watchdog where timestamp < $cutoff"

Recently Read needs to do a lot more:
- different limits per entity
- different cutoff time per session, per entity

This makes the procedure up to 2 orders of magnitude more expensive than the watchdog query.
For a sufficiently large site, this could cause significant performance overhead during cron.

pseudo-code:

for each rr_config:
  records = "SELECT sid, timestamp as cutoff FROM recently_read WHERE entity_type = $entity_type ORDER BY sid, timestamp DESC LIMIT $max_record"
  for each records:
    if ( is_least_timestamp_for_sid($record) ) :
      "DELETE FROM recently_read WHERE entity_type = $entity_type AND sid = $sid AND timestamp < $cutoff"

There are tradeoffs to both approaches, but cleaning up during hook_view seemed cleaner and more efficient to me.

zterry95’s picture

zterry95 commented

After think of it, I still prefer to do it on the cron.

in the actual scenario, only a few entity types will be tracked by recently read. so when we clean it in the cron, it's won't be a heavy task.
There are also other benefit by using cron; like we can use elysia_cron or ultimate_cron to do the clean up job, which I think won't cause significant performance overhead during cron.

otherwise, if we check the max_records on hook_entity_view; it will consume the resource on each page view. this will slow down the whole page response in somehow.

aaronbauman’s picture

aaronbauman
he/him
commented
Assigned: aaronbauman » Unassigned
Status: Needs review » Active
Anonymous’s picture

Anonymous (not verified) commented
Status: Active » Closed (outdated)

Closing this issue as it's opened against a version of module meant for Drupal 7 which EOL.