Skip to content

Conversation

dbu
Copy link
Contributor

@dbu dbu commented Oct 24, 2016

fix #198
fix #182

@andrerom @lolautruche i realized that in varnish we don't send a hash header at all for anonymous users (anonymous on http level). i tried to better document this here, and synchronize the behaviour of the symfony subscriber with the varnish configuration.

can you please check if you agree with this and if this resolves the issues you had with the hashes?

i left the option to configure the anonymous hash to something, so a perfectionist could share the cache between anonymous users and authenticated users that have no additional permissions by specifying their hash as anonymous_hash.

@andrerom
Copy link
Contributor

andrerom commented Oct 24, 2016

Hmm, how would we go about clearing cache for anonymous user context hash now? As in if rights for anonymous changes, prior cache variations needs to be expired , and with no hash for no cookies anonymous users, I don't immediate see how that can be accomplished.

@dbu
Copy link
Contributor Author

dbu commented Oct 25, 2016

I think i am missing something here. In varnish, we don't have a hash header for anonymous user, so nothing new. For Symfony HttpCache, there is no BAN, so the header would not help with invalidating. Can you explain a bit more please?

I would assume that when permissions for user groups change, we need to reset the cache completely to be sure to catch everything.

@andrerom
Copy link
Contributor

andrerom commented Oct 25, 2016

In varnish, we don't have a hash header for anonymous user, so nothing new.

there is the hard coded hash being used instead right? so cache was at least in our case using this.

the header would not help with invalidating. Can you explain a bit more please?

Implies multi tagging and what I mentioned here #182 (comment)

I would assume that when permissions for user groups change, we need to reset the cache completely to be sure to catch everything.

if by completely you imply everything: this is what I'm trying to avoid, permissions in eZ is a adminstration tags via UI, can happen quite frequently sometimes with minor changes, especially for editor roles. And then invalidation gigs of varnish cache, even that for anonymous and basically taking down the site/app is not an acceptable way to handle this for high traffic installs.

if you imply just user hash lookup url, making varnish just lookup this again, then yes that would be ok.

@dbu
Copy link
Contributor Author

dbu commented Oct 25, 2016

there are (at least) 3 permission-related types of changes:

  1. permission on a content changes (what role is allowed to see this content). this needs a purge of that content url / ban of the tag of that content. not affected by what we do here
  2. permissions of a user change. we need to invalidate the hash lookup for that user. as we probably don't know that users current session(s), we probably have to invalidate all hash lookups. as the hash will have changed, we don't need to invalidate content.
  3. the application changes what permissions various content needs, not possible to identify affected content: invalidate everything

for the special case of changing the permissions of anonymous users, we always have a problem if we handle the case differently on the caching proxy. having no hash lookup is just a special case of using a hardcoded hash. either way the hash does not change when permissions change. so should we do the opposite of what i tried to do here, and do a lookup both in varnish and with symfony?

@dbu
Copy link
Contributor Author

dbu commented Oct 25, 2016 via email

* BC BREAK: Constructors for PurgeSubscriber and RefreshSubscriber now use an
options array for customization.
* By default, no hash header is sent for anonymous users anymore, to sync
behaviour with Varnish behaviour.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sounds a little vague. What Varnish behaviour do you mean?

skipped and no hash header added to the request. However, we can not avoid
the initial hash lookup request per different cookie, as the caching proxy
can not know which session cookies indicate a logged in user and which an
anonymous session.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps: ‘as there’s no way for the caching proxy to differentiate between cookies that belong to authenticated users and session cookies that belong to anonymous sessions’.

@ddeboer
Copy link
Member

ddeboer commented Oct 31, 2016

Some small comments, 👍 otherwise.

@dbu
Copy link
Contributor Author

dbu commented Oct 31, 2016

ping @lolautruche @andrerom @bdunogier are you ok if we go this way?

@andrerom
Copy link
Contributor

andrerom commented Nov 4, 2016

So returning to this, one concern on our side is:

permissions of a user change. we need to invalidate the hash lookup for that user. as we probably don't know that users current session(s), we probably have to invalidate all hash lookups. as the hash will have changed, we don't need to invalidate content.

This won't any longer be true for permission changes for anonymous, ideally would like to clear hash lookup to force new hash to be generated (assuming no hard coded hash is configured)

@bdunogier
Copy link

This won't any longer be true for permission changes for anonymous, ideally would like to clear hash lookup to force new hash to be generated (assuming no hard coded hash is configured)

If I'm not mistaken, the same is true if you change security rules affecting anonymous users (e.g. start protecting a route, or make a route anonymous), you do need to refresh cache items for anonymous, don't you ?

Has anybody reported scaling issues with that (like clearing gigs of cache at once) ?

@dbu
Copy link
Contributor Author

dbu commented Nov 5, 2016 via email

@dbu dbu force-pushed the cleanup-anonymous-user-context branch from 6031a11 to 15972db Compare November 5, 2016 10:23
@dbu dbu mentioned this pull request Nov 5, 2016
@andrerom
Copy link
Contributor

andrerom commented Nov 7, 2016

Has anybody reported scaling issues with that (like clearing gigs of
cache at once) ?

not to my knowledge.

we have had reports of that, but customers of that size/volume tends to just brute force disable invalidation and instead use a ttl cache with varnish to get more stable performance.

@dbu
Copy link
Contributor Author

dbu commented Nov 7, 2016 via email

@dbu
Copy link
Contributor Author

dbu commented Nov 15, 2016

replaced by #330

@dbu dbu closed this Nov 15, 2016
@ddeboer ddeboer deleted the cleanup-anonymous-user-context branch November 15, 2016 14:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add test for user context caching for different groups of anonymous users Remove context header for anonymous users

4 participants