[RFC] [Discussion] Persistent CurlShareHandle objects

> Apologies Rob, I don't seem to be receiving individual emails from the > mailing list, so I'll have to respond to my own email here. I've > changed my subscription to receive all emails so hopefully I'll be > able to respond directly to future emails. > > > It might be a good idea to get a “strong opinion” on the signature of curl_share_init. > > I think you are right. I've chatted with Rasmus and we both agree that > changing the signature of curl_share_init is preferable to adding a > new function - it's more in line with how we do persistence elsewhere. > I'll update the RFC to remove the signature question. > > Were you also suggesting that I shouldn't ask a question about the > implementation regarding using EG(persistent_list) vs. a module > global? I'm torn; I agree that it is an implementation detail but I > was hoping to actually get wider input since it generated discussion > in my pull request. Since no one responded here I felt like it'd be a > lightweight way to get feedback on what people felt, but the lack of > response probably means that people don't feel strongly enough about > it. > > That said, after reflecting I think I would agree that it is > unnecessary to add this as a question, and I'll continue with the > implementation as-is in the PR. If people would like to send feedback > on using EG(persistent_list) instead, feel free to do so here or in > the PR. > > > Also, it might be worth saying the “next minor version of PHP” just in case there isn’t an 8.5 and goes to 9.0. > > Makes sense to me, I'll make this change. >

> really any major issues that this can cause. We already support curl > share handles via curl_share_init, and adding persistence doesn't > fundamentally change the issues you may have with reusing a curl

>> Persistent handlers don't really fit PHP's model, and are practically >> untestable from a PHPT test perspective as far as I know (at least I >> have never seen any proper tests for the existing ones, but I am happy >> to be corrected). > > Persistence is important to successfully scaling Etsy's monolithic PHP > application. As I mentioned in a response to Tim, we make use of > persistence for things like memcached connections in order to reduce > end-user latency, and we'd like to take that further by persisting the > connections of downstream curl requests we make.

> If we truly feel that persistence is wrong for PHP, we would need to > deprecate and remove things like persistent PDO connections; I would > expect that would be a perfectly fine time to also remove support for > persistent curl share handles, if they existed.

> > On Mon, Nov 4, 2024 at 2:21 PM Jakub Zelenka <bukka@php.net> wrote: >> >> Hi, >> >> On Wed, Oct 9, 2024 at 10:18 PM Eric Norris <eric.t.norris@gmail.com> wrote: >>> >>> Hello all, >>> >>> After receiving some feedback about >>> https://github.com/php/php-src/pull/15603, I'm formally proposing an >>> RFC to add persistent curl share handles here: >>> https://wiki.php.net/rfc/curl_share_persistence >>> >> >> I think what's really needed for this is to have some infrastructure for connection pooling. The current persistent connections in streams (which includes mysqlnd, redis and its other users) are quite primitive and not that well managed. This might occasionally result in incorrect clean up and possibly having too many connections open. >> >> What I think we need is a new internal API that would be handled by SAPI. So basically some generic API that SAPI would hook into and manage the connections. For extension it could work by registering the pool with some connect and clean up callbacks and various parameters (e.g. timeout and maybe something else). Then it would be just getting the connections from it. It should be quite generic so it can support also curl handles so the actual connection should be opaque. The point here is that SAPI should handle the pool and deal with timeouts / clean up. This could work pretty well for the threaded SAPI's. FPM is a bit limited as it's per worker but it could potentially set its own limits and could still better handle timeouts and clean ups. >> >> I think there should be not such a big oppositions to the general concept because this is already rooted in PHP in the form of persistent resources and this just tries to implement the same for objects which we might eventually need for streams if they ever get them converted to objects. There are likely many application successfully using persistent resources and it could be quite problematic to drop the whole concept. >> >> Basically what I want to say is that we have got chance to implement it properly for objects and we should not end up with the same sort of way how it's done currently for resources. The better way is to have SAPI managed API IMHO. >> > > Just for the reference I voted yes on RFC because that's just about the concept and user API which I think is fine and it's inline with persistence that we already have. But note that it doesn't mean that we will accept implementation in the current form.