-
Notifications
You must be signed in to change notification settings - Fork 60
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
LiteSpeed support #444
LiteSpeed support #444
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cool, thanks for this pull request!
interesting approach. is lightspeed supposed to be used in multi-node setup? either way we should discuss that a bit in the documentation to avoid confusion.
the header file is a good solution, but seems against the philosophy of lightspeed. it should actually only be needed if we invalidate from a non-web context. when there is a web request that triggers the things that trigger invalidation, we would ideally set the headers on the web response.
could we support this pattern here? LiteSpeed client could have an additional method to update a PSR-7 response with invalidation for that use case. we would still call flush, in case either there was no web response or when invalidations happen after the response has been sent. for the symfony bundle, we can extend the client to also be able to update a symfony response.
its a bit borderline to push that job into the client, but i think its in the domain of the lightspeed client. we could maybe generalize this concept to define an interface for this pattern to have our listeners check for an interface instead of litespeed specifically.
I think you got something about LiteSpeed wrong in general. LiteSpeed is a drop-in-replacement for Apache. It's not an Apache module but it supports |
maybe we should mention that in the lead then ;-) multi-node setup is still very much a question though. apache can act as a reverse proxy. if you'd use litespeed that way, you would need to make sure the request to selve goes to the correct backend, or your generated header file is not there. if you use multiple litespeed behind a loadbalancer, you'd need to make sure that each of them gets the request (that they are all in the list of servers) so they see the invalidation resposne. and also that the file is generated on each instance. |
I think I'd rather mention that multi-node stuff is not a covered use-case at all. I won't work on support for that because I won't need it. |
i think thats good. lets state that explicitly in the documentation, and mention that if you need more than one server with your app, varnish is a good solution for the caching reverse proxy, and its fully supported already. |
Hi, dbu, This is Lauren from LiteSpeedTech. We also have WebADC product, which follows the same headers. So this solution is generic, if it works with LiteSpeed Web Server, it will work with WebADC. WebADC is a full featured Load balancer with LSCache, ESI, HTTP/2, QUIC support built in. We already have many clients used it production environment, especially for big Magento stores. Thanks for great effort Toflar put in. We can together improve this. This is something we always wanted to do, but haven't got chance. We have a Free Starter license (allow for 1 Domain, 2GB memory), this allows more devs can give a try in his local environment. Hopefully we can get more feedback. Thanks, |
It's the only method that is supported to invalidate cache tags and I actually find it a pretty nice solution. It's really easy to setup and it works across different applications or frameworks. Setting it on the response would again require a framework-specific implementation (e.g. a listener for Symfony). |
src/ProxyClient/LiteSpeed.php
Outdated
{ | ||
$filename = 'fos_cache_litespeed_purger_'; | ||
|
||
if (function_exists('random_bytes')) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dbu we're not talking crypto-stuff here and we're deleting the file right away so probably this part here is way to defensively coded 😄 Maybe a simple sha1(mt_rand())
is perfectly good enough?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, we only care about avoiding collisions. even if somebody could guess that url, nothing too bad happens.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay done in 8e627ab.
hi @litespeedtech Lauren , thanks for looking at this. is WebADC the commercial licensed version of LiteSpeed? or is it a pure reverse proxy without the capability to run web applications itself, but follows the same instructions as LiteSpeed? and what do you think of the discussion about hooking on the actual reply to the request for invalidation header, when possible? |
It would be better of course. It's faster. But then we need to have access to the response somehow and this is something completely new we haven't used anywhere else before. |
@dbu WebADC is a different product than LSWS, it's a Layer 4/7 load balancer with LSCache. That's for our client who needs a cluster solution, they will need to use WebADC, as caching needs to be at the first layer. |
src/ProxyClient/LiteSpeed.php
Outdated
|
||
$this->queueRequest('GET', $url, []); | ||
|
||
$result = parent::flush(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
flush might throw an ExceptionCollection. deletion should be in a finally block
…nto target_dir and document_root
Failing test is unrelated, maybe you need to retrigger the build. I did not update the docs yet as I want to have your feedback on the implementation first 😄 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i looked into this and think we are close. apart from nitpicks, i think we need to check about using the correct host in requests to invalidate with the more than one hostname scenario.
any chance that you could add a functional test that installs litespeed on travis-ci? i know its quite an effort, but would make it certain that things actually work and keep working after refactorings, which would be a huge plus.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll have a look at functional tests but I cannot promise.
ae8e37c
to
ebd0600
Compare
qsCache 1 | ||
|
||
# Disable checking for a cached entry if there's a cookie on the request | ||
reqCookieCache 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we don't support user context caching with litespeed, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right. User context caching requires some kind of preflight request to the app to find out the context which requires implementation in the proxy.
i notice that this seems mostly done. any chance we can wrap it up and merge it? |
It just works with OLS. Apparently there's no interest to integrate the same logic in LSWS which makes absolutely no sense to me. Looks like LiteSpeed has two different products which do not share any of the code base, not even the functional tests :( |
strange. well, lets wrap it up for OLS and add a note that LSWS behaves different and would need more work? |
Yeah, I'll add the docs and write that LSWS currently does not support tag invalidation. |
I really doubt that I want to finish this PR at this point. First of all, I set out to do this PR because I had customers running behind LSWS and as there seems to be no effort at all in implementing cache tag purging there, there's no benefit for me or my customers. So if I was to finish this, it would be pure goodwill because I cannot use it myself anyway. And looking at the failing unit tests after updating to the latest stable OLS release, there are random failures again. So I feel like I am the one that is functional testing OLS and that's definitely not the purpose of this PR. I mean I could of course dig into why exactly OLS doesn't behave like it should anymore but then again, I don't use it myself...why would I? |
pitty, but fair enough. totally understand you. i suggest we keep this around for a while still, in case somebody wants to continue working on it. i updated the description and marked the PR as needing a new contributor. |
i close this as it is pretty old. if somebody wants to pick things up, please feel free to do so, i am not against having a proxy client for litespeed in here. note that with all the refactorings that happened since this was started, the proxy client might has to look different than in this PR. |
WIP for Open Source Litespeed (OLS) and possibly the commercial Litespeed Web Server (LSWS).
Cache tagging is only implemented in OLS but not at all in LSWS. The changes worked with OLS but the latest OLS version seems to have done BC breaks / regressions with cache tagging.
The original author does not continue on this - if you need this, you will need to wrap up this merge request or pay someone to do it...
This is a possible implementation for LSWS (https://www.litespeedtech.com/products/litespeed-web-server) support.