This adds VTCP_Assert() on the result of read and write calls that deals
with TCP sockets.

 Conflicts:
	bin/varnishd/proxy/cache_proxy_proto.c

When used to check the result of read() and write() calls, it is useful
that a positive return value is accepted in VTCP_Check().

Now that VTCP_Assert() accepts EAGAIN as a legal errno value for read()
errors, uncomment this check.

Since the input value is sometimes the result of a read()/write() call,
avoid truncating the ssize_t value on calling it.

When a socket timeout is set on a socket and the timeout expires, read()
and write() calls on that socket will return (-1) with errno set to
EAGAIN/EWOULDBLOCK.

 Conflicts:
	lib/libvarnish/vtcp.c

Consistently use VTCP_Assert when asserting on the result of VTCP_Check().

Once HSH_Unbusy() has been called there is a possibility for
EXP_Remove() to be called before the fetch thread has had a chance to call
EXP_Insert(). By adding a OC_EF_NEW flag on the objects during
HSH_Unbusy(), that is removed again during EXP_Insert(), we can keep track
and clean up once EXP_Insert() is called by the inserting thread if
EXP_Remove() was called in the mean time.

This patch also removes the AZ(OC_F_DYING) in EXP_Insert(), as that is no
longer a requirement.

Fixes: #2999

This makes the order of events the same as on real cache insertions.

The OC_EF_REFD flag indicates whether expiry has a ref on the
OC. Previously, the flag was only gained during the call to
EXP_Insert. With this patch, and the helper function EXP_RefNewObjcore(),
the flag is gained while holding the objhead mutex during
HSH_Unbusy(). This enables the expiry functions to test on missing
OC_EF_REFD and quickly return without having to take the main expiry
mutex.

 Conflicts:
	bin/varnishd/cache/cache_varnishd.h

When posting to the expiry thread, we wrongly counted exp_mailed also if
the OC in question was already on the mail queue. This could cause a
discrepency between the exp_mailed and exp_received counters.

This enables doing extra handling while holding the mutex specific to
EXP_Insert/EXP_Remove before/after calling exp_mail_it.

background: When the ban lurker has finished working the bottom of the
ban list, conceptually we mark all bans it has evaluated as completed
and then remove the tail of the ban list which has no references any
more.

Yet, for efficiency, we first remove the tail and then mark only those
bans completed, which we did not remove. Doing so depends on knowing
where in the (obans) list of bans to be completed is the new tail of
the bans list after pruning.

5dd54f83 was intended to solve this,
but the fix was incomplete (and also unnecessarily complicated): For
example when a duplicate ban was issued, ban_lurker_test_ban() could
remove a ban from the obans list which later happens to become the new
ban tail.

We now - hopefully - solve the problem for real by properly cleaning
the obans list when we prune the ban list.

Fixes #3006
Fixes #2779
Fixes #2556 for real (5dd54f83 was
incomplete)

 Conflicts:
	bin/varnishd/cache/cache_ban_lurker.c

The watchdog mechanism currently triggers when any queueing is happening,
regardless of the priority. Strictly speaking it is only the backend
fetches that are critical to get executed, and this prevents the thread
limits to be used as limits on the amount of work the Varnish instance
should handle.

This can be especially important for instances with H/2 enabled, as these
connections will be holding threads for extended periods of time, possibly
triggering the watchdog in benign situations.

This patch limits the watchdog to only trigger for no queue development
on the highest priority queue.

When accepting new incoming connections in the acceptor thread, it would
schedule, they would be registered with the VCA priority. This priority is
reserved for the acceptor thread itself, and specifically is not included
in the TASK_QUEUE_CLIENT categorisation. This would interfere with the
thread reserve pools.

t02011.vtc had to be adjusted to account for the new priority
categorisation of the initial request.

This test is to detect a deadlock which does not exist any more. IMHO,
the only sensible way to test for the lack of it now is to do a load
test, which is not what we want in vtc.

... introduced with 3bb8b84c:

in Pool_Work_Thread(), we could break out of the for (i = 0; i <
TASK_QUEUE__END; i++) loop with tp set to the value from the previous
iteration of the top while() loop where if should have been NULL (for no
task found).

Noticed staring at #3192 - unclear yet if related

Previously, we used a minimum number of idle threads (the reserve) to
ensure that we do not assign all threads with client requests and no
threads left over for backend requests.

This was actually only a special case of the more general issue
exposed by h2: Lower priority tasks depend on higher priority tasks
(for h2, sessions need streams, which need requests, which may need
backend requests).

To solve this problem, we divide the reserve by the number of priority
classes and schedule lower priority tasks only if there are enough
idle threads to run higher priority tasks eventually.

