On Tue, Jan 11, 2011 at 12:58, Simon Riggs <si...@2ndquadrant.com> wrote: > On Tue, 2011-01-11 at 12:41 +0100, Magnus Hagander wrote: > >> Just to be clear, you're objecting to the *name* of the state, right, >> not how/where it's set? > > Yes > >> In particular, how the catchup/streaming >> things are set? > > You've set it in the right places. > > I would personally constrain the state transitions, so that we can't > ever make illegal changes, such as CATCHUP -> BACKUP.
Well, once we enter the walsender loop we can never get out of it, so it simply cannot happen... > I would also check the state locally before grabbing the spinlock, as is > typical in other xlog code. Continually resetting shared state to > exactly what it is already seems strange, to me. If we make the rule > that the state can only be changed by the WALSender itself, it won't > need to grab spinlock. If you don't like that, a local variable works. Hmm. I don't see why anybody other than the walsender should change it, so yeah. You mean I can just drop the spinlock calls completely, and then do an if (walsnd->state != state) walsnd->state = state; ? Do I need to keep the volatile pointer, or should I drop that as well? > Also, mixing upper camel case and uppercase for the STATe NamES looKS > weIRD. Uppercase makes them look more clearly like enum/states as used > elsewhere in similar code. Yeah, I'll change that. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers