varnishcache / varnish-cache

Varnish Cache source code repository
https://www.varnish-cache.org
Other
3.65k stars 374 forks source link

v1d: Log client write error #4042

Open dridi opened 8 months ago

dridi commented 8 months ago

Some scenarios like hitting send_timeout because of a dripping write leaves no clue that an error occurred from the client request logs alone.

This introduces a client counterpart for:

FetchError backend write error: %d (%s) (%s)

Refs 49e44e390488c57c03d8111fcd43e245933bc151

dridi commented 8 months ago

Bugwash:

nigoroll commented 8 months ago

bugwash:

dridi commented 8 months ago

SLT_Error is not a particularly good tag for this, because it is currently used for internal errors which can usually be addressed with VCL or changing parameters

nitpick: this can be addressed with send_timeout for that case, but point taken, there is nothing we can do when the client disconnects.

dridi commented 8 months ago

I'd like to cover this during bugwash today, before we send this to production.

daghf commented 8 months ago

bionic failure,

cache/cache_expire.c: In function 'exp_inbox':
cache/cache_expire.c:282:61: error: format '%ld' expects argument of type 'long int', but argument 6 has type 'VCL_INT {aka long long int}' [-Werror=format=]
   VSLb(&ep->vsl, SLT_ExpKill, "EXP_Removed x=%ju t=%.0f h=%ld",
                                                           ~~^
                                                           %lld
cache/cache_expire.c:284:7:
       oc->hits);
       ~~~~~~~~                                               
cache/cache_expire.c: In function 'exp_expire':
cache/cache_expire.c:360:61: error: format '%ld' expects argument of type 'long int', but argument 6 has type 'VCL_INT {aka long long int}' [-Werror=format=]
   VSLb(&ep->vsl, SLT_ExpKill, "EXP_Expired x=%ju t=%.0f h=%ld",
                                                           ~~^
                                                           %lld
cache/cache_expire.c:362:7:
       oc->hits);
       ~~~~~~~~                                               
  CC       cache/varnishd-cache_fetch_proc.o
cache/cache_expire.c: At top level:
cc1: error: unrecognized command line option '-Wno-nullability-completeness' [-Werror]
cc1: error: unrecognized command line option '-Wno-missing-variable-declarations' [-Werror]
cc1: error: unrecognized command line option '-Wno-system-headers-Wno-thread-safety' [-Werror]
cc1: all warnings being treated as errors
bsdphk commented 7 months ago

goes after march-15 release

nigoroll commented 7 months ago

during bugwash, @bsdphk brought up the question if this could affect vsl clients, and while it does not look like it, git grep suggests that we should change some other places where FetchError is referred.

bsdphk commented 7 months ago

after march15 release