Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[3.x][VCL] Update purge logic from FOS #118

Merged
merged 2 commits into from
Mar 13, 2020
Merged

[3.x][VCL] Update purge logic from FOS #118

merged 2 commits into from
Mar 13, 2020

Conversation

andrerom
Copy link
Contributor

Question Answer
JIRA issue EZP-XXXXX
Type Improvement
Target version master
BC breaks 🤷‍♂
Doc needed maybe

Updates VCL to be in sync with the purge logic from FOS.
This drops eZ specific Ban which was Varnish specific, and adds support for FOS capability to do hard tag purge.

@andrerom andrerom requested a review from ViniTou February 18, 2020 19:01
Copy link
Contributor

@ViniTou ViniTou left a comment

Choose a reason for hiding this comment

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

Seems like a proper update to our logic based on what is in FOS, just mind I am no varnish expert, I just compared logic in both sources.

@ViniTou ViniTou requested a review from kmadejski February 19, 2020 09:09
@@ -127,32 +127,33 @@ sub ez_purge {
// Retrieve purge token, needs to be here due to restart, match for PURGE method done within
call ez_invalidate_token;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not a regression from this PR, however the code in this subroutine will need to be adapted to also take into account req.method == "PURGEKEYS".

Ref: if (req.restarts == 0 && req.method == "PURGE" && req.http.x-invalidate-token) {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Proposal for fix on that in e377e19

Copy link
Member

@kmadejski kmadejski left a comment

Choose a reason for hiding this comment

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

Looks good to me :)

@micszo
Copy link
Member

micszo commented Mar 2, 2020

@andrerom What is the recommended way of switching between purge and softpurge in this approach? 🙂

@andrerom
Copy link
Contributor Author

andrerom commented Mar 2, 2020

@micszo It's a FOSHttpCache config option, soft is used by default: https://foshttpcachebundle.readthedocs.io/en/latest/reference/configuration/proxy-client.html#tag-mode

Copy link
Member

@micszo micszo left a comment

Choose a reason for hiding this comment

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

Verified with sanities on eZ Platform EE master with Varnish 6.1 with softpurge and purge.
In fact resolves issues encountered earlier on v3 (#107 (comment)).

@micszo
Copy link
Member

micszo commented Mar 3, 2020

Nota bene, @mnocon aptly noticed that there also is https://github.com/ezsystems/ezplatform/blob/master/.platform/varnish.vcl. Will that one have to be adapted to these changes as well?

@andrerom
Copy link
Contributor Author

andrerom commented Mar 3, 2020

Will that one have to be adapted to these changes as well?

not planned on my side, I'd suggest we find a way to drop it and use the one from here.
@vidarl Might have input on how that can be possible to be done.

Same with this btw:
https://github.com/ezsystems/ezplatform/blob/master/doc/varnish/vcl/varnish4_xkey.vcl

@andrerom
Copy link
Contributor Author

andrerom commented Mar 13, 2020

@micszo Changes for meta repo (update platform.sh VCL + remove the other one which was deprecated): ezsystems/ezplatform#505

@micszo micszo removed their assignment Mar 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

4 participants