Improve performance on high-traffic sites by making database writes optional

Making the database writes optional is a good idea, however removing them all together is a better option. Marking as a duplicate of #1263832: Remove session-based flood / loop detection since it performs poorly, and has unfixable race conditions

I'm confused @mfb, in #1263832: Remove session-based flood / loop detection since it performs poorly, and has unfixable race conditions you argue for the removal of this. Should we change back to this issue and make it optional?

Right ok. So given there consensus in #1263832: Remove session-based flood / loop detection since it performs poorly, and has unfixable race conditions to remove loop detection can this issue be refocused on only making redirect logging optional?

Yes, this should definitely be refocused.

I think that everytime a new variable is used, we should add it to the redirect_variables() function.

It works, thanks

I think this could be made a bit more clear 'access log' of redirects doesn't really match what is happening. I would prefer this is exposed in the UI more like the core statistics module counter. Something more like 'Count redirects'

I'm actually going to take this in a separate direction. I'd rather have a separate module for handling the logging and expiration of unused redirects.

Much agree that supporting deletion of unused redirects should not add complexity to the basic module.

I'm still much interested in when a redirect was created and it it was used ever.
Adding a create timestamp is still subject for redirect core functionality: #1853144: Store created date/time
How about the access timestamp then?

access timestamp should still be in a separate table and not available from an entity_load() of a redirect. I was thinking a redirect_access table with rid, timestamp, uid, hostname columns for when a redirect is "logged."

Agree this is a proper approach. Are we rescoping this issue to include the redirect_access table (or stay with expiring topic only and split that log rewrite into a separate issue)?

I was planning on using this issue to add the new redirect_access submodule to the 7.x-1.x codebase for 1.0 release.

Updated the issue summary.

This rework makes it related to....

This feature has been completely removed from the 8.x-1.x port, at least for now.

With caching of redirects enabled by default (and reliably, thanks to cache tags), it was no longer possible to rely on it.

I've decided I can't change something major like this in the 1.x branch, so I'll be opening a 2.x branch shortly for this new sub-module.

doesn't seem to apply anymore

I've decided I can't change something major like this in the 1.x branch, so I'll be opening a 2.x branch shortly for this new sub-module.

The latest patch in #17 does not apply to the latest 7.x-2.x-dev.

Checking patch redirect.admin.inc...
error: while searching for:
    '#default_value' => variable_get('redirect_purge_inactive', 0),
    '#options' => array(0 => t('Never (do not discard)')) + drupal_map_assoc(array(604800, 1209600, 1814400, 2592000, 5184000, 7776000, 10368000, 15552000, 31536000), 'format_interval'),
    '#description' => t('Only redirects managed by the redirect module itself will be deleted. Redirects managed by other modules will be left alone.'),
    '#disabled' => variable_get('redirect_page_cache', 0) && !variable_get('page_cache_invoke_hooks', TRUE),
  );

  $form['globals'] = array(

error: patch failed: redirect.admin.inc:634
error: redirect.admin.inc: patch does not apply
Checking patch redirect.module...
Hunk #1 succeeded at 345 (offset 1 line).
error: while searching for:
    $interval = variable_get('redirect_purge_inactive', 0);
  }

  if (!$interval || !variable_get('redirect_page_cache', 0) || !variable_get('page_cache_invoke_hooks', TRUE)) {
    // If serving redirects from the page cache is enabled and hooks are not
    // executed during page caching, then we cannot track when a redirect is
    // used. Therefore, we cannot remove unused redirects.

error: patch failed: redirect.module:935
error: redirect.module: patch does not apply

@mfb, thank you. The rerolled patch in #31 to redirect.admin.inc and redirect.module does apply cleanly to both 7.x-1.x-dev and 7.x-2.x-dev. I agree with your comments, but I'll leave the issue metadata as it is for now so that someone else more experienced than myself to update as necessary.