cache_set() does not take a headers parameter

Thanks a lot for issuing!

This patch looks good, except that the $headers parameter is ignored by the DrupalCacheInterface::set() method: this problem is due to the radical change of caching API between D6 and D7.

Your patch does not fixes entirely the problem, we have to find a deeper solution: let's start by looking a bit deeper into how cache backends handles their data. Any help/suggestion is welcome!

Thanks again for feedback, ll wait to ear from you then. If I accidentaly (ahah) end up with some spare time, I will do some research on my side, I cant do no promise for the future 2 weeks.

With the current Cache Backport (D7 to D6) cache.inc
method I have the issue of seeing these PHP warning notices:

Via Pressflow: 6.22

PHP Warning: Invalid argument supplied for foreach()
in /bootstrap.inc on line 1126/1166

The code affecting this 'warning' is from D6 Core [Pressflow] bootstrap.inc:

function drupal_page_cache_header(stdClass $cache) {
..
  $default_headers = array();

  foreach ($cache->headers as $name => $value) {   // #line: 1126
..
  // Send the remaining headers.
  foreach ($cache->headers as $name => $value) {  // #line: 1166
    drupal_set_header($name, $value);
  }

Therefore, I conclude that cache.inc (via Cache Backport) should be changed:

-function cache_set($cid, $data, $bin = 'cache', $expire = CACHE_PERMANENT) {
-  return _cache_get_object($bin)->set($cid, $data, $expire);
+function cache_set($cid, $data, $bin = 'cache', $expire = CACHE_PERMANENT, $headers = NULL) {
+  return _cache_get_object($bin)->set($cid, $data, $expire, $headers);

Thus, I agree with the above patch: -> http://drupal.org/files/cache_set_headers_parameter-1297754-1.patch

Also note that 'drupal_apc_cache.inc' steps out of the D6 $headers API level,
and uses:

function set($cid, $data, $expire = CACHE_PERMANENT, array $headers = NULL) {  // as it is for D7
function set($cid, $data, $expire = CACHE_PERMANENT, $headers = NULL) {       // as it should be for D6

I understand the second line (above) is correct for D6.

Sorry!
Posted twice :-)
Slow Server

Ho thanks for the detailed report, I will take some time to see this as soon as possible. Could you provide a patch that does not implies patching the cache backends?

EDIT: Just saw it, let me have a look.

The problem is that D7 API doesn't seem to care at all about headers. The only solution I see in order to be able to handle them correctly without changing the interface signature is that if $headers is present, then I wrap the cache into a specific object that carries the headers with it, on cache_set(), and at cache_get() time I check if I have a wrapper object, is so I rebuild the correct cache object with the headers property. Should work.

@peter bowey
Does the attached patch fixes you problem?

After some testing, I decided to push it as-is, it does not creates any regression over my environment and I don't get the headers warnings anymore.

Re-open if the new 6.x-1.0-rc1 release does not fix the problem.

Thanks to both of you!

Refer #10

Many thanks pounard for your latest work,
I have just installed and tested Cache Backport 6.x-1.0-rc1.

My previous noted issue with the PHP warning notices - no longer occurs.

I am still testing heavily, but no [zero] problems to report at this moment!
Well done!

@ Refer #10

After testing 'new 6.x-1.0-rc1' over-night,
I find that the APC "Cached Variables"
show a statistic drop in hits of about 50%.

User Cache Information
Cached Variables	212 ( 1.3 MBytes)
Hits	317
Misses	239

Moving back to the pre-6.x-1.0-rc1 release, with
older [patch] methods (refer #5). (just to get to stats for comparison)

@ #12
Reverting to older code +patch (refer #5) gave these results,
(after a few minutes testing):

File Cache Information
Cached Files	356 ( 33.7 MBytes)
Hits	11556
Misses	356
Request Rate (hits, misses)	12.28 cache requests/second
Hit Rate	11.91 cache requests/second
Miss Rate	0.37 cache requests/second
Insert Rate	0.37 cache requests/second
Cache full count	0
User Cache Information
Cached Variables	281 ( 1.9 MBytes)
Hits	1932
Misses	1439
Request Rate (hits, misses)	3.48 cache requests/second
Hit Rate	1.99 cache requests/second
Miss Rate	1.48 cache requests/second
Insert Rate	3.12 cache requests/second
Cache full count	0

The # of cached variables is higher than before (refer #10)

The test in #10 was over-night,
the test here (#13) was over a few minutes.

How should I interpret those figures?

Remember that the $headers parameter is not part of the Drupal 7 DrupalCacheInterface interface, APC implementation is wrongly using it, so #5 patch is not acceptable because it will just make $headers being dropped by most backends.