Skip to content
New issue

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

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

Already on GitHub? # to your account

EZP-29194: Improve repo prefix handling for purgeAll() and user context hash tagging #73

Merged
merged 6 commits into from
Jan 15, 2019

Conversation

andrerom
Copy link
Contributor

@andrerom andrerom commented Dec 18, 2018

Issue: https://jira.ez.no/browse/EZP-29194

Followup to fix the following edge cases:

  • Prefix tag for user context hash, so it can be correctly purged when repo prefixes er in use
  • Always tag responses with unprefixed ez-all, as this is used in purgeAll() on Varnish and Fastly. Makes sure purgeAll() works the same in all Proxies; clearing all cache across all repositories.
    • Note: To clear all cache but only for a specific repo, use (purge(['ez-all'])) or from command line using fos command to clear by ez-all tag, these calls will/should be prefixed.

Follow up on this is to add a optional cache clearer for symfony that takes advantage of the fixes in purgeAll() to call that when user is calling bin/console cache:clear

Optional argument given from repo prefixer, take advantage in VarnishPurgeClient.
(and next step will be to take advantage in Fastly too).
@andrerom
Copy link
Contributor Author

andrerom commented Dec 19, 2018

In ExceptionConversion.php line 104:
                  
  Database error  
                  
In AbstractMySQLDriver.php line 42:
                                                                               
  An exception occurred while executing 'SELECT `ezuser`.`contentobject_id`,   
  `ezuser`.`login`, `ezuser`.`email`, `ezuser`.`password_hash`, `ezuser`.`pas  
  sword_hash_type`, `ezuser_setting`.`is_enabled`, `ezuser_setting`.`max_logi  
  n` FROM `ezuser` LEFT JOIN `ezuser_setting` ON `ezuser_setting`.`user_id` =  
   `ezuser`.`contentobject_id` WHERE LOWER(`ezuser`.`login`) = :placeholder1'  
   with params ["admin"]:                                                      
                                                                               
  SQLSTATE[42S02]: Base table or view not found: 1146 Table 'ezp.ezuser' does  
  n't exist   

@adamwojs rings a bell? (this is against ezplatform:master)

Seems this is unrelated

@andrerom andrerom requested a review from kmadejski January 2, 2019 20:03
@andrerom
Copy link
Contributor Author

andrerom commented Jan 14, 2019

@vidarl Passing and ready for your review (behat failure is unrelated to PR)
Same goes for you QA.

@andrerom andrerom changed the title Improve repo prefix handling for purgeAll and user context hash tagging EZP-29194: Improve repo prefix handling for purgeAll and user context hash tagging Jan 14, 2019
@andrerom andrerom changed the title EZP-29194: Improve repo prefix handling for purgeAll and user context hash tagging EZP-29194: Improve repo prefix handling for purgeAll() and user context hash tagging Jan 14, 2019
Copy link
Member

@vidarl vidarl left a comment

Choose a reason for hiding this comment

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

LGTM, but I have a few comments

So, prefix on default install will be "default_" ?
One worry I have with this prefix idea is that key size will increase and varnish and fastly have a limitation on header sizes

@andrerom
Copy link
Contributor Author

andrerom commented Jan 15, 2019

So, prefix on default install will be "default_" ?

afaik default have a empty prefix

But yes, even when this is not set we have had cases where customers have reached the limit on Varnish. So @kmadejski added doc on how to extend the limit as it's configurable. Last followup on that here: https://github.com/ezsystems/developer-documentation/pull/491/files

Limit for Fastly seems to be 16Kb:
https://docs.fastly.com/guides/debugging/resource-limits#request-and-header-limits

So seems to be twice the default 8Kb limit of Varnish, but as it's not configurable that might be an issue in extreme cases.

We have talked about maybe cutting down on the tag sizes and phase out the current tags in favour of shorter variants. Afaik that is the only thing we can do, besides documenting that people should use short repo names (we can also log warnings about that here btw).

@andrerom andrerom merged commit 68535d2 into 0.8 Jan 15, 2019
@andrerom andrerom deleted the repo_prefix_fixes branch January 15, 2019 19:19
# for free to join this conversation on GitHub. Already have an account? # to comment
Development

Successfully merging this pull request may close these issues.

4 participants