Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
We should pass the url query string to the purge method. What Varnish decides to do with the query string -- we can change that in the VCL file, but the query string should be passed nonetheless. I'm passing the pager query string for comments, so comments that show up on multiple pages for the same node can get flushed.
Currently, in purge_urls() in purge.inc, you have:
// The default PURGE method. Native to Squid and configurable in Varnish and Nginx
if ($method == 'purge') {
// Make it a PURGE request (not GET or POST)
$purge_requests[$current_purge_request]['request_method'] = 'PURGE';
// Construct a new url
$proxy_url_base = $proxy_url_parts['scheme'] . "://" . $proxy_url_parts['host'];
if (array_key_exists('port', $proxy_url_parts)) {
$proxy_url_base = $proxy_url_base . ":" . $proxy_url_parts['port'];
}
$purge_requests[$current_purge_request]['purge_url'] = $proxy_url_base . $purge_url_parts['path'];
// Set the host header to the sites hostname
$purge_requests[$current_purge_request]['headers'] = array("Host: " . $purge_url_host);
}
I'd like to suggest:
// The default PURGE method. Native to Squid and configurable in Varnish and Nginx
if ($method == 'purge') {
// Make it a PURGE request (not GET or POST)
$purge_requests[$current_purge_request]['request_method'] = 'PURGE';
// Construct a new url
$proxy_url_base = $proxy_url_parts['scheme'] . "://" . $proxy_url_parts['host'];
if (array_key_exists('port', $proxy_url_parts)) {
$proxy_url_base = $proxy_url_base . ":" . $proxy_url_parts['port'];
}
$purge_path = array_key_exists('query', $purge_url_parts) ? $purge_url_parts['path'] . '?' . $purge_url_parts['query'] : $purge_url_parts['path'];
$purge_requests[$current_purge_request]['purge_url'] = $proxy_url_base . $purge_path;
// Set the host header to the sites hostname
$purge_requests[$current_purge_request]['headers'] = array("Host: " . $purge_url_host);
}
The modified lines are
$purge_path = array_key_exists('query', $purge_url_parts) ? $purge_url_parts['path'] . '?' . $purge_url_parts['query'] : $purge_url_parts['path'];
$purge_requests[$current_purge_request]['purge_url'] = $proxy_url_base . $purge_path;
Let me know what you think.
Comments
Comment #1
mauritsl CreditAttribution: mauritsl commentedWhen we are purging lists (i.e. views) we most of the times want to purge all pages. Expanding a list of URL's to all pages can cost a lot of resources.
I suggest adding a wildcard in the PURGE configuration of varnish to expire that page with all possible GET params ( req.url ~ /view_page(\?.*)?^ )
Comment #2
djbobbydrake CreditAttribution: djbobbydrake commentedThanks for the reply. We're going to go this route. I have an additional question: so when this purge request is made, it's added to Varnish's purge list. Does Varnish actually keep track of a purge request for "/node/123/comments?page=1" vs "/node/123/comments?page=5" if the match in the purge list is a regex? How long does that regex stay in that purge list?
Comment #3
SqyD CreditAttribution: SqyD commentedif I understand it correctly Varnish (which confusingly uses the term "ban" instead of purge since Varnish 3.0) will keep bans in the same chronological lists as it's cached objects. Any requested objects matching the ban first before any cached object in that list will not be served from cache and will be fetched. So an object stored in cache after a matching ban has been issued will be served from cache. With this method a single url or a regex will be handled exactly the same without much performance costs. It's quite simple and brilliant at the same time.
By the end of this week my schedule should allow for some quality time with these modules. I hope to have a go at your patches, feature requests and these ideas.
Comment #4
djbobbydrake CreditAttribution: djbobbydrake commentedThx, that makes sense. We're keeping the patch I suggested above - it allows us to do stuff like tack on "?varnish=x" to our list of urls to purge. Then, in the vcl file, we can match for "?varnish=x", and build some flushing logic around that.
Comment #5
SqyD CreditAttribution: SqyD commentedOk, this makes sense to me as well. I've just committed a commented version of this for D6 and D7.
@djbobbydrake Please review/test them so I can include this in the next stable release. Thanks!
Comment #6
djbobbydrake CreditAttribution: djbobbydrake commentedI can confirm this works for D6, since this is the patch that we have been running in our environment for a few months. Thanks for making the change!
Comment #7
SqyD CreditAttribution: SqyD commentedI also added this feature to the other GET and AH methodes for both d6 and D7. Thanks!
Comment #8
omega8cc CreditAttribution: omega8cc commentedNow it is broken for GET method in Nginx completely.
You have copied the same code and probably forgot that there is a "key" query string
?purge_method=get
already.We are using separate cache keys for mobile devices, so it looks like:
$conf['purge_proxy_urls'] = "http://127.0.0.1:8888/purge-normal?purge_method=get http://127.0.0.1:8888/purge-mobile-tablet?purge_method=get"
With this patch applied for GET method the log for purge server looks like below:
After reverting this patch it works again:
Comment #9
djbobbydrake CreditAttribution: djbobbydrake commentedWould the solution be to ignore that one query string if using Nginx?
Comment #10
omega8cc CreditAttribution: omega8cc commentedProbably. The
purge_method=get
should be ignored/excluded inGET
method logic, as it has special purpose.Comment #11
SqyD CreditAttribution: SqyD commentedOkay, finally figured this one out. Must admit that djbobbydrake's "advanced" php syntax threw me off since I've never encountered that shorthand.
I've refactored the code so it will retain both path and query string and make sure no double slashes are added.
Code will end up in 6.x-1.x and 7.x-1.x branches.
Thank you all for your patience...
Comment #12
SqyD CreditAttribution: SqyD commentedfixed in 1.6 releasese for D6 and D7.