[squid-dev] [PATCH] Revalidate without Last-Modified

[Alex Rousskov]

On 08/23/2016 09:17 AM, Amos Jeffries wrote:
> On 24/08/2016 12:07 a.m., Eduard Bagdasaryan wrote:
>> 2016-08-21 15:58 GMT+03:00 Amos Jeffries <squid3 at treenet.co.nz>:
>>> To change anything between those markers we have to do a full cache
>>> versioning and up/down-grade compatibility dance.

>> Could you please clarify what versioning problems you are talking
>> about?

> I was referring to that block of members being read and written
> directly to cache files. Either swap.state or the object metadata section.
> I dont think they are referenced individully, but as a raw-pointer to
> the first members address cast to some other identically laid out
> StoreMeta* type (yucky).


Yes, this happens in at least two places:

> bzr grep timestamp src | fgrep \&
...
> src/store_rebuild.cc:            memcpy(&what->timestamp, x.value, STORE_HDR_METASIZE);
> src/store_swapmeta.cc:    t = StoreMeta::Factory(STORE_META_STD_LFS,STORE_HDR_METASIZE,&e->timestamp)


> The point of those comments AFAIK is to make it obvious that kind of
> this is/was being done with them and prevent us doing what you did.

Agreed. Since improving this code is way outside this simple fix scope,
I recommend returning the renamed lastmod data member into that "keep
them together" section, with a comment:

time_t lastmod_; ///< received Last-Modified value or -1; treat as private

Please note that "received Last-Modified value or -1" is just my guess
what lastmod_ is -- use the right description if my guess is wrong.

Renaming lastmod (and temporary making it private) already forced you to
check and, if needed, adjust most of the old code using that data
member. Keeping lastmod_ public is not ideal, but it does not make
things worse and allows you to stay focused. Fixing related problems is
only a good idea when those fixes are simple and do not stray you off
course too much.


HTH,

Alex.