diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c index 9098c5d..5c62a5e 100644 --- a/src/backend/commands/vacuum.c +++ b/src/backend/commands/vacuum.c @@ -209,11 +209,12 @@ vacuum(VacuumStmt *vacstmt, Oid relid, bool do_toast, CommitTransactionCommand(); } - /* Turn vacuum cost accounting on or off */ + /* Adjust vacuum cost accounting state and update VacuumActive flag */ PG_TRY(); { ListCell *cur; + VacuumActive = true; VacuumCostActive = (VacuumCostDelay > 0); VacuumCostBalance = 0; @@ -254,8 +255,9 @@ vacuum(VacuumStmt *vacstmt, Oid relid, bool do_toast, } PG_CATCH(); { - /* Make sure cost accounting is turned off after error */ + /* Make sure vacuum state variables are fixed up on error */ VacuumCostActive = false; + VacuumActive = false; PG_RE_THROW(); } PG_END_TRY(); diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c index 1f89e52..6eee6f7 100644 --- a/src/backend/storage/buffer/bufmgr.c +++ b/src/backend/storage/buffer/bufmgr.c @@ -81,6 +81,10 @@ static bool IsForInput; /* local state for LockBufferForCleanup */ static volatile BufferDesc *PinCountWaitBuf = NULL; +/* local state for BufferAlloc */ +static int hint_bit_write_allowance; +static int buffer_allocation_count; + static Buffer ReadBuffer_common(SMgrRelation reln, char relpersistence, ForkNumber forkNum, BlockNumber blockNum, @@ -578,6 +582,7 @@ BufferAlloc(SMgrRelation smgr, char relpersistence, ForkNumber forkNum, for (;;) { bool lock_held; + bool write_buffer = false; /* * Select a victim buffer. The buffer is returned with its header @@ -600,13 +605,47 @@ BufferAlloc(SMgrRelation smgr, char relpersistence, ForkNumber forkNum, LWLockRelease(BufFreelistLock); /* + * If the buffer is dirty, we definitely need to write it out. If it's + * not dirty, but there are hint bits set, then we can decide whether + * or not we want to take the time to write it. We do that + * automatically when vacuuming, on theory that getting hint bits on + * disk will improve performance and is best done as part of a + * background operation. When not vacuuming, we're still willing to + * write a some hint bit pages (because getting hint bits out to disk + * improves performance of future scans), but we avoid writing too many + * in a row to prevent a dramatic performance drop-off when scanning a + * table with many hint bits unset. + * + * It's important to make sure that a large sequential scan which + * forces out hint bit changes doesn't create a lot of random I/O. + * To avoid that, we periodically grant ourselves a chunk of hint bit + * writes, use them up as quickly as needed, and then eventually grant + * ourselves another chunk. + */ + if (buffer_allocation_count++ % 2000) + hint_bit_write_allowance = 100; + if ((oldFlags & BM_DIRTY) != 0) + write_buffer = true; + else if ((oldFlags & BM_HINT_BITS) != 0) + { + if (VacuumActive) + write_buffer = true; + else if (hint_bit_write_allowance > 0) + { + write_buffer = true; + --hint_bit_write_allowance; + } + } + + /* * If the buffer was dirty, try to write it out. There is a race * condition here, in that someone might dirty it after we released it - * above, or even while we are writing it out (since our share-lock - * won't prevent hint-bit updates). We will recheck the dirty bit - * after re-locking the buffer header. + * above. (Once we acquire a share-lock on the buffer contents, it's + * no longer possible for it to get marked dirty, though hint bit + * updates could still occur.) We will recheck the dirty bit after + * re-locking the buffer header. */ - if (oldFlags & BM_DIRTY) + if (write_buffer) { /* * We need a share-lock on the buffer contents to write it out @@ -774,10 +813,11 @@ BufferAlloc(SMgrRelation smgr, char relpersistence, ForkNumber forkNum, LockBufHdr(buf); /* - * Somebody could have pinned or re-dirtied the buffer while we were - * doing the I/O and making the new hashtable entry. If so, we can't - * recycle this buffer; we must undo everything we've done and start - * over with a new victim buffer. + * Somebody could have re-dirtied the buffer before we acquired the + * shared content lock, or pinned it at any time up until we + * we re-locked the buffer header. If so, we can't recycle this + * buffer; we must undo everything we've done and start over with a + * new victim buffer. */ oldFlags = buf->flags; if (buf->refcount == 1 && !(oldFlags & BM_DIRTY)) @@ -801,7 +841,7 @@ BufferAlloc(SMgrRelation smgr, char relpersistence, ForkNumber forkNum, * 1 so that the buffer can survive one clock-sweep pass.) */ buf->tag = newTag; - buf->flags &= ~(BM_VALID | BM_DIRTY | BM_JUST_DIRTIED | BM_CHECKPOINT_NEEDED | BM_IO_ERROR | BM_PERMANENT); + buf->flags &= ~(BM_VALID | BM_DIRTY | BM_HINT_BITS | BM_CHECKPOINT_NEEDED | BM_IO_ERROR | BM_PERMANENT); if (relpersistence == RELPERSISTENCE_PERMANENT) buf->flags |= BM_TAG_VALID | BM_PERMANENT; else @@ -976,7 +1016,7 @@ MarkBufferDirty(Buffer buffer) if (!(bufHdr->flags & BM_DIRTY) && VacuumCostActive) VacuumCostBalance += VacuumCostPageDirty; - bufHdr->flags |= (BM_DIRTY | BM_JUST_DIRTIED); + bufHdr->flags |= BM_DIRTY; UnlockBufHdr(bufHdr); } @@ -1612,7 +1652,16 @@ SyncOneBuffer(int buf_id, bool skip_recently_used) return result; } - if (!(bufHdr->flags & BM_VALID) || !(bufHdr->flags & BM_DIRTY)) + /* + * We can get here either because we're checkpointing, or from the + * background writer cleaning scan. In the latter case, we want to + * write out buffers with hint bit changes so that such changes eventually + * make their way to disk. In the former case we normally won't get called + * if only hint bit changes have occurred, but if it somehow happens we'll + * just write the page anyway. + */ + if (!(bufHdr->flags & BM_VALID) + || !(bufHdr->flags & (BM_DIRTY | BM_HINT_BITS))) { /* It's clean, so nothing to do */ UnlockBufHdr(bufHdr); @@ -1878,13 +1927,11 @@ FlushBuffer(volatile BufferDesc *buf, SMgrRelation reln) * Now it's safe to write buffer to disk. Note that no one else should * have been able to write it while we were busy with log flushing because * we have the io_in_progress lock. + * + * The buffer might get marked BM_HINT_BITS while we're doing this, but + * our share-lock on the buffer contents is enough to prevent it from + * being marked BM_DIRTY. */ - - /* To check if block content changes while flushing. - vadim 01/17/97 */ - LockBufHdr(buf); - buf->flags &= ~BM_JUST_DIRTIED; - UnlockBufHdr(buf); - smgrwrite(reln, buf->tag.forkNum, buf->tag.blockNum, @@ -1894,8 +1941,7 @@ FlushBuffer(volatile BufferDesc *buf, SMgrRelation reln) pgBufferUsage.shared_blks_written++; /* - * Mark the buffer as clean (unless BM_JUST_DIRTIED has become set) and - * end the io_in_progress state. + * Mark the buffer as clean and end the io_in_progress state. */ TerminateBufferIO(buf, true, 0); @@ -2065,7 +2111,10 @@ PrintPinnedBufs(void) * * This function writes all dirty pages of a relation out to disk * (or more accurately, out to kernel disk buffers), ensuring that the - * kernel has an up-to-date view of the relation. + * kernel has an up-to-date view of the relation. We flush pages with + * only hint bit changes, too: this probably isn't critical, but since + * this only happens during relatively heavyweight operations, it + * shouldn't cost much. * * Generally, the caller should be holding AccessExclusiveLock on the * target relation to ensure that no other backend is busy dirtying @@ -2094,7 +2143,8 @@ FlushRelationBuffers(Relation rel) { bufHdr = &LocalBufferDescriptors[i]; if (RelFileNodeEquals(bufHdr->tag.rnode, rel->rd_node) && - (bufHdr->flags & BM_VALID) && (bufHdr->flags & BM_DIRTY)) + (bufHdr->flags & BM_VALID) && + (bufHdr->flags & (BM_DIRTY | BM_HINT_BITS))) { ErrorContextCallback errcontext; @@ -2110,7 +2160,7 @@ FlushRelationBuffers(Relation rel) (char *) LocalBufHdrGetBlock(bufHdr), false); - bufHdr->flags &= ~(BM_DIRTY | BM_JUST_DIRTIED); + bufHdr->flags &= ~BM_DIRTY; /* Pop the error context stack */ error_context_stack = errcontext.previous; @@ -2128,7 +2178,8 @@ FlushRelationBuffers(Relation rel) bufHdr = &BufferDescriptors[i]; LockBufHdr(bufHdr); if (RelFileNodeEquals(bufHdr->tag.rnode, rel->rd_node) && - (bufHdr->flags & BM_VALID) && (bufHdr->flags & BM_DIRTY)) + (bufHdr->flags & BM_VALID) && + (bufHdr->flags & (BM_DIRTY | BM_HINT_BITS))) { PinBuffer_Locked(bufHdr); LWLockAcquire(bufHdr->content_lock, LW_SHARED); @@ -2146,7 +2197,10 @@ FlushRelationBuffers(Relation rel) * * This function writes all dirty pages of a database out to disk * (or more accurately, out to kernel disk buffers), ensuring that the - * kernel has an up-to-date view of the database. + * kernel has an up-to-date view of the relation. We flush pages with + * only hint bit changes, too: this probably isn't critical, but since + * this only happens during relatively heavyweight operations, it + * shouldn't cost much. * * Generally, the caller should be holding an appropriate lock to ensure * no other backend is active in the target database; otherwise more @@ -2170,7 +2224,8 @@ FlushDatabaseBuffers(Oid dbid) bufHdr = &BufferDescriptors[i]; LockBufHdr(bufHdr); if (bufHdr->tag.rnode.dbNode == dbid && - (bufHdr->flags & BM_VALID) && (bufHdr->flags & BM_DIRTY)) + (bufHdr->flags & BM_VALID) && + (bufHdr->flags & (BM_DIRTY | BM_HINT_BITS))) { PinBuffer_Locked(bufHdr); LWLockAcquire(bufHdr->content_lock, LW_SHARED); @@ -2248,15 +2303,20 @@ IncrBufferRefCount(Buffer buffer) /* * SetBufferCommitInfoNeedsSave * - * Mark a buffer dirty when we have updated tuple commit-status bits in it. - * - * This is essentially the same as MarkBufferDirty, except that the caller - * might have only share-lock instead of exclusive-lock on the buffer's - * content lock. We preserve the distinction mainly as a way of documenting - * that the caller has not made a critical data change --- the status-bit - * update could be redone by someone else just as easily. Therefore, no WAL - * log record need be generated, whereas calls to MarkBufferDirty really ought - * to be associated with a WAL-entry-creating action. + * Mark a buffer as containing hint-bit changes. For local buffers, this + * is no different from MarkBufferDirty, excpet that the caller might have + * only share-lock instead of exclusive-lock on the buffer content. But for + * shared buffers, we set BM_HINT_BITS rather than BM_DIRTY. The background + * writer's cleaning scan will still write such buffers to speed up future + * access to the tuples stored in those blocks, but checkpoints and writes + * forced by buffer allocations will skip such writes, since they are + * non-critical. This allows some hint bit changes to make their way to disk + * without (hopefully) consuming too much I/O bandwidth. + * + * Note that a status-bit update could be redone by someone else just as + * easily. Therefore, no WAL log record need be generated, whereas calls to + * MarkBufferDirty really ought to be associated with a WAL-entry-creating + * action. */ void SetBufferCommitInfoNeedsSave(Buffer buffer) @@ -2281,20 +2341,19 @@ SetBufferCommitInfoNeedsSave(Buffer buffer) /* * This routine might get called many times on the same page, if we are * making the first scan after commit of an xact that added/deleted many - * tuples. So, be as quick as we can if the buffer is already dirty. We - * do this by not acquiring spinlock if it looks like the status bits are - * already OK. (Note it is okay if someone else clears BM_JUST_DIRTIED - * immediately after we look, because the buffer content update is already - * done and will be reflected in the I/O.) + * tuples. So, be as quick as we can if the buffer is already marked. We + * do this by not acquiring spinlock if it looks like BM_HINT_BITS is + * already set. (If memory-ordering effects cause us to see a stale value, + * the worst thing that can happen is we fail to set BM_HINT_BITS. But + * that won't result in data corruption, since hint bits are not critical.) */ - if ((bufHdr->flags & (BM_DIRTY | BM_JUST_DIRTIED)) != - (BM_DIRTY | BM_JUST_DIRTIED)) + if (!(bufHdr->flags & BM_HINT_BITS)) { LockBufHdr(bufHdr); Assert(bufHdr->refcount > 0); - if (!(bufHdr->flags & BM_DIRTY) && VacuumCostActive) + if (!(bufHdr->flags & BM_HINT_BITS) && VacuumCostActive) VacuumCostBalance += VacuumCostPageDirty; - bufHdr->flags |= (BM_DIRTY | BM_JUST_DIRTIED); + bufHdr->flags |= BM_HINT_BITS; UnlockBufHdr(bufHdr); } } @@ -2583,8 +2642,8 @@ WaitIO(volatile BufferDesc *buf) * io_in_progress lock until he's done. * * Input operations are only attempted on buffers that are not BM_VALID, - * and output operations only on buffers that are BM_VALID and BM_DIRTY, - * so we can always tell if the work is already done. + * and output operations only on buffers that are BM_VALID and either BM_DIRTY + * or BM_HINT_BITS, so we can always tell if the work is already done. * * Returns TRUE if we successfully marked the buffer as I/O busy, * FALSE if someone else already did the work. @@ -2620,7 +2679,8 @@ StartBufferIO(volatile BufferDesc *buf, bool forInput) /* Once we get here, there is definitely no I/O active on this buffer */ - if (forInput ? (buf->flags & BM_VALID) : !(buf->flags & BM_DIRTY)) + if (forInput ? (buf->flags & BM_VALID) : + !(buf->flags & (BM_DIRTY | BM_HINT_BITS))) { /* someone else already did the I/O */ UnlockBufHdr(buf); @@ -2646,10 +2706,11 @@ StartBufferIO(volatile BufferDesc *buf, bool forInput) * We hold the buffer's io_in_progress lock * The buffer is Pinned * - * If clear_dirty is TRUE and BM_JUST_DIRTIED is not set, we clear the - * buffer's BM_DIRTY flag. This is appropriate when terminating a - * successful write. The check on BM_JUST_DIRTIED is necessary to avoid - * marking the buffer clean if it was re-dirtied while we were writing. + * If clear_dirty is TRUE, we clear the buffer's BM_DIRTY, BM_HINT_BITS, + * and BM_CHECKPOINT_NEEDED flags. It's possible that that more hint bits + * have been set while we the buffer was being written, and that those changes + * were not included in the I/O. But we don't really care, because hint bit + * updates are not critical and can be redone later if they're lost. * * set_flag_bits gets ORed into the buffer's flags. It must include * BM_IO_ERROR in a failure case. For successful completion it could @@ -2665,8 +2726,8 @@ TerminateBufferIO(volatile BufferDesc *buf, bool clear_dirty, Assert(buf->flags & BM_IO_IN_PROGRESS); buf->flags &= ~(BM_IO_IN_PROGRESS | BM_IO_ERROR); - if (clear_dirty && !(buf->flags & BM_JUST_DIRTIED)) - buf->flags &= ~(BM_DIRTY | BM_CHECKPOINT_NEEDED); + if (clear_dirty) + buf->flags &= ~(BM_DIRTY | BM_HINT_BITS | BM_CHECKPOINT_NEEDED); buf->flags |= set_flag_bits; UnlockBufHdr(buf); @@ -2704,7 +2765,7 @@ AbortBufferIO(void) Assert(buf->flags & BM_IO_IN_PROGRESS); if (IsForInput) { - Assert(!(buf->flags & BM_DIRTY)); + Assert(!(buf->flags & (BM_DIRTY | BM_HINT_BITS))); /* We'd better not think buffer is valid yet */ Assert(!(buf->flags & BM_VALID)); UnlockBufHdr(buf); @@ -2714,7 +2775,7 @@ AbortBufferIO(void) BufFlags sv_flags; sv_flags = buf->flags; - Assert(sv_flags & BM_DIRTY); + Assert(sv_flags & (BM_DIRTY | BM_HINT_BITS)); UnlockBufHdr(buf); /* Issue notice if this is not the first failure... */ if (sv_flags & BM_IO_ERROR) diff --git a/src/backend/storage/buffer/localbuf.c b/src/backend/storage/buffer/localbuf.c index 0a01ed5..d1d544e 100644 --- a/src/backend/storage/buffer/localbuf.c +++ b/src/backend/storage/buffer/localbuf.c @@ -248,7 +248,7 @@ LocalBufferAlloc(SMgrRelation smgr, ForkNumber forkNum, BlockNumber blockNum, * it's all ours now. */ bufHdr->tag = newTag; - bufHdr->flags &= ~(BM_VALID | BM_DIRTY | BM_JUST_DIRTIED | BM_IO_ERROR); + bufHdr->flags &= ~(BM_VALID | BM_DIRTY | BM_HINT_BITS | BM_IO_ERROR); bufHdr->flags |= BM_TAG_VALID; bufHdr->usage_count = 1; diff --git a/src/backend/utils/init/globals.c b/src/backend/utils/init/globals.c index 984ffd0..26b08ed 100644 --- a/src/backend/utils/init/globals.c +++ b/src/backend/utils/init/globals.c @@ -116,6 +116,7 @@ int VacuumCostDelay = 0; int VacuumCostBalance = 0; /* working state for vacuum */ bool VacuumCostActive = false; +bool VacuumActive = false; int GinFuzzySearchLimit = 0; diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h index aa8cce5..4d7fbe5 100644 --- a/src/include/miscadmin.h +++ b/src/include/miscadmin.h @@ -231,6 +231,7 @@ extern int VacuumCostDelay; extern int VacuumCostBalance; extern bool VacuumCostActive; +extern bool VacuumActive; /* in tcop/postgres.c */ diff --git a/src/include/storage/buf_internals.h b/src/include/storage/buf_internals.h index 0652bdf..78a4761 100644 --- a/src/include/storage/buf_internals.h +++ b/src/include/storage/buf_internals.h @@ -34,7 +34,7 @@ #define BM_TAG_VALID (1 << 2) /* tag is assigned */ #define BM_IO_IN_PROGRESS (1 << 3) /* read or write in progress */ #define BM_IO_ERROR (1 << 4) /* previous I/O failed */ -#define BM_JUST_DIRTIED (1 << 5) /* dirtied since write started */ +#define BM_HINT_BITS (1 << 5) /* hint bits changed */ #define BM_PIN_COUNT_WAITER (1 << 6) /* have waiter for sole pin */ #define BM_CHECKPOINT_NEEDED (1 << 7) /* must write for checkpoint */ #define BM_PERMANENT (1 << 8) /* permanent relation (not unlogged) */