On Thu, Jun 4, 2015 at 2:42 AM, Noah Misch <noah@xxxxxxxxxxxx> wrote: > I like that change a lot. It's much easier to seek forgiveness for wasting <= > 28 GiB of disk than for deleting visibility information wrongly. I'm glad you like it. I concur. >> 2. If setting the offset stop limit (the point where we refuse to >> create new multixact space), we don't arm the stop point. This means >> that if you're in this situation, you run without member wraparound >> protection until it's corrected. A message gets logged once per >> checkpoint telling you that you have this problem, and another message >> gets logged when things get straightened out and the guards are >> enabled. >> >> 3. If setting the vacuum force point, we assume that it's appropriate >> to immediately force vacuum. > > Those seem reasonable, too. Cool. >> I've only tested this very lightly - this is just to see what you and >> Noah and others think of the approach. As compared with the previous >> approach, it has the advantage of making minimal assumptions about the >> sanity of what's on disk. It has the disadvantage that, for some >> people, the member-wraparound guard won't be enabled at startup -- but >> note that those people can't start 9.3.7/9.4.2 *at all*, so currently >> they are either running without member wraparound protection anyway >> (if they haven't upgraded to those releases) or they're down entirely. > > That disadvantage is negligible, considering. All right. >> Another disadvantage is that we'll be triggering what may be quite a >> bit of autovacuum activity for some people, which could be painful. >> On the plus side, they'll hopefully end up with sane relminmxid and >> datminmxid guards afterwards. > > That sounds good so long as each table requires just one successful emergency > autovacuum. I'm not seeing code to ensure that the launched autovacuum will > indeed perform a full-table scan and update relminmxid; is it there? No. Oops. > For sites that can't tolerate an autovacuum storm, what alternative can we > provide? Is "SET vacuum_multixact_freeze_table_age = 0; VACUUM <table>" of > every table, done before applying the minor update, sufficient? I don't know. In practical terms, they probably need to ensure that if pg_multixact/offsets/0000 does not exist, no relations have relminmxid = 1 and no remaining databases have datminmxid = 1. Exactly what it will take to get there is possibly dependent on which minor release you are running; on current minor releases, I am hopeful that what you propose is sufficient. >> static void >> -DetermineSafeOldestOffset(MultiXactId oldestMXact) >> +DetermineSafeOldestOffset(MultiXactOffset oldestMXact) > > Leftover change from an earlier iteration? The values passed continue to be > MultiXactId values. Oopsie. >> /* move back to start of the corresponding segment */ >> - oldestOffset -= oldestOffset % >> - (MULTIXACT_MEMBERS_PER_PAGE * SLRU_PAGES_PER_SEGMENT); >> + offsetStopLimit = oldestOffset - (oldestOffset % >> + (MULTIXACT_MEMBERS_PER_PAGE * SLRU_PAGES_PER_SEGMENT)); >> + /* always leave one segment before the wraparound point */ >> + offsetStopLimit -= (MULTIXACT_MEMBERS_PER_PAGE * SLRU_PAGES_PER_SEGMENT); >> + >> + /* if nothing has changed, we're done */ >> + if (prevOffsetStopLimitKnown && offsetStopLimit == prevOffsetStopLimit) >> + return; >> >> LWLockAcquire(MultiXactGenLock, LW_EXCLUSIVE); >> - /* always leave one segment before the wraparound point */ >> - MultiXactState->offsetStopLimit = oldestOffset - >> - (MULTIXACT_MEMBERS_PER_PAGE * SLRU_PAGES_PER_SEGMENT); >> + MultiXactState->offsetStopLimit = oldestOffset; > > That last line needs s/oldestOffset/offsetStopLimit/, I presume. Another oops. Thanks for the review. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-general mailing list (pgsql-general@xxxxxxxxxxxxxx) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-general
