[07:56:29] 10netops, 10Operations, 10Patch-For-Review: Find a new PIM RP IP - https://phabricator.wikimedia.org/T167842#3708969 (10Gehel) @ayounsi : wdqs should be clean of unwanted multicast. [08:14:31] 10Traffic, 10Operations, 10Wikimedia-Logstash: Varnish does not vary elasticsearch query by request body - https://phabricator.wikimedia.org/T174960#3708988 (10ema) >>! In T174960#3705072, @EBernhardson wrote: > Actually on closer review, kibana is allowing some POST requests to a limited set of endpoints, b... [09:48:05] mmh, I'm trying to handle the case where CL is unset: [09:48:06] const char *clen_hdr = VRT_GetHdr(ctx, &hdr); [09:48:31] if (clen_hdr) { clen = atoi(clen_hdr); } else { clen = 0; } [09:48:48] the thing does work, tested with vtc [09:49:26] however! the next request cannot be read by the mocked server, as if varnish ended up in a weird limbo by handling the 'unset cl' case [10:14:50] https://phabricator.wikimedia.org/P6177 [10:15:35] the 1st, 2nd and 3rd requests performed by c1 go through properly and all assertions are True [10:16:49] the 4th request, however, never seem to be reaching s1, as if v1 would not do anything with it (neither miss and send it to s1, nor hit and respond to c1) [10:18:39] the C code I'm currently testing is https://phabricator.wikimedia.org/P6178 [11:33:58] not sure if already posted: https://blog.cloudflare.com/the-sad-state-of-linux-socket-balancing/ [11:40:56] elukey: not already posted here, but I did see it the other day [11:41:09] we currently use the reuseport model, so we're on that side of the tradeoff at present [11:41:49] (evenly-balanced workers, probably better average/median latency, but probably worse max latency and deviation) [11:46:59] putting it all together: https://phabricator.wikimedia.org/P6179 [11:47:31] that test there does not use our custom VCL and still the "blocking" behavior described above is reproducible [11:49:11] ema: does it still block if you ditch all the custom C code? maybe it's just that the test scenario is invalid (I'm sure varnish allows to construct invalid ones, e.g. after the 3rd requests the connection is left in an invalid state because it's not Conn:close + no clen + no chunking or whatever) [11:51:23] IIRC even in HTTP/1.1, there's some ugly corner-cases around those things [11:51:40] ha! [11:51:50] bblack: it blocks without the custom C stuff too [11:51:59] if there's no chunklens being sent due to chunked transfer, and no content-length is sent either, the response is "connection delimited", meaning end-of-content is detected by connection closing. [11:52:15] (which is of course, a stupid mode because then an early disconnect delivers truncated response to the user as if it were legit) [11:52:48] so maybe the test sequence ends up in a connection-delimited state but then never closes the connection to start fresh for req#4 [11:55:29] bblack: \o/ [11:55:38] fixed with: [11:55:38] txresp -nolen -hdr "Transfer-Encoding: chunked" [11:55:39] chunkedlen 12 [11:55:39] chunkedlen 0 [11:56:41] thanks!! [11:57:46] yeah I think that's the typical case we get with MW (and others) anyways when we have no CL header [11:57:52] note that interestingly "Connection: close" doesn't fix it [11:57:58] (a chunk response, as opposed to connection-delimited) [11:58:32] so, next up for corner cases, is considering super-long CLs [11:58:52] atoi() is implicitly limited to +/-2GB values [11:59:33] you probably want strtoul() [11:59:51] (and an unsigned long, which will work fine since we're on x86-64) [12:01:41] (which has a different error behavior, you probably want to clear errno before calling, and set to zero if errno is set) [12:25:21] bblack: exp(-clen/adm_param) returns inf if clen is unsigned [12:26:00] it does the right thing like this tho: [12:26:01] const int adm_param = 32736; [12:26:01] const long int clen = strtoul("65536", NULL, 0); [12:26:02] printf("%g\n", exp(-clen/adm_param)); [12:39:29] yeah there's probably something subtle going on there with C type promotion [12:45:26] oh, duh, yeah there is [12:46:15] because "-clen" is going to turn it into a huge value in unsigned first [12:46:37] the math should all be done in double rather than waiting for the printf or rvalue assignment anyways [12:48:02] so maybe change the type of "adm_param" to double from the get-go, leave clen as unsigned long, and write the expression as: admissionprob = exp(-(double)clen/adm_param); [12:48:11] awesome [12:48:28] next challenge is to return CL:3G in vtc without explosions [12:48:34] or if you'd rather not have the typecast, you could do something like "double clen_d = clen;" and then use that in the exp() [12:49:53] or maybe a better style choice would be something like: [12:50:51] const unsigned long clen = strtoul(); const double clen_neg = -1.0 * clen; [12:51:06] I'd have to google or just test it myself to know, I don't remember all the type promotion rules around this offhand [12:51:36] a more explicit/safe variant might be: const unsigned long clen = strtoul(); const double clen_neg = -1.0 * (double)clen; [12:51:47] admissionprob = exp(clen_neg/adm_param); [12:55:57] yup, exp(clen_neg/adm_param) does the trick :) [13:00:27] CR updated [13:13:48] 10Traffic, 10netops, 10Operations, 10Pybal, 10Patch-For-Review: Frequent RST returned by appservers to LVS hosts - https://phabricator.wikimedia.org/T163674#3709732 (10elukey) Addendum to the summary: the rst does not happen in case a HTTP connection explicitly asks to be kept alive, since nginx does not... [14:02:40] 10Traffic, 10Operations, 10Wikimedia-Logstash: Varnish does not vary elasticsearch query by request body - https://phabricator.wikimedia.org/T174960#3709805 (10dbarratt) I tried a `POST` request to the `_msearch` endpoint, but got this response: ``` { "statusCode": 400, "error": "Bad Request", "message":... [14:05:51] 10Traffic, 10netops, 10Operations, 10Pybal, 10Patch-For-Review: Frequent RST returned by appservers to LVS hosts - https://phabricator.wikimedia.org/T163674#3709808 (10BBlack) > would it mean that nginx should keep more TCP connections opened hoping for the client to eventually send a close notify (in re... [14:08:29] 10Traffic, 10Operations, 10Wikimedia-Logstash: Varnish does not vary elasticsearch query by request body - https://phabricator.wikimedia.org/T174960#3578041 (10dcausse) Yes the syntax is slightly different: - you need to set `Content-Type: application/x-ndjson` - every request must be formed of 2 lines: -- f... [14:24:10] 10Traffic, 10Operations, 10Wikimedia-Logstash: Varnish does not vary elasticsearch query by request body - https://phabricator.wikimedia.org/T174960#3709846 (10dbarratt) >>! In T174960#3709823, @dcausse wrote: > -- first line some metadata such as the index you want to query What is our index? errr.. what i... [14:27:35] bblack: so yes! We should get rid of the debugging headers altogether [14:27:50] I don't seem to be able to find a decent way to log to VSL from inline C [14:28:01] this works: [14:28:02] VRT_acl_log(ctx, "Admission Probability: BANANA"); [14:28:32] but yeah, VTR_acl_log is the only VRT function I've found that does log to VSL [14:29:27] of course we could set the headers in C, log with std.log outside of C and then remove them [14:29:43] but it would be better to just log from within C without all the overhead [14:30:43] then it's pretty easy to run assertions in VTC with logexpect to see if the probabilities being logged match what we expect [14:30:47] well, we still need the debug headers for the vcl-testing case right, because it looks at the emitted probability header? [14:30:59] ah, that answers that heh :) [14:31:03] yep! [14:32:11] so, you can look at the log() function in libvmod_std [14:32:14] we've got multiple options here, we could also as you said set the headers only in test-mode, but perhaps it could be interesting to get the admission prob. in varnishlog in general? [14:33:03] yes, I think so [14:33:17] if we can use the log entries for VTC, no need for headers at all [14:33:54] maybe instead of duplicating the crazy C of libvmod_std's log(), we can invoke it? [14:34:04] IIRC there's a way to invoke vmod functions from inline C [14:34:13] my kingdom to the first man who finds out how! [14:34:55] we've done it before in legacy VCL code, I think for the vmod_headers functions for Set-Cookie and such? [14:35:24] // Use libvmod-header to ensure the Set-Cookie header we are adding [14:35:27] // does not clobber or manipulate existing cookie headers (if any). [14:35:30] const struct gethdr_s hdr_set_cookie = { HDR_RESP, "\013Set-Cookie:" }; [14:35:33] Vmod_header_Func.append(ctx, &hdr_set_cookie, [14:35:35] out, "; Path=/; secure; Domain=.", [14:35:37] so maybe Vmod_std_Func.log(...) ? [14:35:40] host_safe, vrt_magic_string_end); [14:35:42] ^ from geoip.inc.vcl [14:38:01] I think if you want to be more C-style than Varnish-style, you can even explicitly use the fmt/args stuff [14:38:24] Vmod_std_Func.log("Foo: %g", admissionprob); or whatever? [14:39:17] oh context too: Vmod_std_Func.log(ctx, "Foo: %g", admissionprob); [14:39:29] yeah context too, I get a runtime explosion though [14:39:35] Vmod_std_Func.log(ctx, "Admission Probability: BANANA"); [14:40:04] [...] [14:40:05] *** v1 1.0 debug| 0x44374b: varnishd(VRT_StringList+0x5b) [0x44374b]\n [14:40:08] *** v1 1.0 debug| 0x7f4c4e3e2745: libvmod_std.so(vmod_log+0xd5) [0x7f4c4e3e2745]\n [14:40:11] *** v1 1.0 debug| 0x7f4c531e6c42: vgc.so(VGC_function_vcl_backend_response+0x2f2) [0x7f4c531e6c42]\n [14:41:28] 10Traffic, 10Operations, 10Wikimedia-Logstash: Varnish does not vary elasticsearch query by request body - https://phabricator.wikimedia.org/T174960#3709879 (10dcausse) @dbarratt sadly I don't know all the details of this cluster, but you could get it working by not specifying an index: with a `requests` f... [14:41:41] so the logical possibilities are probably: (1) adding an actual format argument would help (unlikely) or (2) it actually has to be in VRT_string type of stuff [14:43:11] ah so, it's just Vmod_std_Func.log() arguments are confusing from the C perspective [14:43:22] it's actually expecting a list of strings ending with magic [14:43:28] grr [14:43:37] so I need vrt_magic_string_end perhaps? [14:43:53] and you can't do simple sprintf()-style formatting, either [14:44:07] you'll have to convert the probability to a varnish (or C, I guess) string [14:44:32] it always compiles fantastically well though :) [14:44:35] for BANANA adding vrt_magic_string_end should work :) [14:45:05] hey, it worked with print-style! [14:45:10] Vmod_std_Func.log(ctx, "Admission Probability: %g", admissionprob, vrt_magic_string_end); [14:45:10] oh? [14:45:34] yeah lol [14:45:37] **** v1 1.1 vsl| 1002 VCL_Log b Admission Probability: %g [14:45:45] haha [14:46:05] VRT_StringList assumes all arguments are strings and just concats them [14:46:21] the misleading "fmt, ..." is just to let you have a variable-length list of strings [14:46:46] for the real thing it will be like .log(ctx, "Admission Probability: ", admprob_converted_to_string, vrt_magic_string_end) [14:46:55] with VRT_REAL_string perhaps [14:47:01] yeah [14:47:13] which in turn uses WS_Printf [14:47:30] may as well use WS_Printf directly and put the prefix in there? [14:48:06] .log(ctx, WS_Printf(ctx->ws, "foo: %g", some_double), vrt_magic_string_end) [14:48:44] \o/ [14:48:45] **** v1 1.1 vsl| 1002 VCL_Log b Admission Probability: 1.000 [14:49:01] like this: [14:49:02] Vmod_std_Func.log(ctx, "Admission Probability: ", VRT_REAL_string(ctx, admissionprob), vrt_magic_string_end); [14:49:11] let's try with WS_Printf instead [14:49:20] yeah I don't know if WS_Printf() is even exported for inline-C usage [14:50:29] it's not [14:51:25] let's use VRT_REAL_string then? [14:52:30] yeah [14:55:24] so, separately-but-relatedly, we should really investigate the whole "if (!next_is_cache && !CL) { do_stream = false }" [14:56:03] I know you did some VTC tests before. I wonder if it's really transitive though (if we stream/chunk through the rest of the layers, does the user-facing side at the edge get a CL header?) [14:56:22] or if varnish ends up undoing the work when it does its own gzipping or something [14:57:00] although if our gzip behaviors are layer-consistent, only the backend-most should ever be gzipping in varnish [14:58:07] and we are consistnt, in the layer+cluster-common VCL as: [14:58:07] // Compress compressible things if the backend didn't already [14:58:07] if (beresp.http.content-type ~ "json|text|html|script|xml|icon|ms-fontobject|ms-opentype|x-font") { [14:58:10] set beresp.do_gzip = true; [14:58:26] 10Traffic, 10Operations, 10Wikimedia-Logstash: Varnish does not vary elasticsearch query by request body - https://phabricator.wikimedia.org/T174960#3709944 (10dbarratt) @dcausse so that does work, so @ema this is a valid work around, although, imho, it's not elegant. I've [[ https://wikitech.wikimedia.org/... [15:05:30] bblack: so in vtc you can define multiple varnishes talking to each other, it should be easy to check if the client ends up with CL [15:05:35] since cache_misc already has "if (!next_is_cache && !CL) { do_stream = false }", we could even test these hypotheses live there, if we have some backends we know don't provide CL, and may have a couple different content types for gzip-vs-not [15:05:57] or that [15:06:06] the VTC should test the compressible case too with do_gzip [15:35:52] 10netops, 10Operations, 10Patch-For-Review: Find a new PIM RP IP - https://phabricator.wikimedia.org/T167842#3710211 (10ayounsi) Indeed, confirmed. Thanks! [15:56:45] 16:30 < ema> then it's pretty easy to run assertions in VTC with logexpect to see if the probabilities being logged match what we expect [15:56:50] bullshit ^ [15:57:58] lol [15:58:17] <%- if @varnish_testing -%> FTW [15:58:18] well, we can always do an ERB if-condition around setting debug headers for the VTC only in the vtc-testing case [15:58:21] yeah [16:17:12] bblack: oh, I forgot! should we bump timeout_idle on text/upload soonish ? I can do that tomorrow if you agree https://gerrit.wikimedia.org/r/#/c/385985/ [16:22:37] yeah +1 [16:22:46] kind of an open question whether it's ok on the fe too [16:23:03] (probably, since nginx is fronting those connections, except for the port 80 case, where we have had recent sporadic DoS....) [16:24:35] right, but those were spoofed SYNs, no 3WHS completion, so I imagine timeout_idle wouldn't have applied in that case? [16:25:08] probably not [16:25:22] but it might make slowloris-type attacks on port 80 worse [16:26:24] yup [16:26:46] gotta go, see you! [16:26:50] cya! [18:02:54] 10Traffic, 10Operations: Migrate to nginx-light - https://phabricator.wikimedia.org/T164456#3710675 (10BBlack) Seems the bot missed logging this here: https://gerrit.wikimedia.org/r/#/c/386424/ [18:23:27] 10Traffic, 10Operations: LVS hosts should have static-mapped IPv6 on all virtual interfaces - https://phabricator.wikimedia.org/T179025#3710743 (10BBlack) [18:24:24] 10Traffic, 10Operations: LVS IPv6 IPs should all be recorded in DNS - https://phabricator.wikimedia.org/T179026#3710760 (10BBlack) [18:24:33] 10Traffic, 10Operations: LVS hosts should have static-mapped IPv6 on all virtual interfaces - https://phabricator.wikimedia.org/T179025#3710774 (10BBlack) [18:24:36] 10Traffic, 10Operations: LVS IPv6 IPs should all be recorded in DNS - https://phabricator.wikimedia.org/T179026#3710773 (10BBlack) [18:26:41] 10Traffic, 10Operations: Puppetize LVS interface IP sets per-DC for easy use in ferm rules - https://phabricator.wikimedia.org/T179027#3710778 (10BBlack) [18:29:10] 10Traffic, 10Operations: Puppetize LVS interface IP sets per-DC for easy use in ferm rules - https://phabricator.wikimedia.org/T179027#3710800 (10Ottomata) [18:47:29] 10Traffic, 10netops, 10Operations, 10Pybal, 10Patch-For-Review: Frequent RST returned by appservers to LVS hosts - https://phabricator.wikimedia.org/T163674#3710945 (10BBlack) patch above released with `nginx-1.13.6-2+wmf1`, so we're capable of experimentation now. Flag isn't turned on anywhere yet. [21:24:18] 10Traffic, 10Operations, 10ops-ulsfo: cp4024 kernel errors - https://phabricator.wikimedia.org/T174891#3711338 (10BBlack) 05Resolved>03Open `cp4024` died randomly today. I've left it alone other than to connect to the console and verify no response there. `21:07 < icinga-wm> PROBLEM - Host cp4024 is DOW... [21:40:33] 10netops, 10Operations, 10ops-eqiad: Setup eqsin atlas anchor - https://phabricator.wikimedia.org/T179042#3711364 (10ayounsi) [23:15:37] 10Traffic, 10Operations, 10ops-ulsfo: decom cp40(09|1[078]) - https://phabricator.wikimedia.org/T178815#3711530 (10RobH) a:05BBlack>03RobH