On Thu, Sep 22, 2016 at 1:23 AM, Robert Haas <robertmh...@gmail.com> wrote: > On Tue, Sep 20, 2016 at 7:13 PM, Thomas Munro > <thomas.mu...@enterprisedb.com> wrote: >> This paragraph seems a bit confused. I suggest something more like >> this: "The server process is waiting for one or more sockets, a timer >> or an interprocess latch. The wait happens in a WaitEventSet, >> <productname>PostgreSQL</>'s portable IO multiplexing abstraction." > > I'm worried we're exposing an awful lot of internal detail here.
Ok, maybe it's not a good idea to mention WaitEventSet in the manual. I was trying to keep the same basic structure of text as Michael proposed, but fix the bit that implied that waiting happens "in a latch", even when no latch is involved. Perhaps only the first sentence is necessary. This will all become much clearer if there is a follow-up patch that shows strings like "Socket", "Socket|Latch", "Latch" instead of a general catch-all wait_event_type of "EventSet", as discussed. > Moreover, it's pretty confusing that we have this general concept of > wait events in pg_stat_activity, and then here the specific type of > wait event we're waiting for is the ... wait event kind. Uh, what? Yeah, that is confusing. It comes about because of the coincidence that pg_stat_activity finished up with a wait_event column, and our IO multiplexing abstraction finished up with the name WaitEventSet. <stuck-record-mode>We could instead call these new things "wait points", because, well, erm, they name points in the code at which we wait. They don't name events (they just happen to use the WaitEventSet mechanism, which itself does involve events: but those events are things like "hey, this socket is now ready for writing").</> > I have to admit that I like the individual event names quite a bit, > and I think the detail will be useful to users. But I wonder if > there's a better way to describe the class of events that we're > talking about that's not so dependent on internal data structures. > Maybe we could divide these waits into a couple of categories - e.g. > "Socket", "Timeout", "Process" - and then divide these detailed wait > events among those classes. Well we already discussed a couple of different ways to get "Socket", "Latch", "Socket|Latch", ... or something like that into the wait_event_type column or new columns. Wouldn't that be better, and automatically fall out of the code rather than needing human curated categories? Although Michael suggested that that should be done as a separate patch on top of the basic feature. > The "SecureRead" and "SecureWrite" wait events are going to be > confusing, because the connection isn't necessarily secure. I think > those should be called "ClientRead" and "ClientWrite". > Comprehensibility is more important than absolute consistency with the > C function names. Devil's advocate mode: Then why not improve those function names? Keeping the wait point names systematically in sync with the actual code makes things super simple and avoids a whole decision process and discussion to create new user-friendly obfuscation every time anyone introduces a new wait point. This is fundamentally an introspection mechanism allowing expert users to shine a light on the engine and see what's going on inside it, so I don't see what's wrong with being straight up about what is actually going on and using the names for parts of our code that we already have. If that leads us to improve some function names I'm not sure why that's a bad thing. Obviously this gets complicated by the existence of static functions whose names are ambiguous and lack context, and multiple wait points in a single function. Previously I've suggested a hierarchical arrangement for these names which might help with that. Imagine names like: executor.Hash.<function> (reported by a background worker executing a hypothetical parallel hash join), executor.Hash.<function>.<something> to disambiguate multiple wait points in one function, walsender.<function> etc. That way we could have a tidy curated meaningful naming scheme based on modules, but a no-brainer systematic way to name the most numerous leaf bits of that hierarchy. Just an idea... > Another thing to think about is that there's no way to actually see > wait event information for a number of the processes which this patch > instruments, because they don't appear in pg_stat_activity. Good point. Perhaps there could be another extended view, or system process view, or some other mechanism. That could be material for a separate patch? -- Thomas Munro http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
