To avoid this 'going around releaseRequest() API' I would like
to suggest renaming 'RELEASE_REQUEST' flag to something
more suitable, e.g., 'ENTRY_REUSABLE' and adding a helper
API avoiding direct flag manipulation. This would not contradict
with the existing setReleaseFlag() meaning (i.e., "marking the
corresponding entry for eventual removal"), adding an additional
meaning (i.e., since then, the entry is not suggested to be re-used
by others). This intersects a little with the existing 'private entries'
definition. However, private entries may eventually become public,
while an entry with ENTRY_REUSABLE unset, would stay 'private'
until it is removed.
Eduard.
On 24.07.2017 02:00, Eduard Bagdasaryan wrote:
On 23.07.2017 02:04, Alex Rousskov wrote:
+ if (!flags.cachable)
+ EBIT_SET(e->flags, RELEASE_REQUEST);
This release request feels out of place and direct flags setting goes
around the existing releaseRequest() API. Please check all callers --
perhaps we do not need the above because all callers already do an
equivalent action (e.g., makePrivate()) for "uncachable" requests?
I don't think this lines are 'out of place': storeCreatePureEntry() just
initializes the just created StoreEntry fields (including
StoreEntry::flags) with correct values. If we definitely know a
this moment that 'flags' should have RELEASE_REQUEST set, why do we need
to postpone this to many callers, hoping that all of them will do that
work correctly? There are lots of storeCreateEntry() calls and it is
hardly possible to track that all of them end up with
'releaseRequest()', when flags.cachable is false. BTW, at the time of
StoreEntry initialization we do not need to do most of the work
releaseRequest() does. E.g., there are no connected storages to
disconnect from, no public keys to make them private, etc. The only
thing to do is RELEASE_REQUEST flag setting.
_______________________________________________
squid-dev mailing list
[email protected]
http://lists.squid-cache.org/listinfo/squid-dev
