POC/VIP26: limit Transient usage for private objects

[ehocdet]

It's an old attempt to limit Transient usage for private objects (named "soft pipe" initially). It looks like this attempt is more stable since #2963 has been fixed. I guess my approach is a little too hackish but it can be a start for https://github.com/varnishcache/varnish-cache/wiki/VIP-26:-limit-private-prefetch.

[nigoroll]

Your efforts are appreciated, thank you!

@ehocdet, do you intend this as a poc to give the issue some traction or do you actually want to work towards code fit for integration? I am asking before spending time on a comprehensive review...

[ehocdet]

@nigoroll, i can work on this code, but the main functionality "limit private prefetch" greatly differ from VIP26: the stevedore gets notified by the requester as soon as it consumes a buffer. So, i think it will just a poc.

[dridi]

@ehocdet would you like to test #3480? It's another take on VIP26 without an additional lock and condvar.

[ehocdet]

@Dridi thanks you for the update. #3480 have 2 issues : CondWait on slow clients is not good, lack of multi-buffering will slow down stream. It's why I used additional lock and condvar on my poc.

[mbgrydeland]

While working on #3480, I had a quick look at this. I believe there are cases where this patch set as is fails and causes deadlocks. Specifically, it assumes that there will always be a streaming client in order to update the fetch thread and not cause that to wait indefinitely.

The following test case from #3480, slightly adapted, demonstrates one mode in which this can happen:

varnishtest "Pass buffering deadlock test"

server s1 {
	rxreq
	txresp -status 404
} -start

server s2 {
	non_fatal
	rxreq
	txresp -bodylen 2000000
} -start

varnish v1 -vcl+backend {
	import vtc;
	sub vcl_recv {
		set req.backend_hint = s2;
		if (req.restarts == 1) {
			set req.backend_hint = s1;
		}
		return (pass);
	}
	sub vcl_deliver {
		if (req.restarts < 1) {
			vtc.sleep(0.5s);
			return (restart);
		}
	}
	sub vcl_backend_response {
	}
} -start

varnish v1 -cliok "param.set debug +syncvsl"

client c1 {
	txreq
	rxresp
	expect resp.bodylen == 0
	expect resp.status == 404
} -run

varnish v1 -expect VBE.vcl1.s1.conn == 0
varnish v1 -expect VBE.vcl1.s1.busy == 0
varnish v1 -expect VBE.vcl1.s2.conn == 0
varnish v1 -expect VBE.vcl1.s2.busy == 0

You also write that you have some reservations about the approach made in #3480. I think those are based on missunderstandings of the way this feature is implemented there. I added a comment there attempting to explain why. Please have a look and we can continue the discussion there on that ticket.

Would you be able to have the implementation in #3480 a go in your setups? Some real world testing of this would be much appreciated.

[bsdphk]

See #3572