This change does not guarantee any upper limit on the amount of time
it can take for a task to be scheduled (e.g. backend requests could be
blocking on arbitrarily long timeouts), so the thread pool watchdog is
still warranted. But this change should guarantee that we do make
progress eventually.

With the reserves, thread_pool_min needs to be no smaller than the
number of priority classes (TASK_QUEUE__END). Ideally, we should have
an even higher minimum (@Dridi rightly suggested to make it 2 *
TASK_QUEUE__END), but that would prevent the very useful test
t02011.vtc.

For now, the value of TASK_QUEUE__END (5) is hardcoded as such for the
parameter configuration and documentation because auto-generating it
would require include/macro dances which I consider over the top for
now. Instead, the respective places are marked and an assert is in
place to ensure we do not start a worker with too small a number of
workers. I dicided against checks in the manager to avoid include
pollution from the worker (cache.h) into the manager.

Fixes #2418 for real

 Conflicts:
	bin/varnishd/cache/cache_wrk.c
	bin/varnishd/mgt/mgt_pool.c

There's no point waiting for the backend to complain if we weren't able
to properly send the backend request.

Fixes #3556

 Conflicts:
	bin/varnishd/cache/cache_backend.c

The reason we expect here can be summarized as: this was a pipe
transaction or an error occurred. This could be much simpler if
we replaced enum sess_close with a struct stream_close instead.

Refs dc5bddbd

Also move the lock up to cover more operations.

Previously we would read the response Content-Length from a failed oc,
which would make the error response valid. Now, if this is detected,
we don't touch the Content-Length.

VRT_delete_backend() sets be->cooled to non-zero as the only place where
that is done. Assert that it is zero on entry as a check that
VRT_delete_backend isn't called multiple times.

We refuse to accept new dynamic backends while the VCL is cooling, and
drop adding the attempted backend on the VCL's backend list when that
condition is found. But that would cause an assert later when it is picked
off the cool_backends list for destruction. Fix this by directly
destroying the backend instead of going through the cooling list.

Note that this patch removes the ASSERT_CLI() macro in vbe_destroy().

Several functions (VBE_Poll and vbe_destroy) tests be->cooled == 0 to
determine which of the two lists backends and cool_backends a specific
instance currently lives on. If the flag is in the process of being
changed, then the wrong list head may be used and will result in strange
bugs.

 Conflicts:
	bin/varnishd/cache/cache_backend.c

The last three commits already made configure recommend installing
Python 3 packages and look for versioned executables, however with a low
priority.

This is a problem on systems such as OpenBSD 6.5 with a default Python
version at 2.7, where 3.7 flavored Python packages get installed with
a "-3" binary suffix.  That is, when both rst2man and rst2man-3 are
installed at configure time, the lower version will be picked unless
explicitly passed through `--with-feature' arguments.

Regardless of this specific case, trying more specificly versioned tool
names first seems correctly in line with recent development and less
error prone, so change it accordingly.

 Conflicts:
	configure.ac

For a given definition of "future" or "now".

Until the naked "python" executable refers to python3 (currently it is
still python2) it now takes lower precedence.

The assertion that the stale objcore of a conditional fetch cannot be
failed unless it was streaming is incorrect. Between the moment when
we grab the stale objcore in HSH_Lookup and the moment we try to use
it after vcl_backend_response, the backend fetch may have completed or
failed.

Instead, we need to treat an ongoing fetch and a failed fetch as
separate checks since the latter may happen with or without a boc.

 Conflicts:
	bin/varnishd/cache/cache_fetch.c

Test case by Reza, thank you

Fixes #3433
Closes #3434

Once we ask the backend to close the connection after a given request
there is no benefit from putting the backend connection back in the
pool. It's actually a surefire way to force a subsequent backend fetch
to fail its first attempt and go straight to its extra chance.

Since we try to maximize connection reuse this would have to come from
VCL and a user asking for the backend to close the connection should
have a good reason to do so, for example when the backend is known to
misbehave under certain circumstances.

Closes #3400
Refs #3405

Original author: @dridi

This commit is from ba78ebeb but
with no analogous file in 6.0, it is added to changes-6.0.rst instead of changes-trunk.rst

Whether the header was set by the backend or directly in VCL, it is now
possible to signal that a backend connection should not be added back to
the pool after a successful fetch with a Connection:close header.

Pooling such a connection would be counter-productive if closing the
session was requested by the backend itself, because it would then be
likely that reusing the connection would result in busting the extra
chance.

Setting the Connection:close directly in VCL can help mitigating against
a misbehaving backend.

Refs #3400