On Tue, May 30, 2023 at 6:46 PM Mark Thomas <ma...@apache.org> wrote: > > On 30/05/2023 15:10, Rémy Maucherat wrote: > > > Ok. I proposed the removal based on "It is intended to be used with > > non-sticky load-balancers" from the javadoc. I think the clustering > > and stickness have to be used instead of this valve, to comply with > > all reasonable concurrency requirements from the container and > > application in that use case. Basically the BZ can be fixed only if > > there's one Tomcat. I understand this valve is in use since we got the > > BZ, but given everything I don't understand how it is useful, I > > considered users should either stop using it or move to session > > clustering. > > Ah. Now I see why you were suggesting deprecation. > > Non-sticky LB implies more than one Tomcat instance. > > The requirement for no more than one concurrent request per session is > something we can enforce on a single Tomcat node (with something like PR > #623) but we can't enforce it across multiple nodes. > > Given we can't enforce it across multiple nodes, there is no point > trying to enforce it on a single node. > > Similarly, PersistentManager isn't going to work well with non-sticky > load balancers. We should document that. > > The question becomes, is there a use case for the following: > > - multiple nodes > - non-sticky LB > - PersistentManager > - client(s) guarantee of no more than one concurrent request per session > > The last point can't be enforced by Tomcat and stuff will break if a > client breaks the guarantee. > > If there is a use case, we should keep PersistentValve, document the > requirements even more clearly that they are already. > > If there is no use case, we should remove PersistentValve from Tomcat 11 > and deprecate it in earlier versions as per your suggestion. > > Either way, BZ 66513 gets resolved as WONTFIX (more like can't fix).
Your fix is better than what I thought was possible, since I only considered a map of lock objects (and good cleanup was a problem). I think your patch should be merged. > I can't think of a use case that justifies keeping PersistentValve. BZ > 66513 talks about non-sticky LB with containers. I'll ask some follow-up > questions to see if there is something we are missing but my sense is > that this was always broken but the users using it either never observed > breakage or it happened rarely enough it wasn't followed up. +1 It sounds reasonable to redocument stating the limitations better, deprecate, then remove in 11. Rémy --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
