Hi Patrick, On Thu, Aug 09, 2018 at 06:29:33PM -0400, Patrick Hemmer wrote: > I also went and removed the queue position counter code from > stream_process_counters(), and the logging still appears to work fine > (but I could have easily missed some potential scenarios).
OK nice! > In regards to the idea you added to the commit message on the queue index > > It could even be further improved to avoid manipulating the stream > > from the queue : > > - queue_idx = initial position when enqueuing > > - queue_idx = measured position when dequeuing > > Because the stream can pass through multiple queues, we'd have to make > sure that every time we de-queue, that the stream code pulls the value > and increments itself. However looking at all the various places > pendconn_unlink() gets called, I think it would be difficult for the > stream code to know when the value needs to be pulled. In fact the only two ways to be queued multiple times are : - get redispatched after a failed connection attempt to a server ; in this case the pendconn was already dequeued to connect to the first server so the state is unambigous - being redistributed when a pendconn was lying in a server's queue and the server is marked down. In this case the pendconn is simply unlinked, process_stream is woken up and I don't know if we collect queue_idx during the call to process_stream before queuing again. Anyway this is very minor, I'd even say cosmetic. I'm pretty fine with the current state already! > Anyway, attached is the latest patch set for review again. I've applied it to a local temporary branch and am willing to merge it if you're OK. I've only adjusted 3 minor things : - removed the warning in the 3rd patch's commit message mentioning the missing calls to pendconn_unlink() before logs, because these ones were finally moved to patch 1 but we didn't edit this message ; - removed the now unused queue_idx from stream_process_counters() which emits a warning since it's unused - removed the "__decl_hathreads(HA_SPINLOCK_T lock);" from struct pendconn that is a leftover from the rebase onto the last queue updates. So please just let me know, I'm ready ;-) Thanks, Willy