agora inbox for [email protected]help / color / mirror / Atom feed
[PATCH v6 13/14] v7 Streaming Read API 5+ messages / 2 participants [nested] [flat]
* [PATCH v6 13/14] v7 Streaming Read API @ 2024-03-14 16:59 Melanie Plageman <[email protected]> 0 siblings, 0 replies; 5+ messages in thread From: Melanie Plageman @ 2024-03-14 16:59 UTC (permalink / raw) --- src/backend/storage/Makefile | 2 +- src/backend/storage/aio/Makefile | 14 + src/backend/storage/aio/meson.build | 5 + src/backend/storage/aio/streaming_read.c | 659 +++++++++++++++++++++++ src/backend/storage/buffer/bufmgr.c | 642 +++++++++++++++------- src/backend/storage/buffer/localbuf.c | 14 +- src/backend/storage/meson.build | 1 + src/include/storage/bufmgr.h | 45 ++ src/include/storage/streaming_read.h | 52 ++ src/tools/pgindent/typedefs.list | 3 + 10 files changed, 1227 insertions(+), 210 deletions(-) create mode 100644 src/backend/storage/aio/Makefile create mode 100644 src/backend/storage/aio/meson.build create mode 100644 src/backend/storage/aio/streaming_read.c create mode 100644 src/include/storage/streaming_read.h diff --git a/src/backend/storage/Makefile b/src/backend/storage/Makefile index 8376cdfca20..eec03f6f2b4 100644 --- a/src/backend/storage/Makefile +++ b/src/backend/storage/Makefile @@ -8,6 +8,6 @@ subdir = src/backend/storage top_builddir = ../../.. include $(top_builddir)/src/Makefile.global -SUBDIRS = buffer file freespace ipc large_object lmgr page smgr sync +SUBDIRS = aio buffer file freespace ipc large_object lmgr page smgr sync include $(top_srcdir)/src/backend/common.mk diff --git a/src/backend/storage/aio/Makefile b/src/backend/storage/aio/Makefile new file mode 100644 index 00000000000..bcab44c802f --- /dev/null +++ b/src/backend/storage/aio/Makefile @@ -0,0 +1,14 @@ +# +# Makefile for storage/aio +# +# src/backend/storage/aio/Makefile +# + +subdir = src/backend/storage/aio +top_builddir = ../../../.. +include $(top_builddir)/src/Makefile.global + +OBJS = \ + streaming_read.o + +include $(top_srcdir)/src/backend/common.mk diff --git a/src/backend/storage/aio/meson.build b/src/backend/storage/aio/meson.build new file mode 100644 index 00000000000..39aef2a84a2 --- /dev/null +++ b/src/backend/storage/aio/meson.build @@ -0,0 +1,5 @@ +# Copyright (c) 2024, PostgreSQL Global Development Group + +backend_sources += files( + 'streaming_read.c', +) diff --git a/src/backend/storage/aio/streaming_read.c b/src/backend/storage/aio/streaming_read.c new file mode 100644 index 00000000000..d5c29b750d9 --- /dev/null +++ b/src/backend/storage/aio/streaming_read.c @@ -0,0 +1,659 @@ +#include "postgres.h" + +#include "catalog/pg_tablespace.h" +#include "miscadmin.h" +#include "storage/streaming_read.h" +#include "utils/rel.h" +#include "utils/spccache.h" + +/* + * Element type for PgStreamingRead's circular array of block ranges. + */ +typedef struct PgStreamingReadRange +{ + bool need_wait; + bool advice_issued; + BlockNumber blocknum; + int nblocks; + int per_buffer_data_index; + Buffer buffers[MAX_BUFFERS_PER_TRANSFER]; + ReadBuffersOperation operation; +} PgStreamingReadRange; + +/* + * Streaming read object. + */ +struct PgStreamingRead +{ + int max_ios; + int ios_in_progress; + int max_pinned_buffers; + int pinned_buffers; + int next_tail_buffer; + int distance; + bool started; + bool finished; + bool advice_enabled; + void *pgsr_private; + PgStreamingReadBufferCB callback; + + BufferAccessStrategy strategy; + BufferManagerRelation bmr; + ForkNumber forknum; + + /* Sometimes we need to buffer one block for flow control. */ + BlockNumber unget_blocknum; + void *unget_per_buffer_data; + + /* Next expected block, for detecting sequential access. */ + BlockNumber seq_blocknum; + + /* Space for optional per-buffer private data. */ + size_t per_buffer_data_size; + void *per_buffer_data; + + /* Circular buffer of ranges. */ + int size; + int head; + int tail; + PgStreamingReadRange ranges[FLEXIBLE_ARRAY_MEMBER]; +}; + +/* + * Create a new streaming read object that can be used to perform the + * equivalent of a series of ReadBuffer() calls for one fork of one relation. + * Internally, it generates larger vectored reads where possible by looking + * ahead. + */ +PgStreamingRead * +pg_streaming_read_buffer_alloc(int flags, + void *pgsr_private, + size_t per_buffer_data_size, + BufferAccessStrategy strategy, + BufferManagerRelation bmr, + ForkNumber forknum, + PgStreamingReadBufferCB next_block_cb) +{ + PgStreamingRead *pgsr; + int size; + int max_ios; + uint32 max_pinned_buffers; + Oid tablespace_id; + + /* + * Make sure our bmr's smgr and persistent are populated. The caller + * asserts that the storage manager will remain valid. + */ + if (!bmr.smgr) + { + bmr.smgr = RelationGetSmgr(bmr.rel); + bmr.relpersistence = bmr.rel->rd_rel->relpersistence; + } + + /* + * Decide how many assumed I/Os we will allow to run concurrently. That + * is, advice to the kernel to tell it that we will soon read. This + * number also affects how far we look ahead for opportunities to start + * more I/Os. + */ + tablespace_id = bmr.smgr->smgr_rlocator.locator.spcOid; + if (!OidIsValid(MyDatabaseId) || + (bmr.rel && IsCatalogRelation(bmr.rel)) || + IsCatalogRelationOid(bmr.smgr->smgr_rlocator.locator.relNumber)) + { + /* + * Avoid circularity while trying to look up tablespace settings or + * before spccache.c is ready. + */ + max_ios = effective_io_concurrency; + } + else if (flags & PGSR_FLAG_MAINTENANCE) + max_ios = get_tablespace_maintenance_io_concurrency(tablespace_id); + else + max_ios = get_tablespace_io_concurrency(tablespace_id); + + /* + * Choose a maximum number of buffers we're prepared to pin. We try to + * pin fewer if we can, though. We clamp it to at least + * MAX_BUFFER_PER_TRANSFER so that we can have a chance to build up a full + * sized read, even when max_ios is zero. + */ + max_pinned_buffers = Max(max_ios * 4, MAX_BUFFERS_PER_TRANSFER); + + /* Don't allow this backend to pin more than its share of buffers. */ + if (SmgrIsTemp(bmr.smgr)) + LimitAdditionalLocalPins(&max_pinned_buffers); + else + LimitAdditionalPins(&max_pinned_buffers); + Assert(max_pinned_buffers > 0); + + /* + * pgsr->ranges is a circular buffer. When it is empty, head == tail. + * When it is full, there is an empty element between head and tail. Head + * can also be empty (nblocks == 0), therefore we need two extra elements + * for non-occupied ranges, on top of max_pinned_buffers to allow for the + * maxmimum possible number of occupied ranges of the smallest possible + * size of one. + */ + size = max_pinned_buffers + 2; + + pgsr = (PgStreamingRead *) + palloc0(offsetof(PgStreamingRead, ranges) + + sizeof(pgsr->ranges[0]) * size); + + pgsr->max_ios = max_ios; + pgsr->per_buffer_data_size = per_buffer_data_size; + pgsr->max_pinned_buffers = max_pinned_buffers; + pgsr->pgsr_private = pgsr_private; + pgsr->strategy = strategy; + pgsr->size = size; + + pgsr->callback = next_block_cb; + pgsr->bmr = bmr; + pgsr->forknum = forknum; + + pgsr->unget_blocknum = InvalidBlockNumber; + +#ifdef USE_PREFETCH + + /* + * This system supports prefetching advice. As long as direct I/O isn't + * enabled, and the caller hasn't promised sequential access, we can use + * it. + */ + if ((io_direct_flags & IO_DIRECT_DATA) == 0 && + (flags & PGSR_FLAG_SEQUENTIAL) == 0) + pgsr->advice_enabled = true; +#endif + + /* + * Skip the initial ramp-up phase if the caller says we're going to be + * reading the whole relation. This way we start out doing full-sized + * reads. + */ + if (flags & PGSR_FLAG_FULL) + pgsr->distance = Min(MAX_BUFFERS_PER_TRANSFER, pgsr->max_pinned_buffers); + else + pgsr->distance = 1; + + /* + * Space for the callback to store extra data along with each block. Note + * that we need one more than max_pinned_buffers, so we can return a + * pointer to a slot that can't be overwritten until the next call. + */ + if (per_buffer_data_size) + pgsr->per_buffer_data = palloc(per_buffer_data_size * size); + + return pgsr; +} + +/* + * Find the per-buffer data index for the Nth block of a range. + */ +static int +get_per_buffer_data_index(PgStreamingRead *pgsr, PgStreamingReadRange *range, int n) +{ + int result; + + /* + * Find slot in the circular buffer of per-buffer data, without using the + * expensive % operator. + */ + result = range->per_buffer_data_index + n; + while (result >= pgsr->size) + result -= pgsr->size; + Assert(result == (range->per_buffer_data_index + n) % pgsr->size); + + return result; +} + +/* + * Return a pointer to the per-buffer data by index. + */ +static void * +get_per_buffer_data_by_index(PgStreamingRead *pgsr, int per_buffer_data_index) +{ + return (char *) pgsr->per_buffer_data + + pgsr->per_buffer_data_size * per_buffer_data_index; +} + +/* + * Return a pointer to the per-buffer data for the Nth block of a range. + */ +static void * +get_per_buffer_data(PgStreamingRead *pgsr, PgStreamingReadRange *range, int n) +{ + return get_per_buffer_data_by_index(pgsr, + get_per_buffer_data_index(pgsr, + range, + n)); +} + +/* + * Start reading the head range, and create a new head range. The new head + * range is returned. It may not be empty, if StartReadBuffers() couldn't + * start the entire range; in that case the returned range contains the + * remaining portion of the range. + */ +static PgStreamingReadRange * +pg_streaming_read_start_head_range(PgStreamingRead *pgsr) +{ + PgStreamingReadRange *head_range; + PgStreamingReadRange *new_head_range; + int nblocks_pinned; + int flags; + + /* Caller should make sure we never exceed max_ios. */ + Assert((pgsr->ios_in_progress < pgsr->max_ios) || + (pgsr->ios_in_progress == 0 && pgsr->max_ios == 0)); + + /* Should only call if the head range has some blocks to read. */ + head_range = &pgsr->ranges[pgsr->head]; + Assert(head_range->nblocks > 0); + + /* + * If advice hasn't been suppressed, and this system supports it, this + * isn't a strictly sequential pattern, then we'll issue advice. + */ + if (pgsr->advice_enabled && + pgsr->max_ios > 0 && + pgsr->started && + head_range->blocknum != pgsr->seq_blocknum) + flags = READ_BUFFERS_ISSUE_ADVICE; + else + flags = 0; + + /* Suppress advice on the first call, because it's too late to benefit. */ + if (!pgsr->started) + pgsr->started = true; + + /* We shouldn't be trying to pin more buffers that we're allowed to. */ + Assert(pgsr->pinned_buffers + head_range->nblocks <= pgsr->max_pinned_buffers); + + /* Start reading as many blocks as we can from the head range. */ + nblocks_pinned = head_range->nblocks; + head_range->need_wait = + StartReadBuffers(pgsr->bmr, + head_range->buffers, + pgsr->forknum, + head_range->blocknum, + &nblocks_pinned, + pgsr->strategy, + flags, + &head_range->operation); + + Assert(pgsr->pinned_buffers <= pgsr->max_pinned_buffers); + + if (head_range->need_wait && (flags & READ_BUFFERS_ISSUE_ADVICE)) + { + /* + * Since we've issued advice, we count an I/O in progress until we + * call WaitReadBuffers(). + */ + head_range->advice_issued = true; + pgsr->ios_in_progress++; + Assert(pgsr->ios_in_progress <= pgsr->max_ios); + } + + /* + * StartReadBuffers() might have pinned fewer blocks than we asked it to, + * but always at least one. + */ + Assert(nblocks_pinned <= head_range->nblocks); + Assert(nblocks_pinned >= 1); + pgsr->pinned_buffers += nblocks_pinned; + + /* + * Remember where the next block would be after that, so we can detect + * sequential access next time. + */ + pgsr->seq_blocknum = head_range->blocknum + nblocks_pinned; + + /* + * Create a new head range. There must be space, because we have enough + * elements for every range to hold just one block, up to the pin limit. + */ + Assert(pgsr->size > pgsr->max_pinned_buffers); + Assert((pgsr->head + 1) % pgsr->size != pgsr->tail); + if (++pgsr->head == pgsr->size) + pgsr->head = 0; + new_head_range = &pgsr->ranges[pgsr->head]; + new_head_range->nblocks = 0; + new_head_range->advice_issued = false; + + /* + * If we didn't manage to start the whole read above, we split the range, + * moving the remainder into the new head range. + */ + if (nblocks_pinned < head_range->nblocks) + { + int nblocks_remaining = head_range->nblocks - nblocks_pinned; + + head_range->nblocks = nblocks_pinned; + + new_head_range->blocknum = head_range->blocknum + nblocks_pinned; + new_head_range->nblocks = nblocks_remaining; + } + + /* The new range has per-buffer data starting after the previous range. */ + new_head_range->per_buffer_data_index = + get_per_buffer_data_index(pgsr, head_range, nblocks_pinned); + + return new_head_range; +} + +/* + * Ask the callback which block it would like us to read next, with a small + * buffer in front to allow pg_streaming_unget_block() to work. + */ +static BlockNumber +pg_streaming_get_block(PgStreamingRead *pgsr, void *per_buffer_data) +{ + BlockNumber result; + + if (unlikely(pgsr->unget_blocknum != InvalidBlockNumber)) + { + /* + * If we had to unget a block, now it is time to return that one + * again. + */ + result = pgsr->unget_blocknum; + pgsr->unget_blocknum = InvalidBlockNumber; + + /* + * The same per_buffer_data element must have been used, and still + * contains whatever data the callback wrote into it. So we just + * sanity-check that we were called with the value that + * pg_streaming_unget_block() pushed back. + */ + Assert(per_buffer_data == pgsr->unget_per_buffer_data); + } + else + { + /* Use the installed callback directly. */ + result = pgsr->callback(pgsr, pgsr->pgsr_private, per_buffer_data); + } + + return result; +} + +/* + * In order to deal with short reads in StartReadBuffers(), we sometimes need + * to defer handling of a block until later. This *must* be called with the + * last value returned by pg_streaming_get_block(). + */ +static void +pg_streaming_unget_block(PgStreamingRead *pgsr, BlockNumber blocknum, void *per_buffer_data) +{ + Assert(pgsr->unget_blocknum == InvalidBlockNumber); + pgsr->unget_blocknum = blocknum; + pgsr->unget_per_buffer_data = per_buffer_data; +} + +static void +pg_streaming_read_look_ahead(PgStreamingRead *pgsr) +{ + PgStreamingReadRange *range; + + /* If we're finished, don't look ahead. */ + if (pgsr->finished) + return; + + /* + * We we've already started the maximum allowed number of I/Os, don't look + * ahead. There is a special case for max_ios == 0. + */ + if (pgsr->max_ios > 0 && pgsr->ios_in_progress == pgsr->max_ios) + return; + + /* Can't pin any more buffers. */ + if (pgsr->pinned_buffers == pgsr->distance) + return; + + /* + * Keep trying to add new blocks to the end of the head range while doing + * so wouldn't exceed the distance limit. + */ + range = &pgsr->ranges[pgsr->head]; + while (pgsr->pinned_buffers + range->nblocks < pgsr->distance) + { + BlockNumber blocknum; + void *per_buffer_data; + + /* Do we have a full-sized range? */ + if (range->nblocks == lengthof(range->buffers)) + { + /* Start as much of it as we can. */ + range = pg_streaming_read_start_head_range(pgsr); + + /* If we're now at the I/O limit, stop here. */ + if (pgsr->ios_in_progress == pgsr->max_ios) + return; + + /* + * That might have only been partially started, but always + * processes at least one so that'll do for now. + */ + Assert(range->nblocks < lengthof(range->buffers)); + } + + /* Find per-buffer data slot for the next block. */ + per_buffer_data = get_per_buffer_data(pgsr, range, range->nblocks); + + /* Find out which block the callback wants to read next. */ + blocknum = pg_streaming_get_block(pgsr, per_buffer_data); + if (blocknum == InvalidBlockNumber) + { + /* End of stream. */ + pgsr->finished = true; + break; + } + + /* + * Is there a head range that we cannot extend, because the requested + * block is not consecutive? + */ + if (range->nblocks > 0 && + range->blocknum + range->nblocks != blocknum) + { + /* Yes. Start it, so we can begin building a new one. */ + range = pg_streaming_read_start_head_range(pgsr); + + /* + * It's possible that it was only partially started, and we have a + * new range with the remainder. Keep starting I/Os until we get + * it all out of the way, or we hit the I/O limit. + */ + while (range->nblocks > 0 && pgsr->ios_in_progress < pgsr->max_ios) + range = pg_streaming_read_start_head_range(pgsr); + + /* + * We do have to worry about I/O capacity running out if the head + * range was split. In that case we have to 'unget' the block + * returned by the callback. + */ + if (pgsr->ios_in_progress == pgsr->max_ios) + { + pg_streaming_unget_block(pgsr, blocknum, per_buffer_data); + return; + } + } + + /* If we have a new, empty range, initialize the start block. */ + if (range->nblocks == 0) + range->blocknum = blocknum; + + /* This block extends the range by one. */ + Assert(range->blocknum + range->nblocks == blocknum); + range->nblocks++; + }; + + /* + * Normally we don't start the head range, preferring to give it a chance + * to grow to full size once more buffers have been consumed. In cases + * where that can't possibly happen, we might as well start the read + * immediately. + */ + if ((range->nblocks > 0 && pgsr->finished) || + (range->nblocks == pgsr->distance)) + pg_streaming_read_start_head_range(pgsr); +} + +Buffer +pg_streaming_read_buffer_get_next(PgStreamingRead *pgsr, void **per_buffer_data) +{ + PgStreamingReadRange *tail_range; + + for (;;) + { + if (pgsr->tail != pgsr->head) + { + tail_range = &pgsr->ranges[pgsr->tail]; + + /* + * Do we need to wait for a ReadBuffers operation to finish before + * returning the buffers in this range? + */ + if (tail_range->need_wait) + { + int distance; + + Assert(pgsr->next_tail_buffer == 0); + WaitReadBuffers(&tail_range->operation); + tail_range->need_wait = false; + + /* + * We don't really know if the kernel generated a physical I/O + * when we issued advice, let alone when it finished, but it + * has certainly finished now because we've performed the + * read. + */ + if (tail_range->advice_issued) + { + + Assert(pgsr->ios_in_progress > 0); + pgsr->ios_in_progress--; + + /* + * Look-ahead distance ramps up rapidly if we're issuing + * advice, so we can search for new more I/Os to start. + */ + distance = pgsr->distance * 2; + distance = Min(distance, pgsr->max_pinned_buffers); + pgsr->distance = distance; + } + else + { + /* + * There is no point in increasing look-ahead distance if + * we've already reached the full I/O size, since we're + * not issuing advice. Extra distance would only pin more + * buffers for no benefit. + */ + if (pgsr->distance > MAX_BUFFERS_PER_TRANSFER) + { + /* + * Look-ahead distance gradually decays to full I/O + * size. + */ + pgsr->distance--; + } + else + { + /* + * Look-ahead distance ramps up rapidly, but not more + * that the full I/O size. + */ + distance = pgsr->distance * 2; + distance = Min(distance, MAX_BUFFERS_PER_TRANSFER); + distance = Min(distance, pgsr->max_pinned_buffers); + pgsr->distance = distance; + } + } + } + else if (pgsr->next_tail_buffer == 0) + { + /* No I/O necessary. Look-ahead distance gradually decays. */ + if (pgsr->distance > 1) + pgsr->distance--; + } + + /* Are there more buffers available in this range? */ + if (pgsr->next_tail_buffer < tail_range->nblocks) + { + int buffer_index; + Buffer buffer; + + buffer_index = pgsr->next_tail_buffer++; + buffer = tail_range->buffers[buffer_index]; + + Assert(BufferIsValid(buffer)); + + /* We are giving away ownership of this pinned buffer. */ + Assert(pgsr->pinned_buffers > 0); + pgsr->pinned_buffers--; + + if (per_buffer_data) + *per_buffer_data = get_per_buffer_data(pgsr, tail_range, buffer_index); + + /* We may be able to get another I/O started. */ + pg_streaming_read_look_ahead(pgsr); + + return buffer; + } + + /* Advance tail to next range. */ + if (++pgsr->tail == pgsr->size) + pgsr->tail = 0; + pgsr->next_tail_buffer = 0; + } + else + { + /* + * If tail crashed into head, and head is not empty, then it is + * time to start that range. Otherwise, force a look-ahead, to + * kick start the stream. + */ + Assert(pgsr->tail == pgsr->head); + if (pgsr->ranges[pgsr->head].nblocks > 0) + { + pg_streaming_read_start_head_range(pgsr); + } + else + { + pg_streaming_read_look_ahead(pgsr); + + /* Finished? */ + if (pgsr->tail == pgsr->head && + pgsr->ranges[pgsr->head].nblocks == 0) + break; + } + } + } + + Assert(pgsr->pinned_buffers == 0); + + return InvalidBuffer; +} + +void +pg_streaming_read_free(PgStreamingRead *pgsr) +{ + Buffer buffer; + + /* Stop looking ahead. */ + pgsr->finished = true; + + /* Unpin anything that wasn't consumed. */ + while ((buffer = pg_streaming_read_buffer_get_next(pgsr, NULL)) != InvalidBuffer) + ReleaseBuffer(buffer); + + Assert(pgsr->pinned_buffers == 0); + Assert(pgsr->ios_in_progress == 0); + + /* Release memory. */ + if (pgsr->per_buffer_data) + pfree(pgsr->per_buffer_data); + + pfree(pgsr); +} diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c index f0f8d4259c5..d0e9c7deffa 100644 --- a/src/backend/storage/buffer/bufmgr.c +++ b/src/backend/storage/buffer/bufmgr.c @@ -19,6 +19,11 @@ * and pin it so that no one can destroy it while this process * is using it. * + * StartReadBuffers() -- as above, but for multiple contiguous blocks in + * two steps. + * + * WaitReadBuffers() -- second step of StartReadBuffers(). + * * ReleaseBuffer() -- unpin a buffer * * MarkBufferDirty() -- mark a pinned buffer's contents as "dirty". @@ -471,10 +476,9 @@ ForgetPrivateRefCountEntry(PrivateRefCountEntry *ref) ) -static Buffer ReadBuffer_common(SMgrRelation smgr, char relpersistence, +static Buffer ReadBuffer_common(BufferManagerRelation bmr, ForkNumber forkNum, BlockNumber blockNum, - ReadBufferMode mode, BufferAccessStrategy strategy, - bool *hit); + ReadBufferMode mode, BufferAccessStrategy strategy); static BlockNumber ExtendBufferedRelCommon(BufferManagerRelation bmr, ForkNumber fork, BufferAccessStrategy strategy, @@ -500,7 +504,7 @@ static uint32 WaitBufHdrUnlocked(BufferDesc *buf); static int SyncOneBuffer(int buf_id, bool skip_recently_used, WritebackContext *wb_context); static void WaitIO(BufferDesc *buf); -static bool StartBufferIO(BufferDesc *buf, bool forInput); +static bool StartBufferIO(BufferDesc *buf, bool forInput, bool nowait); static void TerminateBufferIO(BufferDesc *buf, bool clear_dirty, uint32 set_flag_bits, bool forget_owner); static void AbortBufferIO(Buffer buffer); @@ -781,7 +785,6 @@ Buffer ReadBufferExtended(Relation reln, ForkNumber forkNum, BlockNumber blockNum, ReadBufferMode mode, BufferAccessStrategy strategy) { - bool hit; Buffer buf; /* @@ -794,15 +797,9 @@ ReadBufferExtended(Relation reln, ForkNumber forkNum, BlockNumber blockNum, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("cannot access temporary tables of other sessions"))); - /* - * Read the buffer, and update pgstat counters to reflect a cache hit or - * miss. - */ - pgstat_count_buffer_read(reln); - buf = ReadBuffer_common(RelationGetSmgr(reln), reln->rd_rel->relpersistence, - forkNum, blockNum, mode, strategy, &hit); - if (hit) - pgstat_count_buffer_hit(reln); + buf = ReadBuffer_common(BMR_REL(reln), + forkNum, blockNum, mode, strategy); + return buf; } @@ -822,13 +819,12 @@ ReadBufferWithoutRelcache(RelFileLocator rlocator, ForkNumber forkNum, BlockNumber blockNum, ReadBufferMode mode, BufferAccessStrategy strategy, bool permanent) { - bool hit; - SMgrRelation smgr = smgropen(rlocator, INVALID_PROC_NUMBER); - return ReadBuffer_common(smgr, permanent ? RELPERSISTENCE_PERMANENT : - RELPERSISTENCE_UNLOGGED, forkNum, blockNum, - mode, strategy, &hit); + return ReadBuffer_common(BMR_SMGR(smgr, permanent ? RELPERSISTENCE_PERMANENT : + RELPERSISTENCE_UNLOGGED), + forkNum, blockNum, + mode, strategy); } /* @@ -994,35 +990,68 @@ ExtendBufferedRelTo(BufferManagerRelation bmr, */ if (buffer == InvalidBuffer) { - bool hit; - Assert(extended_by == 0); - buffer = ReadBuffer_common(bmr.smgr, bmr.relpersistence, - fork, extend_to - 1, mode, strategy, - &hit); + buffer = ReadBuffer_common(bmr, fork, extend_to - 1, mode, strategy); } return buffer; } +/* + * Zero a buffer and lock it, as part of the implementation of + * RBM_ZERO_AND_LOCK or RBM_ZERO_AND_CLEANUP_LOCK. The buffer must be already + * pinned. It does not have to be valid, but it is valid and locked on + * return. + */ +static void +ZeroBuffer(Buffer buffer, ReadBufferMode mode) +{ + BufferDesc *bufHdr; + uint32 buf_state; + + Assert(mode == RBM_ZERO_AND_LOCK || mode == RBM_ZERO_AND_CLEANUP_LOCK); + + if (BufferIsLocal(buffer)) + bufHdr = GetLocalBufferDescriptor(-buffer - 1); + else + { + bufHdr = GetBufferDescriptor(buffer - 1); + if (mode == RBM_ZERO_AND_LOCK) + LockBuffer(buffer, BUFFER_LOCK_EXCLUSIVE); + else + LockBufferForCleanup(buffer); + } + + memset(BufferGetPage(buffer), 0, BLCKSZ); + + if (BufferIsLocal(buffer)) + { + buf_state = pg_atomic_read_u32(&bufHdr->state); + buf_state |= BM_VALID; + pg_atomic_unlocked_write_u32(&bufHdr->state, buf_state); + } + else + { + buf_state = LockBufHdr(bufHdr); + buf_state |= BM_VALID; + UnlockBufHdr(bufHdr, buf_state); + } +} + /* * ReadBuffer_common -- common logic for all ReadBuffer variants * * *hit is set to true if the request was satisfied from shared buffer cache. */ static Buffer -ReadBuffer_common(SMgrRelation smgr, char relpersistence, ForkNumber forkNum, +ReadBuffer_common(BufferManagerRelation bmr, ForkNumber forkNum, BlockNumber blockNum, ReadBufferMode mode, - BufferAccessStrategy strategy, bool *hit) + BufferAccessStrategy strategy) { - BufferDesc *bufHdr; - Block bufBlock; - bool found; - IOContext io_context; - IOObject io_object; - bool isLocalBuf = SmgrIsTemp(smgr); - - *hit = false; + ReadBuffersOperation operation; + Buffer buffer; + int nblocks; + int flags; /* * Backward compatibility path, most code should use ExtendBufferedRel() @@ -1041,181 +1070,405 @@ ReadBuffer_common(SMgrRelation smgr, char relpersistence, ForkNumber forkNum, if (mode == RBM_ZERO_AND_LOCK || mode == RBM_ZERO_AND_CLEANUP_LOCK) flags |= EB_LOCK_FIRST; - return ExtendBufferedRel(BMR_SMGR(smgr, relpersistence), - forkNum, strategy, flags); + return ExtendBufferedRel(bmr, forkNum, strategy, flags); } - TRACE_POSTGRESQL_BUFFER_READ_START(forkNum, blockNum, - smgr->smgr_rlocator.locator.spcOid, - smgr->smgr_rlocator.locator.dbOid, - smgr->smgr_rlocator.locator.relNumber, - smgr->smgr_rlocator.backend); + nblocks = 1; + if (mode == RBM_ZERO_ON_ERROR) + flags = READ_BUFFERS_ZERO_ON_ERROR; + else + flags = 0; + if (StartReadBuffers(bmr, + &buffer, + forkNum, + blockNum, + &nblocks, + strategy, + flags, + &operation)) + WaitReadBuffers(&operation); + Assert(nblocks == 1); /* single block can't be short */ + + if (mode == RBM_ZERO_AND_CLEANUP_LOCK || mode == RBM_ZERO_AND_LOCK) + ZeroBuffer(buffer, mode); + + return buffer; +} +static Buffer +PrepareReadBuffer(BufferManagerRelation bmr, + ForkNumber forkNum, + BlockNumber blockNum, + BufferAccessStrategy strategy, + bool *foundPtr) +{ + BufferDesc *bufHdr; + bool isLocalBuf; + IOContext io_context; + IOObject io_object; + + Assert(blockNum != P_NEW); + + Assert(bmr.smgr); + + isLocalBuf = SmgrIsTemp(bmr.smgr); if (isLocalBuf) { - /* - * We do not use a BufferAccessStrategy for I/O of temporary tables. - * However, in some cases, the "strategy" may not be NULL, so we can't - * rely on IOContextForStrategy() to set the right IOContext for us. - * This may happen in cases like CREATE TEMPORARY TABLE AS... - */ io_context = IOCONTEXT_NORMAL; io_object = IOOBJECT_TEMP_RELATION; - bufHdr = LocalBufferAlloc(smgr, forkNum, blockNum, &found); - if (found) - pgBufferUsage.local_blks_hit++; - else if (mode == RBM_NORMAL || mode == RBM_NORMAL_NO_LOG || - mode == RBM_ZERO_ON_ERROR) - pgBufferUsage.local_blks_read++; } else { - /* - * lookup the buffer. IO_IN_PROGRESS is set if the requested block is - * not currently in memory. - */ io_context = IOContextForStrategy(strategy); io_object = IOOBJECT_RELATION; - bufHdr = BufferAlloc(smgr, relpersistence, forkNum, blockNum, - strategy, &found, io_context); - if (found) - pgBufferUsage.shared_blks_hit++; - else if (mode == RBM_NORMAL || mode == RBM_NORMAL_NO_LOG || - mode == RBM_ZERO_ON_ERROR) - pgBufferUsage.shared_blks_read++; } - /* At this point we do NOT hold any locks. */ + TRACE_POSTGRESQL_BUFFER_READ_START(forkNum, blockNum, + bmr.smgr->smgr_rlocator.locator.spcOid, + bmr.smgr->smgr_rlocator.locator.dbOid, + bmr.smgr->smgr_rlocator.locator.relNumber, + bmr.smgr->smgr_rlocator.backend); - /* if it was already in the buffer pool, we're done */ - if (found) + ResourceOwnerEnlarge(CurrentResourceOwner); + if (isLocalBuf) + { + bufHdr = LocalBufferAlloc(bmr.smgr, forkNum, blockNum, foundPtr); + if (*foundPtr) + pgBufferUsage.local_blks_hit++; + } + else + { + bufHdr = BufferAlloc(bmr.smgr, bmr.relpersistence, forkNum, blockNum, + strategy, foundPtr, io_context); + if (*foundPtr) + pgBufferUsage.shared_blks_hit++; + } + if (bmr.rel) + { + /* + * While pgBufferUsage's "read" counter isn't bumped unless we reach + * WaitReadBuffers() (so, not for hits, and not for buffers that are + * zeroed instead), the per-relation stats always count them. + */ + pgstat_count_buffer_read(bmr.rel); + if (*foundPtr) + pgstat_count_buffer_hit(bmr.rel); + } + if (*foundPtr) { - /* Just need to update stats before we exit */ - *hit = true; VacuumPageHit++; pgstat_count_io_op(io_object, io_context, IOOP_HIT); - if (VacuumCostActive) VacuumCostBalance += VacuumCostPageHit; TRACE_POSTGRESQL_BUFFER_READ_DONE(forkNum, blockNum, - smgr->smgr_rlocator.locator.spcOid, - smgr->smgr_rlocator.locator.dbOid, - smgr->smgr_rlocator.locator.relNumber, - smgr->smgr_rlocator.backend, - found); + bmr.smgr->smgr_rlocator.locator.spcOid, + bmr.smgr->smgr_rlocator.locator.dbOid, + bmr.smgr->smgr_rlocator.locator.relNumber, + bmr.smgr->smgr_rlocator.backend, + true); + } - /* - * In RBM_ZERO_AND_LOCK mode the caller expects the page to be locked - * on return. - */ - if (!isLocalBuf) - { - if (mode == RBM_ZERO_AND_LOCK) - LWLockAcquire(BufferDescriptorGetContentLock(bufHdr), - LW_EXCLUSIVE); - else if (mode == RBM_ZERO_AND_CLEANUP_LOCK) - LockBufferForCleanup(BufferDescriptorGetBuffer(bufHdr)); - } + return BufferDescriptorGetBuffer(bufHdr); +} - return BufferDescriptorGetBuffer(bufHdr); +/* + * Begin reading a range of blocks beginning at blockNum and extending for + * *nblocks. On return, up to *nblocks pinned buffers holding those blocks + * are written into the buffers array, and *nblocks is updated to contain the + * actual number, which may be fewer than requested. + * + * If false is returned, no I/O is necessary and WaitReadBuffers() is not + * necessary. If true is returned, one I/O has been started, and + * WaitReadBuffers() must be called with the same operation object before the + * buffers are accessed. Along with the operation object, the caller-supplied + * array of buffers must remain valid until WaitReadBuffers() is called. + * + * Currently the I/O is only started with optional operating system advice, + * and the real I/O happens in WaitReadBuffers(). In future work, true I/O + * could be initiated here. + */ +bool +StartReadBuffers(BufferManagerRelation bmr, + Buffer *buffers, + ForkNumber forkNum, + BlockNumber blockNum, + int *nblocks, + BufferAccessStrategy strategy, + int flags, + ReadBuffersOperation *operation) +{ + int actual_nblocks = *nblocks; + + if (bmr.rel) + { + bmr.smgr = RelationGetSmgr(bmr.rel); + bmr.relpersistence = bmr.rel->rd_rel->relpersistence; } - /* - * if we have gotten to this point, we have allocated a buffer for the - * page but its contents are not yet valid. IO_IN_PROGRESS is set for it, - * if it's a shared buffer. - */ - Assert(!(pg_atomic_read_u32(&bufHdr->state) & BM_VALID)); /* spinlock not needed */ + operation->bmr = bmr; + operation->forknum = forkNum; + operation->blocknum = blockNum; + operation->buffers = buffers; + operation->nblocks = actual_nblocks; + operation->strategy = strategy; + operation->flags = flags; - bufBlock = isLocalBuf ? LocalBufHdrGetBlock(bufHdr) : BufHdrGetBlock(bufHdr); + operation->io_buffers_len = 0; - /* - * Read in the page, unless the caller intends to overwrite it and just - * wants us to allocate a buffer. - */ - if (mode == RBM_ZERO_AND_LOCK || mode == RBM_ZERO_AND_CLEANUP_LOCK) - MemSet((char *) bufBlock, 0, BLCKSZ); - else + for (int i = 0; i < actual_nblocks; ++i) { - instr_time io_start = pgstat_prepare_io_time(track_io_timing); + bool found; - smgrread(smgr, forkNum, blockNum, bufBlock); + buffers[i] = PrepareReadBuffer(bmr, + forkNum, + blockNum + i, + strategy, + &found); - pgstat_count_io_op_time(io_object, io_context, - IOOP_READ, io_start, 1); + if (found) + { + /* + * Terminate the read as soon as we get a hit. It could be a + * single buffer hit, or it could be a hit that follows a readable + * range. We don't want to create more than one readable range, + * so we stop here. + */ + actual_nblocks = operation->nblocks = *nblocks = i + 1; + break; + } + else + { + /* Extend the readable range to cover this block. */ + operation->io_buffers_len++; + } + } - /* check for garbage data */ - if (!PageIsVerifiedExtended((Page) bufBlock, blockNum, - PIV_LOG_WARNING | PIV_REPORT_STAT)) + if (operation->io_buffers_len > 0) + { + if (flags & READ_BUFFERS_ISSUE_ADVICE) { - if (mode == RBM_ZERO_ON_ERROR || zero_damaged_pages) - { - ereport(WARNING, - (errcode(ERRCODE_DATA_CORRUPTED), - errmsg("invalid page in block %u of relation %s; zeroing out page", - blockNum, - relpath(smgr->smgr_rlocator, forkNum)))); - MemSet((char *) bufBlock, 0, BLCKSZ); - } - else - ereport(ERROR, - (errcode(ERRCODE_DATA_CORRUPTED), - errmsg("invalid page in block %u of relation %s", - blockNum, - relpath(smgr->smgr_rlocator, forkNum)))); + /* + * In theory we should only do this if PrepareReadBuffers() had to + * allocate new buffers above. That way, if two calls to + * StartReadBuffers() were made for the same blocks before + * WaitReadBuffers(), only the first would issue the advice. + * That'd be a better simulation of true asynchronous I/O, which + * would only start the I/O once, but isn't done here for + * simplicity. Note also that the following call might actually + * issue two advice calls if we cross a segment boundary; in a + * true asynchronous version we might choose to process only one + * real I/O at a time in that case. + */ + smgrprefetch(bmr.smgr, forkNum, blockNum, operation->io_buffers_len); } + + /* Indicate that WaitReadBuffers() should be called. */ + return true; } + else + { + return false; + } +} - /* - * In RBM_ZERO_AND_LOCK / RBM_ZERO_AND_CLEANUP_LOCK mode, grab the buffer - * content lock before marking the page as valid, to make sure that no - * other backend sees the zeroed page before the caller has had a chance - * to initialize it. - * - * Since no-one else can be looking at the page contents yet, there is no - * difference between an exclusive lock and a cleanup-strength lock. (Note - * that we cannot use LockBuffer() or LockBufferForCleanup() here, because - * they assert that the buffer is already valid.) - */ - if ((mode == RBM_ZERO_AND_LOCK || mode == RBM_ZERO_AND_CLEANUP_LOCK) && - !isLocalBuf) +static inline bool +WaitReadBuffersCanStartIO(Buffer buffer, bool nowait) +{ + if (BufferIsLocal(buffer)) { - LWLockAcquire(BufferDescriptorGetContentLock(bufHdr), LW_EXCLUSIVE); + BufferDesc *bufHdr = GetLocalBufferDescriptor(-buffer - 1); + + return (pg_atomic_read_u32(&bufHdr->state) & BM_VALID) == 0; } + else + return StartBufferIO(GetBufferDescriptor(buffer - 1), true, nowait); +} + +void +WaitReadBuffers(ReadBuffersOperation *operation) +{ + BufferManagerRelation bmr; + Buffer *buffers; + int nblocks; + BlockNumber blocknum; + ForkNumber forknum; + bool isLocalBuf; + IOContext io_context; + IOObject io_object; + + /* + * Currently operations are only allowed to include a read of some range, + * with an optional extra buffer that is already pinned at the end. So + * nblocks can be at most one more than io_buffers_len. + */ + Assert((operation->nblocks == operation->io_buffers_len) || + (operation->nblocks == operation->io_buffers_len + 1)); + /* Find the range of the physical read we need to perform. */ + nblocks = operation->io_buffers_len; + if (nblocks == 0) + return; /* nothing to do */ + + buffers = &operation->buffers[0]; + blocknum = operation->blocknum; + forknum = operation->forknum; + bmr = operation->bmr; + + isLocalBuf = SmgrIsTemp(bmr.smgr); if (isLocalBuf) { - /* Only need to adjust flags */ - uint32 buf_state = pg_atomic_read_u32(&bufHdr->state); - - buf_state |= BM_VALID; - pg_atomic_unlocked_write_u32(&bufHdr->state, buf_state); + io_context = IOCONTEXT_NORMAL; + io_object = IOOBJECT_TEMP_RELATION; } else { - /* Set BM_VALID, terminate IO, and wake up any waiters */ - TerminateBufferIO(bufHdr, false, BM_VALID, true); + io_context = IOContextForStrategy(operation->strategy); + io_object = IOOBJECT_RELATION; } - VacuumPageMiss++; - if (VacuumCostActive) - VacuumCostBalance += VacuumCostPageMiss; + /* + * We count all these blocks as read by this backend. This is traditional + * behavior, but might turn out to be not true if we find that someone + * else has beaten us and completed the read of some of these blocks. In + * that case the system globally double-counts, but we traditionally don't + * count this as a "hit", and we don't have a separate counter for "miss, + * but another backend completed the read". + */ + if (isLocalBuf) + pgBufferUsage.local_blks_read += nblocks; + else + pgBufferUsage.shared_blks_read += nblocks; - TRACE_POSTGRESQL_BUFFER_READ_DONE(forkNum, blockNum, - smgr->smgr_rlocator.locator.spcOid, - smgr->smgr_rlocator.locator.dbOid, - smgr->smgr_rlocator.locator.relNumber, - smgr->smgr_rlocator.backend, - found); + for (int i = 0; i < nblocks; ++i) + { + int io_buffers_len; + Buffer io_buffers[MAX_BUFFERS_PER_TRANSFER]; + void *io_pages[MAX_BUFFERS_PER_TRANSFER]; + instr_time io_start; + BlockNumber io_first_block; - return BufferDescriptorGetBuffer(bufHdr); + /* + * Skip this block if someone else has already completed it. If an + * I/O is already in progress in another backend, this will wait for + * the outcome: either done, or something went wrong and we will + * retry. + */ + if (!WaitReadBuffersCanStartIO(buffers[i], false)) + { + /* + * Report this as a 'hit' for this backend, even though it must + * have started out as a miss in PrepareReadBuffer(). + */ + TRACE_POSTGRESQL_BUFFER_READ_DONE(forknum, blocknum + i, + bmr.smgr->smgr_rlocator.locator.spcOid, + bmr.smgr->smgr_rlocator.locator.dbOid, + bmr.smgr->smgr_rlocator.locator.relNumber, + bmr.smgr->smgr_rlocator.backend, + true); + continue; + } + + /* We found a buffer that we need to read in. */ + io_buffers[0] = buffers[i]; + io_pages[0] = BufferGetBlock(buffers[i]); + io_first_block = blocknum + i; + io_buffers_len = 1; + + /* + * How many neighboring-on-disk blocks can we can scatter-read into + * other buffers at the same time? In this case we don't wait if we + * see an I/O already in progress. We already hold BM_IO_IN_PROGRESS + * for the head block, so we should get on with that I/O as soon as + * possible. We'll come back to this block again, above. + */ + while ((i + 1) < nblocks && + WaitReadBuffersCanStartIO(buffers[i + 1], true)) + { + /* Must be consecutive block numbers. */ + Assert(BufferGetBlockNumber(buffers[i + 1]) == + BufferGetBlockNumber(buffers[i]) + 1); + + io_buffers[io_buffers_len] = buffers[++i]; + io_pages[io_buffers_len++] = BufferGetBlock(buffers[i]); + } + + io_start = pgstat_prepare_io_time(track_io_timing); + smgrreadv(bmr.smgr, forknum, io_first_block, io_pages, io_buffers_len); + pgstat_count_io_op_time(io_object, io_context, IOOP_READ, io_start, + io_buffers_len); + + /* Verify each block we read, and terminate the I/O. */ + for (int j = 0; j < io_buffers_len; ++j) + { + BufferDesc *bufHdr; + Block bufBlock; + + if (isLocalBuf) + { + bufHdr = GetLocalBufferDescriptor(-io_buffers[j] - 1); + bufBlock = LocalBufHdrGetBlock(bufHdr); + } + else + { + bufHdr = GetBufferDescriptor(io_buffers[j] - 1); + bufBlock = BufHdrGetBlock(bufHdr); + } + + /* check for garbage data */ + if (!PageIsVerifiedExtended((Page) bufBlock, io_first_block + j, + PIV_LOG_WARNING | PIV_REPORT_STAT)) + { + if ((operation->flags & READ_BUFFERS_ZERO_ON_ERROR) || zero_damaged_pages) + { + ereport(WARNING, + (errcode(ERRCODE_DATA_CORRUPTED), + errmsg("invalid page in block %u of relation %s; zeroing out page", + io_first_block + j, + relpath(bmr.smgr->smgr_rlocator, forknum)))); + memset(bufBlock, 0, BLCKSZ); + } + else + ereport(ERROR, + (errcode(ERRCODE_DATA_CORRUPTED), + errmsg("invalid page in block %u of relation %s", + io_first_block + j, + relpath(bmr.smgr->smgr_rlocator, forknum)))); + } + + /* Terminate I/O and set BM_VALID. */ + if (isLocalBuf) + { + uint32 buf_state = pg_atomic_read_u32(&bufHdr->state); + + buf_state |= BM_VALID; + pg_atomic_unlocked_write_u32(&bufHdr->state, buf_state); + } + else + { + /* Set BM_VALID, terminate IO, and wake up any waiters */ + TerminateBufferIO(bufHdr, false, BM_VALID, true); + } + + /* Report I/Os as completing individually. */ + TRACE_POSTGRESQL_BUFFER_READ_DONE(forknum, io_first_block + j, + bmr.smgr->smgr_rlocator.locator.spcOid, + bmr.smgr->smgr_rlocator.locator.dbOid, + bmr.smgr->smgr_rlocator.locator.relNumber, + bmr.smgr->smgr_rlocator.backend, + false); + } + + VacuumPageMiss += io_buffers_len; + if (VacuumCostActive) + VacuumCostBalance += VacuumCostPageMiss * io_buffers_len; + } } /* - * BufferAlloc -- subroutine for ReadBuffer. Handles lookup of a shared - * buffer. If no buffer exists already, selects a replacement - * victim and evicts the old page, but does NOT read in new page. + * BufferAlloc -- subroutine for StartReadBuffers. Handles lookup of a shared + * buffer. If no buffer exists already, selects a replacement victim and + * evicts the old page, but does NOT read in new page. * * "strategy" can be a buffer replacement strategy object, or NULL for * the default strategy. The selected buffer's usage_count is advanced when @@ -1223,11 +1476,7 @@ ReadBuffer_common(SMgrRelation smgr, char relpersistence, ForkNumber forkNum, * * The returned buffer is pinned and is already marked as holding the * desired page. If it already did have the desired page, *foundPtr is - * set true. Otherwise, *foundPtr is set false and the buffer is marked - * as IO_IN_PROGRESS; ReadBuffer will now need to do I/O to fill it. - * - * *foundPtr is actually redundant with the buffer's BM_VALID flag, but - * we keep it for simplicity in ReadBuffer. + * set true. Otherwise, *foundPtr is set false. * * io_context is passed as an output parameter to avoid calling * IOContextForStrategy() when there is a shared buffers hit and no IO @@ -1286,19 +1535,10 @@ BufferAlloc(SMgrRelation smgr, char relpersistence, ForkNumber forkNum, { /* * We can only get here if (a) someone else is still reading in - * the page, or (b) a previous read attempt failed. We have to - * wait for any active read attempt to finish, and then set up our - * own read attempt if the page is still not BM_VALID. - * StartBufferIO does it all. + * the page, (b) a previous read attempt failed, or (c) someone + * called StartReadBuffers() but not yet WaitReadBuffers(). */ - if (StartBufferIO(buf, true)) - { - /* - * If we get here, previous attempts to read the buffer must - * have failed ... but we shall bravely try again. - */ - *foundPtr = false; - } + *foundPtr = false; } return buf; @@ -1363,19 +1603,10 @@ BufferAlloc(SMgrRelation smgr, char relpersistence, ForkNumber forkNum, { /* * We can only get here if (a) someone else is still reading in - * the page, or (b) a previous read attempt failed. We have to - * wait for any active read attempt to finish, and then set up our - * own read attempt if the page is still not BM_VALID. - * StartBufferIO does it all. + * the page, (b) a previous read attempt failed, or (c) someone + * called StartReadBuffers() but not yet WaitReadBuffers(). */ - if (StartBufferIO(existing_buf_hdr, true)) - { - /* - * If we get here, previous attempts to read the buffer must - * have failed ... but we shall bravely try again. - */ - *foundPtr = false; - } + *foundPtr = false; } return existing_buf_hdr; @@ -1407,15 +1638,9 @@ BufferAlloc(SMgrRelation smgr, char relpersistence, ForkNumber forkNum, LWLockRelease(newPartitionLock); /* - * Buffer contents are currently invalid. Try to obtain the right to - * start I/O. If StartBufferIO returns false, then someone else managed - * to read it before we did, so there's nothing left for BufferAlloc() to - * do. + * Buffer contents are currently invalid. */ - if (StartBufferIO(victim_buf_hdr, true)) - *foundPtr = false; - else - *foundPtr = true; + *foundPtr = false; return victim_buf_hdr; } @@ -1769,7 +1994,7 @@ again: * pessimistic, but outside of toy-sized shared_buffers it should allow * sufficient pins. */ -static void +void LimitAdditionalPins(uint32 *additional_pins) { uint32 max_backends; @@ -2034,7 +2259,7 @@ ExtendBufferedRelShared(BufferManagerRelation bmr, buf_state &= ~BM_VALID; UnlockBufHdr(existing_hdr, buf_state); - } while (!StartBufferIO(existing_hdr, true)); + } while (!StartBufferIO(existing_hdr, true, false)); } else { @@ -2057,7 +2282,7 @@ ExtendBufferedRelShared(BufferManagerRelation bmr, LWLockRelease(partition_lock); /* XXX: could combine the locked operations in it with the above */ - StartBufferIO(victim_buf_hdr, true); + StartBufferIO(victim_buf_hdr, true, false); } } @@ -2372,7 +2597,12 @@ PinBuffer(BufferDesc *buf, BufferAccessStrategy strategy) else { /* - * If we previously pinned the buffer, it must surely be valid. + * If we previously pinned the buffer, it is likely to be valid, but + * it may not be if StartReadBuffers() was called and + * WaitReadBuffers() hasn't been called yet. We'll check by loading + * the flags without locking. This is racy, but it's OK to return + * false spuriously: when WaitReadBuffers() calls StartBufferIO(), + * it'll see that it's now valid. * * Note: We deliberately avoid a Valgrind client request here. * Individual access methods can optionally superimpose buffer page @@ -2381,7 +2611,7 @@ PinBuffer(BufferDesc *buf, BufferAccessStrategy strategy) * that the buffer page is legitimately non-accessible here. We * cannot meddle with that. */ - result = true; + result = (pg_atomic_read_u32(&buf->state) & BM_VALID) != 0; } ref->refcount++; @@ -3449,7 +3679,7 @@ FlushBuffer(BufferDesc *buf, SMgrRelation reln, IOObject io_object, * someone else flushed the buffer before we could, so we need not do * anything. */ - if (!StartBufferIO(buf, false)) + if (!StartBufferIO(buf, false, false)) return; /* Setup error traceback support for ereport() */ @@ -5184,9 +5414,15 @@ WaitIO(BufferDesc *buf) * * Returns true if we successfully marked the buffer as I/O busy, * false if someone else already did the work. + * + * If nowait is true, then we don't wait for an I/O to be finished by another + * backend. In that case, false indicates either that the I/O was already + * finished, or is still in progress. This is useful for callers that want to + * find out if they can perform the I/O as part of a larger operation, without + * waiting for the answer or distinguishing the reasons why not. */ static bool -StartBufferIO(BufferDesc *buf, bool forInput) +StartBufferIO(BufferDesc *buf, bool forInput, bool nowait) { uint32 buf_state; @@ -5199,6 +5435,8 @@ StartBufferIO(BufferDesc *buf, bool forInput) if (!(buf_state & BM_IO_IN_PROGRESS)) break; UnlockBufHdr(buf, buf_state); + if (nowait) + return false; WaitIO(buf); } diff --git a/src/backend/storage/buffer/localbuf.c b/src/backend/storage/buffer/localbuf.c index fcfac335a57..985a2c7049c 100644 --- a/src/backend/storage/buffer/localbuf.c +++ b/src/backend/storage/buffer/localbuf.c @@ -108,10 +108,9 @@ PrefetchLocalBuffer(SMgrRelation smgr, ForkNumber forkNum, * LocalBufferAlloc - * Find or create a local buffer for the given page of the given relation. * - * API is similar to bufmgr.c's BufferAlloc, except that we do not need - * to do any locking since this is all local. Also, IO_IN_PROGRESS - * does not get set. Lastly, we support only default access strategy - * (hence, usage_count is always advanced). + * API is similar to bufmgr.c's BufferAlloc, except that we do not need to do + * any locking since this is all local. We support only default access + * strategy (hence, usage_count is always advanced). */ BufferDesc * LocalBufferAlloc(SMgrRelation smgr, ForkNumber forkNum, BlockNumber blockNum, @@ -287,7 +286,7 @@ GetLocalVictimBuffer(void) } /* see LimitAdditionalPins() */ -static void +void LimitAdditionalLocalPins(uint32 *additional_pins) { uint32 max_pins; @@ -297,9 +296,10 @@ LimitAdditionalLocalPins(uint32 *additional_pins) /* * In contrast to LimitAdditionalPins() other backends don't play a role - * here. We can allow up to NLocBuffer pins in total. + * here. We can allow up to NLocBuffer pins in total, but it might not be + * initialized yet so read num_temp_buffers. */ - max_pins = (NLocBuffer - NLocalPinnedBuffers); + max_pins = (num_temp_buffers - NLocalPinnedBuffers); if (*additional_pins >= max_pins) *additional_pins = max_pins; diff --git a/src/backend/storage/meson.build b/src/backend/storage/meson.build index 40345bdca27..739d13293fb 100644 --- a/src/backend/storage/meson.build +++ b/src/backend/storage/meson.build @@ -1,5 +1,6 @@ # Copyright (c) 2022-2024, PostgreSQL Global Development Group +subdir('aio') subdir('buffer') subdir('file') subdir('freespace') diff --git a/src/include/storage/bufmgr.h b/src/include/storage/bufmgr.h index d51d46d3353..b57f71f97e3 100644 --- a/src/include/storage/bufmgr.h +++ b/src/include/storage/bufmgr.h @@ -14,6 +14,7 @@ #ifndef BUFMGR_H #define BUFMGR_H +#include "port/pg_iovec.h" #include "storage/block.h" #include "storage/buf.h" #include "storage/bufpage.h" @@ -158,6 +159,11 @@ extern PGDLLIMPORT int32 *LocalRefCount; #define BUFFER_LOCK_SHARE 1 #define BUFFER_LOCK_EXCLUSIVE 2 +/* + * Maximum number of buffers for multi-buffer I/O functions. This is set to + * allow 128kB transfers, unless BLCKSZ and IOV_MAX imply a a smaller maximum. + */ +#define MAX_BUFFERS_PER_TRANSFER Min(PG_IOV_MAX, (128 * 1024) / BLCKSZ) /* * prototypes for functions in bufmgr.c @@ -177,6 +183,42 @@ extern Buffer ReadBufferWithoutRelcache(RelFileLocator rlocator, ForkNumber forkNum, BlockNumber blockNum, ReadBufferMode mode, BufferAccessStrategy strategy, bool permanent); + +#define READ_BUFFERS_ZERO_ON_ERROR 0x01 +#define READ_BUFFERS_ISSUE_ADVICE 0x02 + +/* + * Private state used by StartReadBuffers() and WaitReadBuffers(). Declared + * in public header only to allow inclusion in other structs, but contents + * should not be accessed. + */ +struct ReadBuffersOperation +{ + /* Parameters passed in to StartReadBuffers(). */ + BufferManagerRelation bmr; + Buffer *buffers; + ForkNumber forknum; + BlockNumber blocknum; + int nblocks; + BufferAccessStrategy strategy; + int flags; + + /* Range of buffers, if we need to perform a read. */ + int io_buffers_len; +}; + +typedef struct ReadBuffersOperation ReadBuffersOperation; + +extern bool StartReadBuffers(BufferManagerRelation bmr, + Buffer *buffers, + ForkNumber forknum, + BlockNumber blocknum, + int *nblocks, + BufferAccessStrategy strategy, + int flags, + ReadBuffersOperation *operation); +extern void WaitReadBuffers(ReadBuffersOperation *operation); + extern void ReleaseBuffer(Buffer buffer); extern void UnlockReleaseBuffer(Buffer buffer); extern bool BufferIsExclusiveLocked(Buffer buffer); @@ -250,6 +292,9 @@ extern bool HoldingBufferPinThatDelaysRecovery(void); extern bool BgBufferSync(struct WritebackContext *wb_context); +extern void LimitAdditionalPins(uint32 *additional_pins); +extern void LimitAdditionalLocalPins(uint32 *additional_pins); + /* in buf_init.c */ extern void InitBufferPool(void); extern Size BufferShmemSize(void); diff --git a/src/include/storage/streaming_read.h b/src/include/storage/streaming_read.h new file mode 100644 index 00000000000..c4d3892bb26 --- /dev/null +++ b/src/include/storage/streaming_read.h @@ -0,0 +1,52 @@ +#ifndef STREAMING_READ_H +#define STREAMING_READ_H + +#include "storage/bufmgr.h" +#include "storage/fd.h" +#include "storage/smgr.h" + +/* Default tuning, reasonable for many users. */ +#define PGSR_FLAG_DEFAULT 0x00 + +/* + * I/O streams that are performing maintenance work on behalf of potentially + * many users. + */ +#define PGSR_FLAG_MAINTENANCE 0x01 + +/* + * We usually avoid issuing prefetch advice automatically when sequential + * access is detected, but this flag explicitly disables it, for cases that + * might not be correctly detected. Explicit advice is known to perform worse + * than letting the kernel (at least Linux) detect sequential access. + */ +#define PGSR_FLAG_SEQUENTIAL 0x02 + +/* + * We usually ramp up from smaller reads to larger ones, to support users who + * don't know if it's worth reading lots of buffers yet. This flag disables + * that, declaring ahead of time that we'll be reading all available buffers. + */ +#define PGSR_FLAG_FULL 0x04 + +struct PgStreamingRead; +typedef struct PgStreamingRead PgStreamingRead; + +/* Callback that returns the next block number to read. */ +typedef BlockNumber (*PgStreamingReadBufferCB) (PgStreamingRead *pgsr, + void *pgsr_private, + void *per_buffer_private); + +extern PgStreamingRead *pg_streaming_read_buffer_alloc(int flags, + void *pgsr_private, + size_t per_buffer_private_size, + BufferAccessStrategy strategy, + BufferManagerRelation bmr, + ForkNumber forknum, + PgStreamingReadBufferCB next_block_cb); + +extern void pg_streaming_read_prefetch(PgStreamingRead *pgsr); +extern Buffer pg_streaming_read_buffer_get_next(PgStreamingRead *pgsr, void **per_buffer_private); +extern void pg_streaming_read_free(PgStreamingRead *pgsr); + +#endif diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index aa7a25b8f8c..8b5c2a1576d 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -2099,6 +2099,8 @@ PgStat_TableCounts PgStat_TableStatus PgStat_TableXactStatus PgStat_WalStats +PgStreamingRead +PgStreamingReadRange PgXmlErrorContext PgXmlStrictness Pg_finfo_record @@ -2269,6 +2271,7 @@ ReInitializeDSMForeignScan_function ReScanForeignScan_function ReadBufPtrType ReadBufferMode +ReadBuffersOperation ReadBytePtrType ReadExtraTocPtrType ReadFunc -- 2.40.1 --w4wcjcocxsm37usi Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v6-0014-BitmapHeapScan-uses-streaming-read-API.patch" ^ permalink raw reply [nested|flat] 5+ messages in thread
* [PATCH v16 2/2] Move dynamically-allocated tranche names to shared memory. @ 2025-08-27 20:13 Nathan Bossart <[email protected]> 0 siblings, 0 replies; 5+ messages in thread From: Nathan Bossart @ 2025-08-27 20:13 UTC (permalink / raw) --- contrib/pg_prewarm/autoprewarm.c | 3 +- doc/src/sgml/xfunc.sgml | 15 +- src/backend/postmaster/launch_backend.c | 6 +- src/backend/storage/ipc/dsm_registry.c | 12 +- src/backend/storage/lmgr/lwlock.c | 179 ++++++++---------- src/include/storage/lwlock.h | 24 +-- src/test/modules/test_dsa/test_dsa.c | 6 +- .../test_dsm_registry/test_dsm_registry.c | 3 +- .../modules/test_radixtree/test_radixtree.c | 9 +- src/test/modules/test_slru/test_slru.c | 6 +- .../modules/test_tidstore/test_tidstore.c | 3 +- 11 files changed, 99 insertions(+), 167 deletions(-) diff --git a/contrib/pg_prewarm/autoprewarm.c b/contrib/pg_prewarm/autoprewarm.c index c01b9c7e6a4..880e897796a 100644 --- a/contrib/pg_prewarm/autoprewarm.c +++ b/contrib/pg_prewarm/autoprewarm.c @@ -864,7 +864,7 @@ apw_init_state(void *ptr) { AutoPrewarmSharedState *state = (AutoPrewarmSharedState *) ptr; - LWLockInitialize(&state->lock, LWLockNewTrancheId()); + LWLockInitialize(&state->lock, LWLockNewTrancheId("autoprewarm")); state->bgworker_pid = InvalidPid; state->pid_using_dumpfile = InvalidPid; } @@ -883,7 +883,6 @@ apw_init_shmem(void) sizeof(AutoPrewarmSharedState), apw_init_state, &found); - LWLockRegisterTranche(apw_state->lock.tranche, "autoprewarm"); return found; } diff --git a/doc/src/sgml/xfunc.sgml b/doc/src/sgml/xfunc.sgml index f116d0648e5..da21ef56891 100644 --- a/doc/src/sgml/xfunc.sgml +++ b/doc/src/sgml/xfunc.sgml @@ -3759,7 +3759,7 @@ LWLockPadded *GetNamedLWLockTranche(const char *tranche_name) <literal>shmem_request_hook</literal>. To do so, first allocate a <literal>tranche_id</literal> by calling: <programlisting> -int LWLockNewTrancheId(void) +int LWLockNewTrancheId(const char *name) </programlisting> Next, initialize each LWLock, passing the new <literal>tranche_id</literal> as an argument: @@ -3777,17 +3777,8 @@ void LWLockInitialize(LWLock *lock, int tranche_id) </para> <para> - Finally, each backend using the <literal>tranche_id</literal> should - associate it with a <literal>tranche_name</literal> by calling: -<programlisting> -void LWLockRegisterTranche(int tranche_id, const char *tranche_name) -</programlisting> - </para> - - <para> - A complete usage example of <function>LWLockNewTrancheId</function>, - <function>LWLockInitialize</function>, and - <function>LWLockRegisterTranche</function> can be found in + A complete usage example of <function>LWLockNewTrancheId</function> and + <function>LWLockInitialize</function> can be found in <filename>contrib/pg_prewarm/autoprewarm.c</filename> in the <productname>PostgreSQL</productname> source tree. </para> diff --git a/src/backend/postmaster/launch_backend.c b/src/backend/postmaster/launch_backend.c index bf6b55ee830..cbb44344b5a 100644 --- a/src/backend/postmaster/launch_backend.c +++ b/src/backend/postmaster/launch_backend.c @@ -101,7 +101,7 @@ typedef struct struct InjectionPointsCtl *ActiveInjectionPoints; #endif int NamedLWLockTrancheRequests; - NamedLWLockTranche *NamedLWLockTrancheArray; + char **NamedLWLockTrancheNames; LWLockPadded *MainLWLockArray; slock_t *ProcStructLock; PROC_HDR *ProcGlobal; @@ -760,7 +760,7 @@ save_backend_variables(BackendParameters *param, #endif param->NamedLWLockTrancheRequests = NamedLWLockTrancheRequests; - param->NamedLWLockTrancheArray = NamedLWLockTrancheArray; + param->NamedLWLockTrancheNames = NamedLWLockTrancheNames; param->MainLWLockArray = MainLWLockArray; param->ProcStructLock = ProcStructLock; param->ProcGlobal = ProcGlobal; @@ -1020,7 +1020,7 @@ restore_backend_variables(BackendParameters *param) #endif NamedLWLockTrancheRequests = param->NamedLWLockTrancheRequests; - NamedLWLockTrancheArray = param->NamedLWLockTrancheArray; + NamedLWLockTrancheNames = param->NamedLWLockTrancheNames; MainLWLockArray = param->MainLWLockArray; ProcStructLock = param->ProcStructLock; ProcGlobal = param->ProcGlobal; diff --git a/src/backend/storage/ipc/dsm_registry.c b/src/backend/storage/ipc/dsm_registry.c index 332796465ff..8999dee6761 100644 --- a/src/backend/storage/ipc/dsm_registry.c +++ b/src/backend/storage/ipc/dsm_registry.c @@ -299,8 +299,7 @@ GetNamedDSA(const char *name, bool *found) entry->type = DSMR_ENTRY_TYPE_DSA; /* Initialize the LWLock tranche for the DSA. */ - state->tranche = LWLockNewTrancheId(); - LWLockRegisterTranche(state->tranche, name); + state->tranche = LWLockNewTrancheId(name); /* Initialize the DSA. */ ret = dsa_create(state->tranche); @@ -321,9 +320,6 @@ GetNamedDSA(const char *name, bool *found) ereport(ERROR, (errmsg("requested DSA already attached to current process"))); - /* Initialize existing LWLock tranche for the DSA. */ - LWLockRegisterTranche(state->tranche, name); - /* Attach to existing DSA. */ ret = dsa_attach(state->handle); dsa_pin_mapping(ret); @@ -377,8 +373,7 @@ GetNamedDSHash(const char *name, const dshash_parameters *params, bool *found) entry->type = DSMR_ENTRY_TYPE_DSH; /* Initialize the LWLock tranche for the hash table. */ - dsh_state->tranche = LWLockNewTrancheId(); - LWLockRegisterTranche(dsh_state->tranche, name); + dsh_state->tranche = LWLockNewTrancheId(name); /* Initialize the DSA for the hash table. */ dsa = dsa_create(dsh_state->tranche); @@ -408,9 +403,6 @@ GetNamedDSHash(const char *name, const dshash_parameters *params, bool *found) ereport(ERROR, (errmsg("requested DSHash already attached to current process"))); - /* Initialize existing LWLock tranche for the hash table. */ - LWLockRegisterTranche(dsh_state->tranche, name); - /* Attach to existing DSA for the hash table. */ dsa = dsa_attach(dsh_state->dsa_handle); dsa_pin_mapping(dsa); diff --git a/src/backend/storage/lmgr/lwlock.c b/src/backend/storage/lmgr/lwlock.c index c80b43f1f55..39f17c387f3 100644 --- a/src/backend/storage/lmgr/lwlock.c +++ b/src/backend/storage/lmgr/lwlock.c @@ -126,8 +126,8 @@ StaticAssertDecl((LW_VAL_EXCLUSIVE & LW_FLAG_MASK) == 0, * in lwlocklist.h. We absorb the names of these tranches, too. * * 3. Extensions can create new tranches, via either RequestNamedLWLockTranche - * or LWLockRegisterTranche. The names of these that are known in the current - * process appear in LWLockTrancheNames[]. + * or LWLockNewTrancheId. These names are stored in shared memory and can be + * accessed via NamedLWLockTrancheNames. * * All these names are user-visible as wait event names, so choose with care * ... and do not forget to update the documentation's list of wait events. @@ -146,11 +146,12 @@ StaticAssertDecl(lengthof(BuiltinTrancheNames) == /* * This is indexed by tranche ID minus LWTRANCHE_FIRST_USER_DEFINED, and - * stores the names of all dynamically-created tranches known to the current - * process. Any unused entries in the array will contain NULL. + * points to the shared memory locations of the names of all + * dynamically-created tranches. Backends inherit the pointer by fork from the + * postmaster (except in the EXEC_BACKEND case, where we have special measures + * to pass it down). */ -static const char **LWLockTrancheNames = NULL; -static int LWLockTrancheNamesAllocated = 0; +char **NamedLWLockTrancheNames = NULL; /* * This points to the main array of LWLocks in shared memory. Backends inherit @@ -184,18 +185,18 @@ typedef struct NamedLWLockTrancheRequest } NamedLWLockTrancheRequest; static NamedLWLockTrancheRequest *NamedLWLockTrancheRequestArray = NULL; -static int NamedLWLockTrancheRequestsAllocated = 0; /* - * NamedLWLockTrancheRequests is both the valid length of the request array, - * and the length of the shared-memory NamedLWLockTrancheArray later on. - * This variable and NamedLWLockTrancheArray are non-static so that - * postmaster.c can copy them to child processes in EXEC_BACKEND builds. + * NamedLWLockTrancheRequests is the valid length of the request array. This + * variable is non-static so that postmaster.c can copy them to child processes + * in EXEC_BACKEND builds. */ int NamedLWLockTrancheRequests = 0; -/* points to data in shared memory: */ -NamedLWLockTranche *NamedLWLockTrancheArray = NULL; +/* backend-local counter of registered tranches */ +static int LocalLWLockCounter; + +#define MAX_NAMED_TRANCHES 256 static void InitializeLWLocks(void); static inline void LWLockReportWaitStart(LWLock *lock); @@ -391,7 +392,6 @@ Size LWLockShmemSize(void) { Size size; - int i; int numLocks = NUM_FIXED_LWLOCKS; /* Calculate total number of locks needed in the main array. */ @@ -404,18 +404,15 @@ LWLockShmemSize(void) size = add_size(size, sizeof(int) + LWLOCK_PADDED_SIZE); /* space for named tranches. */ - size = add_size(size, mul_size(NamedLWLockTrancheRequests, sizeof(NamedLWLockTranche))); - - /* space for name of each tranche. */ - for (i = 0; i < NamedLWLockTrancheRequests; i++) - size = add_size(size, strlen(NamedLWLockTrancheRequestArray[i].tranche_name) + 1); + size = add_size(size, mul_size(MAX_NAMED_TRANCHES, sizeof(char *))); + size = add_size(size, mul_size(MAX_NAMED_TRANCHES, NAMEDATALEN)); return size; } /* * Allocate shmem space for the main LWLock array and all tranches and - * initialize it. We also register extension LWLock tranches here. + * initialize it. */ void CreateLWLocks(void) @@ -429,6 +426,15 @@ CreateLWLocks(void) /* Allocate space */ ptr = (char *) ShmemAlloc(spaceLocks); + /* Initialize tranche names */ + NamedLWLockTrancheNames = (char **) ptr; + ptr += MAX_NAMED_TRANCHES * sizeof(char *); + for (int i = 0; i < MAX_NAMED_TRANCHES; i++) + { + NamedLWLockTrancheNames[i] = ptr; + ptr += NAMEDATALEN; + } + /* Leave room for dynamic allocation of tranches */ ptr += sizeof(int); @@ -447,11 +453,6 @@ CreateLWLocks(void) /* Initialize all LWLocks */ InitializeLWLocks(); } - - /* Register named extension LWLock tranches in the current process. */ - for (int i = 0; i < NamedLWLockTrancheRequests; i++) - LWLockRegisterTranche(NamedLWLockTrancheArray[i].trancheId, - NamedLWLockTrancheArray[i].trancheName); } /* @@ -460,7 +461,6 @@ CreateLWLocks(void) static void InitializeLWLocks(void) { - int numNamedLocks = NumLWLocksForNamedTranches(); int id; int i; int j; @@ -491,32 +491,18 @@ InitializeLWLocks(void) */ if (NamedLWLockTrancheRequests > 0) { - char *trancheNames; - - NamedLWLockTrancheArray = (NamedLWLockTranche *) - &MainLWLockArray[NUM_FIXED_LWLOCKS + numNamedLocks]; - - trancheNames = (char *) NamedLWLockTrancheArray + - (NamedLWLockTrancheRequests * sizeof(NamedLWLockTranche)); lock = &MainLWLockArray[NUM_FIXED_LWLOCKS]; for (i = 0; i < NamedLWLockTrancheRequests; i++) { NamedLWLockTrancheRequest *request; - NamedLWLockTranche *tranche; - char *name; + int tranche; request = &NamedLWLockTrancheRequestArray[i]; - tranche = &NamedLWLockTrancheArray[i]; - - name = trancheNames; - trancheNames += strlen(request->tranche_name) + 1; - strcpy(name, request->tranche_name); - tranche->trancheId = LWLockNewTrancheId(); - tranche->trancheName = name; + tranche = LWLockNewTrancheId(request->tranche_name); for (j = 0; j < request->num_lwlocks; j++, lock++) - LWLockInitialize(&lock->lock, tranche->trancheId); + LWLockInitialize(&lock->lock, tranche); } } } @@ -568,61 +554,41 @@ GetNamedLWLockTranche(const char *tranche_name) } /* - * Allocate a new tranche ID. + * Allocate a new tranche ID with the provided name. */ int -LWLockNewTrancheId(void) +LWLockNewTrancheId(const char *name) { int result; int *LWLockCounter; + if (strlen(name) >= NAMEDATALEN) + ereport(ERROR, + (errcode(ERRCODE_NAME_TOO_LONG), + errmsg("tranche name too long"), + errdetail("LWLock tranche name must be less than %d bytes.", + NAMEDATALEN))); + LWLockCounter = (int *) ((char *) MainLWLockArray - sizeof(int)); + /* We use the ShmemLock spinlock to protect LWLockCounter */ SpinLockAcquire(ShmemLock); - result = (*LWLockCounter)++; - SpinLockRelease(ShmemLock); - - return result; -} - -/* - * Register a dynamic tranche name in the lookup table of the current process. - * - * This routine will save a pointer to the tranche name passed as an argument, - * so the name should be allocated in a backend-lifetime context - * (shared memory, TopMemoryContext, static constant, or similar). - * - * The tranche name will be user-visible as a wait event name, so try to - * use a name that fits the style for those. - */ -void -LWLockRegisterTranche(int tranche_id, const char *tranche_name) -{ - /* This should only be called for user-defined tranches. */ - if (tranche_id < LWTRANCHE_FIRST_USER_DEFINED) - return; - /* Convert to array index. */ - tranche_id -= LWTRANCHE_FIRST_USER_DEFINED; - - /* If necessary, create or enlarge array. */ - if (tranche_id >= LWLockTrancheNamesAllocated) + if (*LWLockCounter - LWTRANCHE_FIRST_USER_DEFINED >= MAX_NAMED_TRANCHES) { - int newalloc; + SpinLockRelease(ShmemLock); + ereport(ERROR, + (errmsg("maximum number of tranches already registered"), + errdetail("At most %d tranches may be registered.", + MAX_NAMED_TRANCHES))); + } - newalloc = pg_nextpower2_32(Max(8, tranche_id + 1)); + result = LocalLWLockCounter = (*LWLockCounter)++; + strcpy(NamedLWLockTrancheNames[result - LWTRANCHE_FIRST_USER_DEFINED], name); - if (LWLockTrancheNames == NULL) - LWLockTrancheNames = (const char **) - MemoryContextAllocZero(TopMemoryContext, - newalloc * sizeof(char *)); - else - LWLockTrancheNames = - repalloc0_array(LWLockTrancheNames, const char *, LWLockTrancheNamesAllocated, newalloc); - LWLockTrancheNamesAllocated = newalloc; - } + SpinLockRelease(ShmemLock); - LWLockTrancheNames[tranche_id] = tranche_name; + return result; } /* @@ -647,22 +613,17 @@ RequestNamedLWLockTranche(const char *tranche_name, int num_lwlocks) if (NamedLWLockTrancheRequestArray == NULL) { - NamedLWLockTrancheRequestsAllocated = 16; NamedLWLockTrancheRequestArray = (NamedLWLockTrancheRequest *) MemoryContextAlloc(TopMemoryContext, - NamedLWLockTrancheRequestsAllocated + MAX_NAMED_TRANCHES * sizeof(NamedLWLockTrancheRequest)); } - if (NamedLWLockTrancheRequests >= NamedLWLockTrancheRequestsAllocated) - { - int i = pg_nextpower2_32(NamedLWLockTrancheRequests + 1); - - NamedLWLockTrancheRequestArray = (NamedLWLockTrancheRequest *) - repalloc(NamedLWLockTrancheRequestArray, - i * sizeof(NamedLWLockTrancheRequest)); - NamedLWLockTrancheRequestsAllocated = i; - } + if (NamedLWLockTrancheRequests >= MAX_NAMED_TRANCHES) + ereport(ERROR, + (errmsg("maximum number of tranches already registered"), + errdetail("At most %d tranches may be registered.", + MAX_NAMED_TRANCHES))); request = &NamedLWLockTrancheRequestArray[NamedLWLockTrancheRequests]; Assert(strlen(tranche_name) + 1 <= NAMEDATALEN); @@ -677,6 +638,9 @@ RequestNamedLWLockTranche(const char *tranche_name, int num_lwlocks) void LWLockInitialize(LWLock *lock, int tranche_id) { + /* verify the tranche_id is valid */ + (void) GetLWTrancheName(tranche_id); + pg_atomic_init_u32(&lock->state, LW_FLAG_RELEASE_OK); #ifdef LOCK_DEBUG pg_atomic_init_u32(&lock->nwaiters, 0); @@ -717,18 +681,27 @@ GetLWTrancheName(uint16 trancheId) if (trancheId < LWTRANCHE_FIRST_USER_DEFINED) return BuiltinTrancheNames[trancheId]; + /* verify the trancheId is valid */ + if (trancheId >= LocalLWLockCounter) + { + int *LWLockCounter; + + LWLockCounter = (int *) ((char *) MainLWLockArray - sizeof(int)); + + SpinLockAcquire(ShmemLock); + LocalLWLockCounter = *LWLockCounter; + SpinLockRelease(ShmemLock); + + if (trancheId >= LocalLWLockCounter) + elog(ERROR, "tranche %d is not registered", trancheId); + } + /* - * It's an extension tranche, so look in LWLockTrancheNames[]. However, - * it's possible that the tranche has never been registered in the current - * process, in which case give up and return "extension". + * It's an extension tranche, so look in NamedLWLockTrancheNames. */ trancheId -= LWTRANCHE_FIRST_USER_DEFINED; - if (trancheId >= LWLockTrancheNamesAllocated || - LWLockTrancheNames[trancheId] == NULL) - return "extension"; - - return LWLockTrancheNames[trancheId]; + return NamedLWLockTrancheNames[trancheId]; } /* diff --git a/src/include/storage/lwlock.h b/src/include/storage/lwlock.h index 5e717765764..9f9c4c7b5ca 100644 --- a/src/include/storage/lwlock.h +++ b/src/include/storage/lwlock.h @@ -73,14 +73,7 @@ typedef union LWLockPadded extern PGDLLIMPORT LWLockPadded *MainLWLockArray; -/* struct for storing named tranche information */ -typedef struct NamedLWLockTranche -{ - int trancheId; - char *trancheName; -} NamedLWLockTranche; - -extern PGDLLIMPORT NamedLWLockTranche *NamedLWLockTrancheArray; +extern PGDLLIMPORT char **NamedLWLockTrancheNames; extern PGDLLIMPORT int NamedLWLockTrancheRequests; /* @@ -157,18 +150,11 @@ extern LWLockPadded *GetNamedLWLockTranche(const char *tranche_name); /* * There is another, more flexible method of obtaining lwlocks. First, call - * LWLockNewTrancheId just once to obtain a tranche ID; this allocates from - * a shared counter. Next, each individual process using the tranche should - * call LWLockRegisterTranche() to associate that tranche ID with a name. - * Finally, LWLockInitialize should be called just once per lwlock, passing - * the tranche ID as an argument. - * - * It may seem strange that each process using the tranche must register it - * separately, but dynamic shared memory segments aren't guaranteed to be - * mapped at the same address in all coordinating backends, so storing the - * registration in the main shared memory segment wouldn't work for that case. + * LWLockNewTrancheId to obtain a tranche ID; this allocates from a shared + * counter. Second, LWLockInitialize should be called just once per lwlock, + * passing the tranche ID as an argument. */ -extern int LWLockNewTrancheId(void); +extern int LWLockNewTrancheId(const char *name); extern void LWLockRegisterTranche(int tranche_id, const char *tranche_name); extern void LWLockInitialize(LWLock *lock, int tranche_id); diff --git a/src/test/modules/test_dsa/test_dsa.c b/src/test/modules/test_dsa/test_dsa.c index cd24d0f4873..01d5c6fa67f 100644 --- a/src/test/modules/test_dsa/test_dsa.c +++ b/src/test/modules/test_dsa/test_dsa.c @@ -29,8 +29,7 @@ test_dsa_basic(PG_FUNCTION_ARGS) dsa_pointer p[100]; /* XXX: this tranche is leaked */ - tranche_id = LWLockNewTrancheId(); - LWLockRegisterTranche(tranche_id, "test_dsa"); + tranche_id = LWLockNewTrancheId("test_dsa"); a = dsa_create(tranche_id); for (int i = 0; i < 100; i++) @@ -70,8 +69,7 @@ test_dsa_resowners(PG_FUNCTION_ARGS) ResourceOwner childowner; /* XXX: this tranche is leaked */ - tranche_id = LWLockNewTrancheId(); - LWLockRegisterTranche(tranche_id, "test_dsa"); + tranche_id = LWLockNewTrancheId("test_dsa"); /* Create DSA in parent resource owner */ a = dsa_create(tranche_id); diff --git a/src/test/modules/test_dsm_registry/test_dsm_registry.c b/src/test/modules/test_dsm_registry/test_dsm_registry.c index 141c8ed1b34..4cc2ccdac3f 100644 --- a/src/test/modules/test_dsm_registry/test_dsm_registry.c +++ b/src/test/modules/test_dsm_registry/test_dsm_registry.c @@ -48,7 +48,7 @@ init_tdr_dsm(void *ptr) { TestDSMRegistryStruct *dsm = (TestDSMRegistryStruct *) ptr; - LWLockInitialize(&dsm->lck, LWLockNewTrancheId()); + LWLockInitialize(&dsm->lck, LWLockNewTrancheId("test_dsm_registry")); dsm->val = 0; } @@ -61,7 +61,6 @@ tdr_attach_shmem(void) sizeof(TestDSMRegistryStruct), init_tdr_dsm, &found); - LWLockRegisterTranche(tdr_dsm->lck.tranche, "test_dsm_registry"); if (tdr_dsa == NULL) tdr_dsa = GetNamedDSA("test_dsm_registry_dsa", &found); diff --git a/src/test/modules/test_radixtree/test_radixtree.c b/src/test/modules/test_radixtree/test_radixtree.c index 80ad0296164..787162c8793 100644 --- a/src/test/modules/test_radixtree/test_radixtree.c +++ b/src/test/modules/test_radixtree/test_radixtree.c @@ -124,10 +124,9 @@ test_empty(void) rt_iter *iter; uint64 key; #ifdef TEST_SHARED_RT - int tranche_id = LWLockNewTrancheId(); + int tranche_id = LWLockNewTrancheId("test_radix_tree"); dsa_area *dsa; - LWLockRegisterTranche(tranche_id, "test_radix_tree"); dsa = dsa_create(tranche_id); radixtree = rt_create(dsa, tranche_id); #else @@ -167,10 +166,9 @@ test_basic(rt_node_class_test_elem *test_info, int shift, bool asc) uint64 *keys; int children = test_info->nkeys; #ifdef TEST_SHARED_RT - int tranche_id = LWLockNewTrancheId(); + int tranche_id = LWLockNewTrancheId("test_radix_tree"); dsa_area *dsa; - LWLockRegisterTranche(tranche_id, "test_radix_tree"); dsa = dsa_create(tranche_id); radixtree = rt_create(dsa, tranche_id); #else @@ -304,10 +302,9 @@ test_random(void) int num_keys = 100000; uint64 *keys; #ifdef TEST_SHARED_RT - int tranche_id = LWLockNewTrancheId(); + int tranche_id = LWLockNewTrancheId("test_radix_tree"); dsa_area *dsa; - LWLockRegisterTranche(tranche_id, "test_radix_tree"); dsa = dsa_create(tranche_id); radixtree = rt_create(dsa, tranche_id); #else diff --git a/src/test/modules/test_slru/test_slru.c b/src/test/modules/test_slru/test_slru.c index 32750930e43..8c0367eeee4 100644 --- a/src/test/modules/test_slru/test_slru.c +++ b/src/test/modules/test_slru/test_slru.c @@ -232,11 +232,9 @@ test_slru_shmem_startup(void) (void) MakePGDirectory(slru_dir_name); /* initialize the SLRU facility */ - test_tranche_id = LWLockNewTrancheId(); - LWLockRegisterTranche(test_tranche_id, "test_slru_tranche"); + test_tranche_id = LWLockNewTrancheId("test_slru_tranche"); - test_buffer_tranche_id = LWLockNewTrancheId(); - LWLockRegisterTranche(test_tranche_id, "test_buffer_tranche"); + test_buffer_tranche_id = LWLockNewTrancheId("test_buffer_tranche"); TestSlruCtl->PagePrecedes = test_slru_page_precedes_logically; SimpleLruInit(TestSlruCtl, "TestSLRU", diff --git a/src/test/modules/test_tidstore/test_tidstore.c b/src/test/modules/test_tidstore/test_tidstore.c index eb16e0fbfa6..0c8f43867e5 100644 --- a/src/test/modules/test_tidstore/test_tidstore.c +++ b/src/test/modules/test_tidstore/test_tidstore.c @@ -103,8 +103,7 @@ test_create(PG_FUNCTION_ARGS) { int tranche_id; - tranche_id = LWLockNewTrancheId(); - LWLockRegisterTranche(tranche_id, "test_tidstore"); + tranche_id = LWLockNewTrancheId("test_tidstore"); tidstore = TidStoreCreateShared(tidstore_max_size, tranche_id); -- 2.39.5 (Apple Git-154) --PIgUBUNQ6mYWmpJO-- ^ permalink raw reply [nested|flat] 5+ messages in thread
* [PATCH v19 1/2] Move dynamically-allocated tranche names to shared memory. @ 2025-08-27 20:13 Nathan Bossart <[email protected]> 0 siblings, 0 replies; 5+ messages in thread From: Nathan Bossart @ 2025-08-27 20:13 UTC (permalink / raw) --- contrib/pg_prewarm/autoprewarm.c | 3 +- doc/src/sgml/xfunc.sgml | 15 +- src/backend/postmaster/launch_backend.c | 6 +- src/backend/storage/ipc/dsm_registry.c | 12 +- src/backend/storage/lmgr/lwlock.c | 193 ++++++++---------- src/include/storage/lwlock.h | 24 +-- src/test/modules/test_dsa/test_dsa.c | 6 +- .../test_dsm_registry/test_dsm_registry.c | 3 +- .../modules/test_radixtree/test_radixtree.c | 9 +- src/test/modules/test_slru/test_slru.c | 6 +- .../modules/test_tidstore/test_tidstore.c | 3 +- 11 files changed, 113 insertions(+), 167 deletions(-) diff --git a/contrib/pg_prewarm/autoprewarm.c b/contrib/pg_prewarm/autoprewarm.c index c01b9c7e6a4..880e897796a 100644 --- a/contrib/pg_prewarm/autoprewarm.c +++ b/contrib/pg_prewarm/autoprewarm.c @@ -864,7 +864,7 @@ apw_init_state(void *ptr) { AutoPrewarmSharedState *state = (AutoPrewarmSharedState *) ptr; - LWLockInitialize(&state->lock, LWLockNewTrancheId()); + LWLockInitialize(&state->lock, LWLockNewTrancheId("autoprewarm")); state->bgworker_pid = InvalidPid; state->pid_using_dumpfile = InvalidPid; } @@ -883,7 +883,6 @@ apw_init_shmem(void) sizeof(AutoPrewarmSharedState), apw_init_state, &found); - LWLockRegisterTranche(apw_state->lock.tranche, "autoprewarm"); return found; } diff --git a/doc/src/sgml/xfunc.sgml b/doc/src/sgml/xfunc.sgml index f116d0648e5..da21ef56891 100644 --- a/doc/src/sgml/xfunc.sgml +++ b/doc/src/sgml/xfunc.sgml @@ -3759,7 +3759,7 @@ LWLockPadded *GetNamedLWLockTranche(const char *tranche_name) <literal>shmem_request_hook</literal>. To do so, first allocate a <literal>tranche_id</literal> by calling: <programlisting> -int LWLockNewTrancheId(void) +int LWLockNewTrancheId(const char *name) </programlisting> Next, initialize each LWLock, passing the new <literal>tranche_id</literal> as an argument: @@ -3777,17 +3777,8 @@ void LWLockInitialize(LWLock *lock, int tranche_id) </para> <para> - Finally, each backend using the <literal>tranche_id</literal> should - associate it with a <literal>tranche_name</literal> by calling: -<programlisting> -void LWLockRegisterTranche(int tranche_id, const char *tranche_name) -</programlisting> - </para> - - <para> - A complete usage example of <function>LWLockNewTrancheId</function>, - <function>LWLockInitialize</function>, and - <function>LWLockRegisterTranche</function> can be found in + A complete usage example of <function>LWLockNewTrancheId</function> and + <function>LWLockInitialize</function> can be found in <filename>contrib/pg_prewarm/autoprewarm.c</filename> in the <productname>PostgreSQL</productname> source tree. </para> diff --git a/src/backend/postmaster/launch_backend.c b/src/backend/postmaster/launch_backend.c index cd9547b03a3..de1b06df10d 100644 --- a/src/backend/postmaster/launch_backend.c +++ b/src/backend/postmaster/launch_backend.c @@ -101,7 +101,7 @@ typedef struct struct InjectionPointsCtl *ActiveInjectionPoints; #endif int NamedLWLockTrancheRequests; - NamedLWLockTranche *NamedLWLockTrancheArray; + char **NamedLWLockTrancheNames; int *LWLockCounter; LWLockPadded *MainLWLockArray; slock_t *ProcStructLock; @@ -761,7 +761,7 @@ save_backend_variables(BackendParameters *param, #endif param->NamedLWLockTrancheRequests = NamedLWLockTrancheRequests; - param->NamedLWLockTrancheArray = NamedLWLockTrancheArray; + param->NamedLWLockTrancheNames = NamedLWLockTrancheNames; param->LWLockCounter = LWLockCounter; param->MainLWLockArray = MainLWLockArray; param->ProcStructLock = ProcStructLock; @@ -1022,7 +1022,7 @@ restore_backend_variables(BackendParameters *param) #endif NamedLWLockTrancheRequests = param->NamedLWLockTrancheRequests; - NamedLWLockTrancheArray = param->NamedLWLockTrancheArray; + NamedLWLockTrancheNames = param->NamedLWLockTrancheNames; LWLockCounter = param->LWLockCounter; MainLWLockArray = param->MainLWLockArray; ProcStructLock = param->ProcStructLock; diff --git a/src/backend/storage/ipc/dsm_registry.c b/src/backend/storage/ipc/dsm_registry.c index ca12815f4a8..97130925106 100644 --- a/src/backend/storage/ipc/dsm_registry.c +++ b/src/backend/storage/ipc/dsm_registry.c @@ -299,8 +299,7 @@ GetNamedDSA(const char *name, bool *found) entry->type = DSMR_ENTRY_TYPE_DSA; /* Initialize the LWLock tranche for the DSA. */ - state->tranche = LWLockNewTrancheId(); - LWLockRegisterTranche(state->tranche, name); + state->tranche = LWLockNewTrancheId(name); /* Initialize the DSA. */ ret = dsa_create(state->tranche); @@ -321,9 +320,6 @@ GetNamedDSA(const char *name, bool *found) ereport(ERROR, (errmsg("requested DSA already attached to current process"))); - /* Initialize existing LWLock tranche for the DSA. */ - LWLockRegisterTranche(state->tranche, name); - /* Attach to existing DSA. */ ret = dsa_attach(state->handle); dsa_pin_mapping(ret); @@ -378,8 +374,7 @@ GetNamedDSHash(const char *name, const dshash_parameters *params, bool *found) entry->type = DSMR_ENTRY_TYPE_DSH; /* Initialize the LWLock tranche for the hash table. */ - dsh_state->tranche = LWLockNewTrancheId(); - LWLockRegisterTranche(dsh_state->tranche, name); + dsh_state->tranche = LWLockNewTrancheId(name); /* Initialize the DSA for the hash table. */ dsa = dsa_create(dsh_state->tranche); @@ -409,9 +404,6 @@ GetNamedDSHash(const char *name, const dshash_parameters *params, bool *found) ereport(ERROR, (errmsg("requested DSHash already attached to current process"))); - /* Initialize existing LWLock tranche for the hash table. */ - LWLockRegisterTranche(dsh_state->tranche, name); - /* Attach to existing DSA for the hash table. */ dsa = dsa_attach(dsh_state->dsa_handle); dsa_pin_mapping(dsa); diff --git a/src/backend/storage/lmgr/lwlock.c b/src/backend/storage/lmgr/lwlock.c index a4aecd1fbc3..0e30dc1ba03 100644 --- a/src/backend/storage/lmgr/lwlock.c +++ b/src/backend/storage/lmgr/lwlock.c @@ -126,8 +126,8 @@ StaticAssertDecl((LW_VAL_EXCLUSIVE & LW_FLAG_MASK) == 0, * in lwlocklist.h. We absorb the names of these tranches, too. * * 3. Extensions can create new tranches, via either RequestNamedLWLockTranche - * or LWLockRegisterTranche. The names of these that are known in the current - * process appear in LWLockTrancheNames[]. + * or LWLockNewTrancheId. These names are stored in shared memory and can be + * accessed via NamedLWLockTrancheNames. * * All these names are user-visible as wait event names, so choose with care * ... and do not forget to update the documentation's list of wait events. @@ -146,11 +146,12 @@ StaticAssertDecl(lengthof(BuiltinTrancheNames) == /* * This is indexed by tranche ID minus LWTRANCHE_FIRST_USER_DEFINED, and - * stores the names of all dynamically-created tranches known to the current - * process. Any unused entries in the array will contain NULL. + * points to the shared memory locations of the names of all + * dynamically-created tranches. Backends inherit the pointer by fork from the + * postmaster (except in the EXEC_BACKEND case, where we have special measures + * to pass it down). */ -static const char **LWLockTrancheNames = NULL; -static int LWLockTrancheNamesAllocated = 0; +char **NamedLWLockTrancheNames = NULL; /* * This points to the main array of LWLocks in shared memory. Backends inherit @@ -184,20 +185,22 @@ typedef struct NamedLWLockTrancheRequest } NamedLWLockTrancheRequest; static NamedLWLockTrancheRequest *NamedLWLockTrancheRequestArray = NULL; -static int NamedLWLockTrancheRequestsAllocated = 0; /* - * NamedLWLockTrancheRequests is both the valid length of the request array, - * and the length of the shared-memory NamedLWLockTrancheArray later on. - * This variable and NamedLWLockTrancheArray are non-static so that - * postmaster.c can copy them to child processes in EXEC_BACKEND builds. + * NamedLWLockTrancheRequests is the valid length of the request array. This + * variable is non-static so that postmaster.c can copy them to child processes + * in EXEC_BACKEND builds. */ int NamedLWLockTrancheRequests = 0; /* points to data in shared memory: */ -NamedLWLockTranche *NamedLWLockTrancheArray = NULL; int *LWLockCounter = NULL; +/* backend-local counter of registered tranches */ +static int LocalLWLockCounter; + +#define MAX_NAMED_TRANCHES 256 + static void InitializeLWLocks(void); static inline void LWLockReportWaitStart(LWLock *lock); static inline void LWLockReportWaitEnd(void); @@ -392,7 +395,6 @@ Size LWLockShmemSize(void) { Size size; - int i; int numLocks = NUM_FIXED_LWLOCKS; /* Calculate total number of locks needed in the main array. */ @@ -405,18 +407,15 @@ LWLockShmemSize(void) size = add_size(size, mul_size(numLocks, sizeof(LWLockPadded))); /* space for named tranches. */ - size = add_size(size, mul_size(NamedLWLockTrancheRequests, sizeof(NamedLWLockTranche))); - - /* space for name of each tranche. */ - for (i = 0; i < NamedLWLockTrancheRequests; i++) - size = add_size(size, strlen(NamedLWLockTrancheRequestArray[i].tranche_name) + 1); + size = add_size(size, mul_size(MAX_NAMED_TRANCHES, sizeof(char *))); + size = add_size(size, mul_size(MAX_NAMED_TRANCHES, NAMEDATALEN)); return size; } /* * Allocate shmem space for the main LWLock array and all tranches and - * initialize it. We also register extension LWLock tranches here. + * initialize it. */ void CreateLWLocks(void) @@ -434,6 +433,15 @@ CreateLWLocks(void) *LWLockCounter = LWTRANCHE_FIRST_USER_DEFINED; ptr += sizeof(int); + /* Initialize tranche names */ + NamedLWLockTrancheNames = (char **) ptr; + ptr += MAX_NAMED_TRANCHES * sizeof(char *); + for (int i = 0; i < MAX_NAMED_TRANCHES; i++) + { + NamedLWLockTrancheNames[i] = ptr; + ptr += NAMEDATALEN; + } + /* Ensure desired alignment of LWLock array */ ptr += LWLOCK_PADDED_SIZE - ((uintptr_t) ptr) % LWLOCK_PADDED_SIZE; MainLWLockArray = (LWLockPadded *) ptr; @@ -441,11 +449,6 @@ CreateLWLocks(void) /* Initialize all LWLocks */ InitializeLWLocks(); } - - /* Register named extension LWLock tranches in the current process. */ - for (int i = 0; i < NamedLWLockTrancheRequests; i++) - LWLockRegisterTranche(NamedLWLockTrancheArray[i].trancheId, - NamedLWLockTrancheArray[i].trancheName); } /* @@ -454,7 +457,6 @@ CreateLWLocks(void) static void InitializeLWLocks(void) { - int numNamedLocks = NumLWLocksForNamedTranches(); int id; int i; int j; @@ -485,32 +487,18 @@ InitializeLWLocks(void) */ if (NamedLWLockTrancheRequests > 0) { - char *trancheNames; - - NamedLWLockTrancheArray = (NamedLWLockTranche *) - &MainLWLockArray[NUM_FIXED_LWLOCKS + numNamedLocks]; - - trancheNames = (char *) NamedLWLockTrancheArray + - (NamedLWLockTrancheRequests * sizeof(NamedLWLockTranche)); lock = &MainLWLockArray[NUM_FIXED_LWLOCKS]; for (i = 0; i < NamedLWLockTrancheRequests; i++) { NamedLWLockTrancheRequest *request; - NamedLWLockTranche *tranche; - char *name; + int tranche; request = &NamedLWLockTrancheRequestArray[i]; - tranche = &NamedLWLockTrancheArray[i]; - - name = trancheNames; - trancheNames += strlen(request->tranche_name) + 1; - strcpy(name, request->tranche_name); - tranche->trancheId = LWLockNewTrancheId(); - tranche->trancheName = name; + tranche = LWLockNewTrancheId(request->tranche_name); for (j = 0; j < request->num_lwlocks; j++, lock++) - LWLockInitialize(&lock->lock, tranche->trancheId); + LWLockInitialize(&lock->lock, tranche); } } } @@ -562,59 +550,44 @@ GetNamedLWLockTranche(const char *tranche_name) } /* - * Allocate a new tranche ID. + * Allocate a new tranche ID with the provided name. */ int -LWLockNewTrancheId(void) +LWLockNewTrancheId(const char *name) { int result; - /* We use the ShmemLock spinlock to protect LWLockCounter */ - SpinLockAcquire(ShmemLock); - result = (*LWLockCounter)++; - SpinLockRelease(ShmemLock); + if (!name) + ereport(ERROR, + (errcode(ERRCODE_INVALID_NAME), + errmsg("tranche name cannot be NULL"))); - return result; -} + if (strlen(name) >= NAMEDATALEN) + ereport(ERROR, + (errcode(ERRCODE_NAME_TOO_LONG), + errmsg("tranche name too long"), + errdetail("LWLock tranche names must be no longer than %d bytes.", + NAMEDATALEN - 1))); -/* - * Register a dynamic tranche name in the lookup table of the current process. - * - * This routine will save a pointer to the tranche name passed as an argument, - * so the name should be allocated in a backend-lifetime context - * (shared memory, TopMemoryContext, static constant, or similar). - * - * The tranche name will be user-visible as a wait event name, so try to - * use a name that fits the style for those. - */ -void -LWLockRegisterTranche(int tranche_id, const char *tranche_name) -{ - /* This should only be called for user-defined tranches. */ - if (tranche_id < LWTRANCHE_FIRST_USER_DEFINED) - return; - - /* Convert to array index. */ - tranche_id -= LWTRANCHE_FIRST_USER_DEFINED; + /* We use the ShmemLock spinlock to protect LWLockCounter */ + SpinLockAcquire(ShmemLock); - /* If necessary, create or enlarge array. */ - if (tranche_id >= LWLockTrancheNamesAllocated) + if (*LWLockCounter - LWTRANCHE_FIRST_USER_DEFINED >= MAX_NAMED_TRANCHES) { - int newalloc; + SpinLockRelease(ShmemLock); + ereport(ERROR, + (errmsg("maximum number of tranches already registered"), + errdetail("No more than %d tranches may be registered.", + MAX_NAMED_TRANCHES))); + } - newalloc = pg_nextpower2_32(Max(8, tranche_id + 1)); + result = (*LWLockCounter)++; + LocalLWLockCounter = *LWLockCounter; + strlcpy(NamedLWLockTrancheNames[result - LWTRANCHE_FIRST_USER_DEFINED], name, NAMEDATALEN); - if (LWLockTrancheNames == NULL) - LWLockTrancheNames = (const char **) - MemoryContextAllocZero(TopMemoryContext, - newalloc * sizeof(char *)); - else - LWLockTrancheNames = - repalloc0_array(LWLockTrancheNames, const char *, LWLockTrancheNamesAllocated, newalloc); - LWLockTrancheNamesAllocated = newalloc; - } + SpinLockRelease(ShmemLock); - LWLockTrancheNames[tranche_id] = tranche_name; + return result; } /* @@ -637,27 +610,33 @@ RequestNamedLWLockTranche(const char *tranche_name, int num_lwlocks) if (!process_shmem_requests_in_progress) elog(FATAL, "cannot request additional LWLocks outside shmem_request_hook"); + if (!tranche_name) + ereport(ERROR, + (errcode(ERRCODE_INVALID_NAME), + errmsg("tranche name cannot be NULL"))); + + if (strlen(tranche_name) >= NAMEDATALEN) + ereport(ERROR, + (errcode(ERRCODE_NAME_TOO_LONG), + errmsg("tranche name too long"), + errdetail("LWLock tranche names must be no longer than %d bytes.", + NAMEDATALEN - 1))); + if (NamedLWLockTrancheRequestArray == NULL) { - NamedLWLockTrancheRequestsAllocated = 16; NamedLWLockTrancheRequestArray = (NamedLWLockTrancheRequest *) MemoryContextAlloc(TopMemoryContext, - NamedLWLockTrancheRequestsAllocated + MAX_NAMED_TRANCHES * sizeof(NamedLWLockTrancheRequest)); } - if (NamedLWLockTrancheRequests >= NamedLWLockTrancheRequestsAllocated) - { - int i = pg_nextpower2_32(NamedLWLockTrancheRequests + 1); - - NamedLWLockTrancheRequestArray = (NamedLWLockTrancheRequest *) - repalloc(NamedLWLockTrancheRequestArray, - i * sizeof(NamedLWLockTrancheRequest)); - NamedLWLockTrancheRequestsAllocated = i; - } + if (NamedLWLockTrancheRequests >= MAX_NAMED_TRANCHES) + ereport(ERROR, + (errmsg("maximum number of tranches already registered"), + errdetail("No more than %d tranches may be registered.", + MAX_NAMED_TRANCHES))); request = &NamedLWLockTrancheRequestArray[NamedLWLockTrancheRequests]; - Assert(strlen(tranche_name) + 1 <= NAMEDATALEN); strlcpy(request->tranche_name, tranche_name, NAMEDATALEN); request->num_lwlocks = num_lwlocks; NamedLWLockTrancheRequests++; @@ -669,6 +648,9 @@ RequestNamedLWLockTranche(const char *tranche_name, int num_lwlocks) void LWLockInitialize(LWLock *lock, int tranche_id) { + /* verify the tranche_id is valid */ + (void) GetLWTrancheName(tranche_id); + pg_atomic_init_u32(&lock->state, LW_FLAG_RELEASE_OK); #ifdef LOCK_DEBUG pg_atomic_init_u32(&lock->nwaiters, 0); @@ -709,18 +691,23 @@ GetLWTrancheName(uint16 trancheId) if (trancheId < LWTRANCHE_FIRST_USER_DEFINED) return BuiltinTrancheNames[trancheId]; + /* verify the trancheId is valid */ + if (trancheId >= LocalLWLockCounter) + { + SpinLockAcquire(ShmemLock); + LocalLWLockCounter = *LWLockCounter; + SpinLockRelease(ShmemLock); + + if (trancheId >= LocalLWLockCounter) + elog(ERROR, "tranche %d is not registered", trancheId); + } + /* - * It's an extension tranche, so look in LWLockTrancheNames[]. However, - * it's possible that the tranche has never been registered in the current - * process, in which case give up and return "extension". + * It's an extension tranche, so look in NamedLWLockTrancheNames. */ trancheId -= LWTRANCHE_FIRST_USER_DEFINED; - if (trancheId >= LWLockTrancheNamesAllocated || - LWLockTrancheNames[trancheId] == NULL) - return "extension"; - - return LWLockTrancheNames[trancheId]; + return NamedLWLockTrancheNames[trancheId]; } /* diff --git a/src/include/storage/lwlock.h b/src/include/storage/lwlock.h index f9cf57f8d26..3877aaa7f03 100644 --- a/src/include/storage/lwlock.h +++ b/src/include/storage/lwlock.h @@ -73,14 +73,7 @@ typedef union LWLockPadded extern PGDLLIMPORT LWLockPadded *MainLWLockArray; -/* struct for storing named tranche information */ -typedef struct NamedLWLockTranche -{ - int trancheId; - char *trancheName; -} NamedLWLockTranche; - -extern PGDLLIMPORT NamedLWLockTranche *NamedLWLockTrancheArray; +extern PGDLLIMPORT char **NamedLWLockTrancheNames; extern PGDLLIMPORT int NamedLWLockTrancheRequests; extern PGDLLIMPORT int *LWLockCounter; @@ -158,18 +151,11 @@ extern LWLockPadded *GetNamedLWLockTranche(const char *tranche_name); /* * There is another, more flexible method of obtaining lwlocks. First, call - * LWLockNewTrancheId just once to obtain a tranche ID; this allocates from - * a shared counter. Next, each individual process using the tranche should - * call LWLockRegisterTranche() to associate that tranche ID with a name. - * Finally, LWLockInitialize should be called just once per lwlock, passing - * the tranche ID as an argument. - * - * It may seem strange that each process using the tranche must register it - * separately, but dynamic shared memory segments aren't guaranteed to be - * mapped at the same address in all coordinating backends, so storing the - * registration in the main shared memory segment wouldn't work for that case. + * LWLockNewTrancheId to obtain a tranche ID; this allocates from a shared + * counter. Second, LWLockInitialize should be called just once per lwlock, + * passing the tranche ID as an argument. */ -extern int LWLockNewTrancheId(void); +extern int LWLockNewTrancheId(const char *name); extern void LWLockRegisterTranche(int tranche_id, const char *tranche_name); extern void LWLockInitialize(LWLock *lock, int tranche_id); diff --git a/src/test/modules/test_dsa/test_dsa.c b/src/test/modules/test_dsa/test_dsa.c index cd24d0f4873..01d5c6fa67f 100644 --- a/src/test/modules/test_dsa/test_dsa.c +++ b/src/test/modules/test_dsa/test_dsa.c @@ -29,8 +29,7 @@ test_dsa_basic(PG_FUNCTION_ARGS) dsa_pointer p[100]; /* XXX: this tranche is leaked */ - tranche_id = LWLockNewTrancheId(); - LWLockRegisterTranche(tranche_id, "test_dsa"); + tranche_id = LWLockNewTrancheId("test_dsa"); a = dsa_create(tranche_id); for (int i = 0; i < 100; i++) @@ -70,8 +69,7 @@ test_dsa_resowners(PG_FUNCTION_ARGS) ResourceOwner childowner; /* XXX: this tranche is leaked */ - tranche_id = LWLockNewTrancheId(); - LWLockRegisterTranche(tranche_id, "test_dsa"); + tranche_id = LWLockNewTrancheId("test_dsa"); /* Create DSA in parent resource owner */ a = dsa_create(tranche_id); diff --git a/src/test/modules/test_dsm_registry/test_dsm_registry.c b/src/test/modules/test_dsm_registry/test_dsm_registry.c index 141c8ed1b34..4cc2ccdac3f 100644 --- a/src/test/modules/test_dsm_registry/test_dsm_registry.c +++ b/src/test/modules/test_dsm_registry/test_dsm_registry.c @@ -48,7 +48,7 @@ init_tdr_dsm(void *ptr) { TestDSMRegistryStruct *dsm = (TestDSMRegistryStruct *) ptr; - LWLockInitialize(&dsm->lck, LWLockNewTrancheId()); + LWLockInitialize(&dsm->lck, LWLockNewTrancheId("test_dsm_registry")); dsm->val = 0; } @@ -61,7 +61,6 @@ tdr_attach_shmem(void) sizeof(TestDSMRegistryStruct), init_tdr_dsm, &found); - LWLockRegisterTranche(tdr_dsm->lck.tranche, "test_dsm_registry"); if (tdr_dsa == NULL) tdr_dsa = GetNamedDSA("test_dsm_registry_dsa", &found); diff --git a/src/test/modules/test_radixtree/test_radixtree.c b/src/test/modules/test_radixtree/test_radixtree.c index 80ad0296164..787162c8793 100644 --- a/src/test/modules/test_radixtree/test_radixtree.c +++ b/src/test/modules/test_radixtree/test_radixtree.c @@ -124,10 +124,9 @@ test_empty(void) rt_iter *iter; uint64 key; #ifdef TEST_SHARED_RT - int tranche_id = LWLockNewTrancheId(); + int tranche_id = LWLockNewTrancheId("test_radix_tree"); dsa_area *dsa; - LWLockRegisterTranche(tranche_id, "test_radix_tree"); dsa = dsa_create(tranche_id); radixtree = rt_create(dsa, tranche_id); #else @@ -167,10 +166,9 @@ test_basic(rt_node_class_test_elem *test_info, int shift, bool asc) uint64 *keys; int children = test_info->nkeys; #ifdef TEST_SHARED_RT - int tranche_id = LWLockNewTrancheId(); + int tranche_id = LWLockNewTrancheId("test_radix_tree"); dsa_area *dsa; - LWLockRegisterTranche(tranche_id, "test_radix_tree"); dsa = dsa_create(tranche_id); radixtree = rt_create(dsa, tranche_id); #else @@ -304,10 +302,9 @@ test_random(void) int num_keys = 100000; uint64 *keys; #ifdef TEST_SHARED_RT - int tranche_id = LWLockNewTrancheId(); + int tranche_id = LWLockNewTrancheId("test_radix_tree"); dsa_area *dsa; - LWLockRegisterTranche(tranche_id, "test_radix_tree"); dsa = dsa_create(tranche_id); radixtree = rt_create(dsa, tranche_id); #else diff --git a/src/test/modules/test_slru/test_slru.c b/src/test/modules/test_slru/test_slru.c index 32750930e43..8c0367eeee4 100644 --- a/src/test/modules/test_slru/test_slru.c +++ b/src/test/modules/test_slru/test_slru.c @@ -232,11 +232,9 @@ test_slru_shmem_startup(void) (void) MakePGDirectory(slru_dir_name); /* initialize the SLRU facility */ - test_tranche_id = LWLockNewTrancheId(); - LWLockRegisterTranche(test_tranche_id, "test_slru_tranche"); + test_tranche_id = LWLockNewTrancheId("test_slru_tranche"); - test_buffer_tranche_id = LWLockNewTrancheId(); - LWLockRegisterTranche(test_tranche_id, "test_buffer_tranche"); + test_buffer_tranche_id = LWLockNewTrancheId("test_buffer_tranche"); TestSlruCtl->PagePrecedes = test_slru_page_precedes_logically; SimpleLruInit(TestSlruCtl, "TestSLRU", diff --git a/src/test/modules/test_tidstore/test_tidstore.c b/src/test/modules/test_tidstore/test_tidstore.c index eb16e0fbfa6..0c8f43867e5 100644 --- a/src/test/modules/test_tidstore/test_tidstore.c +++ b/src/test/modules/test_tidstore/test_tidstore.c @@ -103,8 +103,7 @@ test_create(PG_FUNCTION_ARGS) { int tranche_id; - tranche_id = LWLockNewTrancheId(); - LWLockRegisterTranche(tranche_id, "test_tidstore"); + tranche_id = LWLockNewTrancheId("test_tidstore"); tidstore = TidStoreCreateShared(tidstore_max_size, tranche_id); -- 2.39.5 (Apple Git-154) --gKvEFVVTDjQ/d7cm Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename=v19-0002-Tests-for-LWLock-tranche-registration-improvemen.patch ^ permalink raw reply [nested|flat] 5+ messages in thread
* [PATCH v20 1/2] Move dynamically-allocated tranche names to shared memory. @ 2025-08-27 20:13 Nathan Bossart <[email protected]> 0 siblings, 0 replies; 5+ messages in thread From: Nathan Bossart @ 2025-08-27 20:13 UTC (permalink / raw) --- contrib/pg_prewarm/autoprewarm.c | 3 +- doc/src/sgml/xfunc.sgml | 15 +- src/backend/postmaster/launch_backend.c | 6 +- src/backend/storage/ipc/dsm_registry.c | 12 +- src/backend/storage/lmgr/lwlock.c | 206 +++++++++--------- src/include/storage/lwlock.h | 24 +- src/test/modules/test_dsa/test_dsa.c | 6 +- .../test_dsm_registry/test_dsm_registry.c | 3 +- .../modules/test_radixtree/test_radixtree.c | 9 +- src/test/modules/test_slru/test_slru.c | 6 +- .../modules/test_tidstore/test_tidstore.c | 3 +- 11 files changed, 120 insertions(+), 173 deletions(-) diff --git a/contrib/pg_prewarm/autoprewarm.c b/contrib/pg_prewarm/autoprewarm.c index c01b9c7e6a4..880e897796a 100644 --- a/contrib/pg_prewarm/autoprewarm.c +++ b/contrib/pg_prewarm/autoprewarm.c @@ -864,7 +864,7 @@ apw_init_state(void *ptr) { AutoPrewarmSharedState *state = (AutoPrewarmSharedState *) ptr; - LWLockInitialize(&state->lock, LWLockNewTrancheId()); + LWLockInitialize(&state->lock, LWLockNewTrancheId("autoprewarm")); state->bgworker_pid = InvalidPid; state->pid_using_dumpfile = InvalidPid; } @@ -883,7 +883,6 @@ apw_init_shmem(void) sizeof(AutoPrewarmSharedState), apw_init_state, &found); - LWLockRegisterTranche(apw_state->lock.tranche, "autoprewarm"); return found; } diff --git a/doc/src/sgml/xfunc.sgml b/doc/src/sgml/xfunc.sgml index f116d0648e5..da21ef56891 100644 --- a/doc/src/sgml/xfunc.sgml +++ b/doc/src/sgml/xfunc.sgml @@ -3759,7 +3759,7 @@ LWLockPadded *GetNamedLWLockTranche(const char *tranche_name) <literal>shmem_request_hook</literal>. To do so, first allocate a <literal>tranche_id</literal> by calling: <programlisting> -int LWLockNewTrancheId(void) +int LWLockNewTrancheId(const char *name) </programlisting> Next, initialize each LWLock, passing the new <literal>tranche_id</literal> as an argument: @@ -3777,17 +3777,8 @@ void LWLockInitialize(LWLock *lock, int tranche_id) </para> <para> - Finally, each backend using the <literal>tranche_id</literal> should - associate it with a <literal>tranche_name</literal> by calling: -<programlisting> -void LWLockRegisterTranche(int tranche_id, const char *tranche_name) -</programlisting> - </para> - - <para> - A complete usage example of <function>LWLockNewTrancheId</function>, - <function>LWLockInitialize</function>, and - <function>LWLockRegisterTranche</function> can be found in + A complete usage example of <function>LWLockNewTrancheId</function> and + <function>LWLockInitialize</function> can be found in <filename>contrib/pg_prewarm/autoprewarm.c</filename> in the <productname>PostgreSQL</productname> source tree. </para> diff --git a/src/backend/postmaster/launch_backend.c b/src/backend/postmaster/launch_backend.c index cd9547b03a3..de1b06df10d 100644 --- a/src/backend/postmaster/launch_backend.c +++ b/src/backend/postmaster/launch_backend.c @@ -101,7 +101,7 @@ typedef struct struct InjectionPointsCtl *ActiveInjectionPoints; #endif int NamedLWLockTrancheRequests; - NamedLWLockTranche *NamedLWLockTrancheArray; + char **NamedLWLockTrancheNames; int *LWLockCounter; LWLockPadded *MainLWLockArray; slock_t *ProcStructLock; @@ -761,7 +761,7 @@ save_backend_variables(BackendParameters *param, #endif param->NamedLWLockTrancheRequests = NamedLWLockTrancheRequests; - param->NamedLWLockTrancheArray = NamedLWLockTrancheArray; + param->NamedLWLockTrancheNames = NamedLWLockTrancheNames; param->LWLockCounter = LWLockCounter; param->MainLWLockArray = MainLWLockArray; param->ProcStructLock = ProcStructLock; @@ -1022,7 +1022,7 @@ restore_backend_variables(BackendParameters *param) #endif NamedLWLockTrancheRequests = param->NamedLWLockTrancheRequests; - NamedLWLockTrancheArray = param->NamedLWLockTrancheArray; + NamedLWLockTrancheNames = param->NamedLWLockTrancheNames; LWLockCounter = param->LWLockCounter; MainLWLockArray = param->MainLWLockArray; ProcStructLock = param->ProcStructLock; diff --git a/src/backend/storage/ipc/dsm_registry.c b/src/backend/storage/ipc/dsm_registry.c index ca12815f4a8..97130925106 100644 --- a/src/backend/storage/ipc/dsm_registry.c +++ b/src/backend/storage/ipc/dsm_registry.c @@ -299,8 +299,7 @@ GetNamedDSA(const char *name, bool *found) entry->type = DSMR_ENTRY_TYPE_DSA; /* Initialize the LWLock tranche for the DSA. */ - state->tranche = LWLockNewTrancheId(); - LWLockRegisterTranche(state->tranche, name); + state->tranche = LWLockNewTrancheId(name); /* Initialize the DSA. */ ret = dsa_create(state->tranche); @@ -321,9 +320,6 @@ GetNamedDSA(const char *name, bool *found) ereport(ERROR, (errmsg("requested DSA already attached to current process"))); - /* Initialize existing LWLock tranche for the DSA. */ - LWLockRegisterTranche(state->tranche, name); - /* Attach to existing DSA. */ ret = dsa_attach(state->handle); dsa_pin_mapping(ret); @@ -378,8 +374,7 @@ GetNamedDSHash(const char *name, const dshash_parameters *params, bool *found) entry->type = DSMR_ENTRY_TYPE_DSH; /* Initialize the LWLock tranche for the hash table. */ - dsh_state->tranche = LWLockNewTrancheId(); - LWLockRegisterTranche(dsh_state->tranche, name); + dsh_state->tranche = LWLockNewTrancheId(name); /* Initialize the DSA for the hash table. */ dsa = dsa_create(dsh_state->tranche); @@ -409,9 +404,6 @@ GetNamedDSHash(const char *name, const dshash_parameters *params, bool *found) ereport(ERROR, (errmsg("requested DSHash already attached to current process"))); - /* Initialize existing LWLock tranche for the hash table. */ - LWLockRegisterTranche(dsh_state->tranche, name); - /* Attach to existing DSA for the hash table. */ dsa = dsa_attach(dsh_state->dsa_handle); dsa_pin_mapping(dsa); diff --git a/src/backend/storage/lmgr/lwlock.c b/src/backend/storage/lmgr/lwlock.c index a4aecd1fbc3..b4636d86ca6 100644 --- a/src/backend/storage/lmgr/lwlock.c +++ b/src/backend/storage/lmgr/lwlock.c @@ -126,8 +126,8 @@ StaticAssertDecl((LW_VAL_EXCLUSIVE & LW_FLAG_MASK) == 0, * in lwlocklist.h. We absorb the names of these tranches, too. * * 3. Extensions can create new tranches, via either RequestNamedLWLockTranche - * or LWLockRegisterTranche. The names of these that are known in the current - * process appear in LWLockTrancheNames[]. + * or LWLockNewTrancheId. These names are stored in shared memory and can be + * accessed via NamedLWLockTrancheNames. * * All these names are user-visible as wait event names, so choose with care * ... and do not forget to update the documentation's list of wait events. @@ -146,11 +146,12 @@ StaticAssertDecl(lengthof(BuiltinTrancheNames) == /* * This is indexed by tranche ID minus LWTRANCHE_FIRST_USER_DEFINED, and - * stores the names of all dynamically-created tranches known to the current - * process. Any unused entries in the array will contain NULL. + * points to the shared memory locations of the names of all + * dynamically-created tranches. Backends inherit the pointer by fork from the + * postmaster (except in the EXEC_BACKEND case, where we have special measures + * to pass it down). */ -static const char **LWLockTrancheNames = NULL; -static int LWLockTrancheNamesAllocated = 0; +char **NamedLWLockTrancheNames = NULL; /* * This points to the main array of LWLocks in shared memory. Backends inherit @@ -184,20 +185,22 @@ typedef struct NamedLWLockTrancheRequest } NamedLWLockTrancheRequest; static NamedLWLockTrancheRequest *NamedLWLockTrancheRequestArray = NULL; -static int NamedLWLockTrancheRequestsAllocated = 0; /* - * NamedLWLockTrancheRequests is both the valid length of the request array, - * and the length of the shared-memory NamedLWLockTrancheArray later on. - * This variable and NamedLWLockTrancheArray are non-static so that - * postmaster.c can copy them to child processes in EXEC_BACKEND builds. + * NamedLWLockTrancheRequests is the valid length of the request array. This + * variable is non-static so that postmaster.c can copy them to child processes + * in EXEC_BACKEND builds. */ int NamedLWLockTrancheRequests = 0; /* points to data in shared memory: */ -NamedLWLockTranche *NamedLWLockTrancheArray = NULL; int *LWLockCounter = NULL; +/* backend-local counter of registered tranches */ +static int LocalLWLockCounter; + +#define MAX_NAMED_TRANCHES 256 + static void InitializeLWLocks(void); static inline void LWLockReportWaitStart(LWLock *lock); static inline void LWLockReportWaitEnd(void); @@ -392,31 +395,28 @@ Size LWLockShmemSize(void) { Size size; - int i; int numLocks = NUM_FIXED_LWLOCKS; /* Calculate total number of locks needed in the main array. */ numLocks += NumLWLocksForNamedTranches(); - /* Space for dynamic allocation counter, plus room for alignment. */ - size = sizeof(int) + LWLOCK_PADDED_SIZE; + /* Space for dynamic allocation counter. */ + size = MAXALIGN(sizeof(int)); - /* Space for the LWLock array. */ - size = add_size(size, mul_size(numLocks, sizeof(LWLockPadded))); + /* Space for named tranches. */ + size = add_size(size, mul_size(MAX_NAMED_TRANCHES, sizeof(char *))); + size = add_size(size, mul_size(MAX_NAMED_TRANCHES, NAMEDATALEN)); - /* space for named tranches. */ - size = add_size(size, mul_size(NamedLWLockTrancheRequests, sizeof(NamedLWLockTranche))); - - /* space for name of each tranche. */ - for (i = 0; i < NamedLWLockTrancheRequests; i++) - size = add_size(size, strlen(NamedLWLockTrancheRequestArray[i].tranche_name) + 1); + /* Space for the LWLock array, plus room for cache line alignment. */ + size = add_size(size, LWLOCK_PADDED_SIZE); + size = add_size(size, mul_size(numLocks, sizeof(LWLockPadded))); return size; } /* * Allocate shmem space for the main LWLock array and all tranches and - * initialize it. We also register extension LWLock tranches here. + * initialize it. */ void CreateLWLocks(void) @@ -432,7 +432,16 @@ CreateLWLocks(void) /* Initialize the dynamic-allocation counter for tranches */ LWLockCounter = (int *) ptr; *LWLockCounter = LWTRANCHE_FIRST_USER_DEFINED; - ptr += sizeof(int); + ptr += MAXALIGN(sizeof(int)); + + /* Initialize tranche names */ + NamedLWLockTrancheNames = (char **) ptr; + ptr += MAX_NAMED_TRANCHES * sizeof(char *); + for (int i = 0; i < MAX_NAMED_TRANCHES; i++) + { + NamedLWLockTrancheNames[i] = ptr; + ptr += NAMEDATALEN; + } /* Ensure desired alignment of LWLock array */ ptr += LWLOCK_PADDED_SIZE - ((uintptr_t) ptr) % LWLOCK_PADDED_SIZE; @@ -441,11 +450,6 @@ CreateLWLocks(void) /* Initialize all LWLocks */ InitializeLWLocks(); } - - /* Register named extension LWLock tranches in the current process. */ - for (int i = 0; i < NamedLWLockTrancheRequests; i++) - LWLockRegisterTranche(NamedLWLockTrancheArray[i].trancheId, - NamedLWLockTrancheArray[i].trancheName); } /* @@ -454,7 +458,6 @@ CreateLWLocks(void) static void InitializeLWLocks(void) { - int numNamedLocks = NumLWLocksForNamedTranches(); int id; int i; int j; @@ -485,32 +488,18 @@ InitializeLWLocks(void) */ if (NamedLWLockTrancheRequests > 0) { - char *trancheNames; - - NamedLWLockTrancheArray = (NamedLWLockTranche *) - &MainLWLockArray[NUM_FIXED_LWLOCKS + numNamedLocks]; - - trancheNames = (char *) NamedLWLockTrancheArray + - (NamedLWLockTrancheRequests * sizeof(NamedLWLockTranche)); lock = &MainLWLockArray[NUM_FIXED_LWLOCKS]; for (i = 0; i < NamedLWLockTrancheRequests; i++) { NamedLWLockTrancheRequest *request; - NamedLWLockTranche *tranche; - char *name; + int tranche; request = &NamedLWLockTrancheRequestArray[i]; - tranche = &NamedLWLockTrancheArray[i]; - - name = trancheNames; - trancheNames += strlen(request->tranche_name) + 1; - strcpy(name, request->tranche_name); - tranche->trancheId = LWLockNewTrancheId(); - tranche->trancheName = name; + tranche = LWLockNewTrancheId(request->tranche_name); for (j = 0; j < request->num_lwlocks; j++, lock++) - LWLockInitialize(&lock->lock, tranche->trancheId); + LWLockInitialize(&lock->lock, tranche); } } } @@ -562,59 +551,44 @@ GetNamedLWLockTranche(const char *tranche_name) } /* - * Allocate a new tranche ID. + * Allocate a new tranche ID with the provided name. */ int -LWLockNewTrancheId(void) +LWLockNewTrancheId(const char *name) { int result; - /* We use the ShmemLock spinlock to protect LWLockCounter */ - SpinLockAcquire(ShmemLock); - result = (*LWLockCounter)++; - SpinLockRelease(ShmemLock); + if (!name) + ereport(ERROR, + (errcode(ERRCODE_INVALID_NAME), + errmsg("tranche name cannot be NULL"))); - return result; -} + if (strlen(name) >= NAMEDATALEN) + ereport(ERROR, + (errcode(ERRCODE_NAME_TOO_LONG), + errmsg("tranche name too long"), + errdetail("LWLock tranche names must be no longer than %d bytes.", + NAMEDATALEN - 1))); -/* - * Register a dynamic tranche name in the lookup table of the current process. - * - * This routine will save a pointer to the tranche name passed as an argument, - * so the name should be allocated in a backend-lifetime context - * (shared memory, TopMemoryContext, static constant, or similar). - * - * The tranche name will be user-visible as a wait event name, so try to - * use a name that fits the style for those. - */ -void -LWLockRegisterTranche(int tranche_id, const char *tranche_name) -{ - /* This should only be called for user-defined tranches. */ - if (tranche_id < LWTRANCHE_FIRST_USER_DEFINED) - return; - - /* Convert to array index. */ - tranche_id -= LWTRANCHE_FIRST_USER_DEFINED; + /* We use the ShmemLock spinlock to protect LWLockCounter */ + SpinLockAcquire(ShmemLock); - /* If necessary, create or enlarge array. */ - if (tranche_id >= LWLockTrancheNamesAllocated) + if (*LWLockCounter - LWTRANCHE_FIRST_USER_DEFINED >= MAX_NAMED_TRANCHES) { - int newalloc; + SpinLockRelease(ShmemLock); + ereport(ERROR, + (errmsg("maximum number of tranches already registered"), + errdetail("No more than %d tranches may be registered.", + MAX_NAMED_TRANCHES))); + } - newalloc = pg_nextpower2_32(Max(8, tranche_id + 1)); + result = (*LWLockCounter)++; + LocalLWLockCounter = *LWLockCounter; + strlcpy(NamedLWLockTrancheNames[result - LWTRANCHE_FIRST_USER_DEFINED], name, NAMEDATALEN); - if (LWLockTrancheNames == NULL) - LWLockTrancheNames = (const char **) - MemoryContextAllocZero(TopMemoryContext, - newalloc * sizeof(char *)); - else - LWLockTrancheNames = - repalloc0_array(LWLockTrancheNames, const char *, LWLockTrancheNamesAllocated, newalloc); - LWLockTrancheNamesAllocated = newalloc; - } + SpinLockRelease(ShmemLock); - LWLockTrancheNames[tranche_id] = tranche_name; + return result; } /* @@ -637,27 +611,33 @@ RequestNamedLWLockTranche(const char *tranche_name, int num_lwlocks) if (!process_shmem_requests_in_progress) elog(FATAL, "cannot request additional LWLocks outside shmem_request_hook"); + if (!tranche_name) + ereport(ERROR, + (errcode(ERRCODE_INVALID_NAME), + errmsg("tranche name cannot be NULL"))); + + if (strlen(tranche_name) >= NAMEDATALEN) + ereport(ERROR, + (errcode(ERRCODE_NAME_TOO_LONG), + errmsg("tranche name too long"), + errdetail("LWLock tranche names must be no longer than %d bytes.", + NAMEDATALEN - 1))); + if (NamedLWLockTrancheRequestArray == NULL) { - NamedLWLockTrancheRequestsAllocated = 16; NamedLWLockTrancheRequestArray = (NamedLWLockTrancheRequest *) MemoryContextAlloc(TopMemoryContext, - NamedLWLockTrancheRequestsAllocated + MAX_NAMED_TRANCHES * sizeof(NamedLWLockTrancheRequest)); } - if (NamedLWLockTrancheRequests >= NamedLWLockTrancheRequestsAllocated) - { - int i = pg_nextpower2_32(NamedLWLockTrancheRequests + 1); - - NamedLWLockTrancheRequestArray = (NamedLWLockTrancheRequest *) - repalloc(NamedLWLockTrancheRequestArray, - i * sizeof(NamedLWLockTrancheRequest)); - NamedLWLockTrancheRequestsAllocated = i; - } + if (NamedLWLockTrancheRequests >= MAX_NAMED_TRANCHES) + ereport(ERROR, + (errmsg("maximum number of tranches already registered"), + errdetail("No more than %d tranches may be registered.", + MAX_NAMED_TRANCHES))); request = &NamedLWLockTrancheRequestArray[NamedLWLockTrancheRequests]; - Assert(strlen(tranche_name) + 1 <= NAMEDATALEN); strlcpy(request->tranche_name, tranche_name, NAMEDATALEN); request->num_lwlocks = num_lwlocks; NamedLWLockTrancheRequests++; @@ -669,6 +649,9 @@ RequestNamedLWLockTranche(const char *tranche_name, int num_lwlocks) void LWLockInitialize(LWLock *lock, int tranche_id) { + /* verify the tranche_id is valid */ + (void) GetLWTrancheName(tranche_id); + pg_atomic_init_u32(&lock->state, LW_FLAG_RELEASE_OK); #ifdef LOCK_DEBUG pg_atomic_init_u32(&lock->nwaiters, 0); @@ -709,18 +692,23 @@ GetLWTrancheName(uint16 trancheId) if (trancheId < LWTRANCHE_FIRST_USER_DEFINED) return BuiltinTrancheNames[trancheId]; + /* verify the trancheId is valid */ + if (trancheId >= LocalLWLockCounter) + { + SpinLockAcquire(ShmemLock); + LocalLWLockCounter = *LWLockCounter; + SpinLockRelease(ShmemLock); + + if (trancheId >= LocalLWLockCounter) + elog(ERROR, "tranche %d is not registered", trancheId); + } + /* - * It's an extension tranche, so look in LWLockTrancheNames[]. However, - * it's possible that the tranche has never been registered in the current - * process, in which case give up and return "extension". + * It's an extension tranche, so look in NamedLWLockTrancheNames. */ trancheId -= LWTRANCHE_FIRST_USER_DEFINED; - if (trancheId >= LWLockTrancheNamesAllocated || - LWLockTrancheNames[trancheId] == NULL) - return "extension"; - - return LWLockTrancheNames[trancheId]; + return NamedLWLockTrancheNames[trancheId]; } /* diff --git a/src/include/storage/lwlock.h b/src/include/storage/lwlock.h index f9cf57f8d26..3877aaa7f03 100644 --- a/src/include/storage/lwlock.h +++ b/src/include/storage/lwlock.h @@ -73,14 +73,7 @@ typedef union LWLockPadded extern PGDLLIMPORT LWLockPadded *MainLWLockArray; -/* struct for storing named tranche information */ -typedef struct NamedLWLockTranche -{ - int trancheId; - char *trancheName; -} NamedLWLockTranche; - -extern PGDLLIMPORT NamedLWLockTranche *NamedLWLockTrancheArray; +extern PGDLLIMPORT char **NamedLWLockTrancheNames; extern PGDLLIMPORT int NamedLWLockTrancheRequests; extern PGDLLIMPORT int *LWLockCounter; @@ -158,18 +151,11 @@ extern LWLockPadded *GetNamedLWLockTranche(const char *tranche_name); /* * There is another, more flexible method of obtaining lwlocks. First, call - * LWLockNewTrancheId just once to obtain a tranche ID; this allocates from - * a shared counter. Next, each individual process using the tranche should - * call LWLockRegisterTranche() to associate that tranche ID with a name. - * Finally, LWLockInitialize should be called just once per lwlock, passing - * the tranche ID as an argument. - * - * It may seem strange that each process using the tranche must register it - * separately, but dynamic shared memory segments aren't guaranteed to be - * mapped at the same address in all coordinating backends, so storing the - * registration in the main shared memory segment wouldn't work for that case. + * LWLockNewTrancheId to obtain a tranche ID; this allocates from a shared + * counter. Second, LWLockInitialize should be called just once per lwlock, + * passing the tranche ID as an argument. */ -extern int LWLockNewTrancheId(void); +extern int LWLockNewTrancheId(const char *name); extern void LWLockRegisterTranche(int tranche_id, const char *tranche_name); extern void LWLockInitialize(LWLock *lock, int tranche_id); diff --git a/src/test/modules/test_dsa/test_dsa.c b/src/test/modules/test_dsa/test_dsa.c index cd24d0f4873..01d5c6fa67f 100644 --- a/src/test/modules/test_dsa/test_dsa.c +++ b/src/test/modules/test_dsa/test_dsa.c @@ -29,8 +29,7 @@ test_dsa_basic(PG_FUNCTION_ARGS) dsa_pointer p[100]; /* XXX: this tranche is leaked */ - tranche_id = LWLockNewTrancheId(); - LWLockRegisterTranche(tranche_id, "test_dsa"); + tranche_id = LWLockNewTrancheId("test_dsa"); a = dsa_create(tranche_id); for (int i = 0; i < 100; i++) @@ -70,8 +69,7 @@ test_dsa_resowners(PG_FUNCTION_ARGS) ResourceOwner childowner; /* XXX: this tranche is leaked */ - tranche_id = LWLockNewTrancheId(); - LWLockRegisterTranche(tranche_id, "test_dsa"); + tranche_id = LWLockNewTrancheId("test_dsa"); /* Create DSA in parent resource owner */ a = dsa_create(tranche_id); diff --git a/src/test/modules/test_dsm_registry/test_dsm_registry.c b/src/test/modules/test_dsm_registry/test_dsm_registry.c index 141c8ed1b34..4cc2ccdac3f 100644 --- a/src/test/modules/test_dsm_registry/test_dsm_registry.c +++ b/src/test/modules/test_dsm_registry/test_dsm_registry.c @@ -48,7 +48,7 @@ init_tdr_dsm(void *ptr) { TestDSMRegistryStruct *dsm = (TestDSMRegistryStruct *) ptr; - LWLockInitialize(&dsm->lck, LWLockNewTrancheId()); + LWLockInitialize(&dsm->lck, LWLockNewTrancheId("test_dsm_registry")); dsm->val = 0; } @@ -61,7 +61,6 @@ tdr_attach_shmem(void) sizeof(TestDSMRegistryStruct), init_tdr_dsm, &found); - LWLockRegisterTranche(tdr_dsm->lck.tranche, "test_dsm_registry"); if (tdr_dsa == NULL) tdr_dsa = GetNamedDSA("test_dsm_registry_dsa", &found); diff --git a/src/test/modules/test_radixtree/test_radixtree.c b/src/test/modules/test_radixtree/test_radixtree.c index 80ad0296164..787162c8793 100644 --- a/src/test/modules/test_radixtree/test_radixtree.c +++ b/src/test/modules/test_radixtree/test_radixtree.c @@ -124,10 +124,9 @@ test_empty(void) rt_iter *iter; uint64 key; #ifdef TEST_SHARED_RT - int tranche_id = LWLockNewTrancheId(); + int tranche_id = LWLockNewTrancheId("test_radix_tree"); dsa_area *dsa; - LWLockRegisterTranche(tranche_id, "test_radix_tree"); dsa = dsa_create(tranche_id); radixtree = rt_create(dsa, tranche_id); #else @@ -167,10 +166,9 @@ test_basic(rt_node_class_test_elem *test_info, int shift, bool asc) uint64 *keys; int children = test_info->nkeys; #ifdef TEST_SHARED_RT - int tranche_id = LWLockNewTrancheId(); + int tranche_id = LWLockNewTrancheId("test_radix_tree"); dsa_area *dsa; - LWLockRegisterTranche(tranche_id, "test_radix_tree"); dsa = dsa_create(tranche_id); radixtree = rt_create(dsa, tranche_id); #else @@ -304,10 +302,9 @@ test_random(void) int num_keys = 100000; uint64 *keys; #ifdef TEST_SHARED_RT - int tranche_id = LWLockNewTrancheId(); + int tranche_id = LWLockNewTrancheId("test_radix_tree"); dsa_area *dsa; - LWLockRegisterTranche(tranche_id, "test_radix_tree"); dsa = dsa_create(tranche_id); radixtree = rt_create(dsa, tranche_id); #else diff --git a/src/test/modules/test_slru/test_slru.c b/src/test/modules/test_slru/test_slru.c index 32750930e43..8c0367eeee4 100644 --- a/src/test/modules/test_slru/test_slru.c +++ b/src/test/modules/test_slru/test_slru.c @@ -232,11 +232,9 @@ test_slru_shmem_startup(void) (void) MakePGDirectory(slru_dir_name); /* initialize the SLRU facility */ - test_tranche_id = LWLockNewTrancheId(); - LWLockRegisterTranche(test_tranche_id, "test_slru_tranche"); + test_tranche_id = LWLockNewTrancheId("test_slru_tranche"); - test_buffer_tranche_id = LWLockNewTrancheId(); - LWLockRegisterTranche(test_tranche_id, "test_buffer_tranche"); + test_buffer_tranche_id = LWLockNewTrancheId("test_buffer_tranche"); TestSlruCtl->PagePrecedes = test_slru_page_precedes_logically; SimpleLruInit(TestSlruCtl, "TestSLRU", diff --git a/src/test/modules/test_tidstore/test_tidstore.c b/src/test/modules/test_tidstore/test_tidstore.c index eb16e0fbfa6..0c8f43867e5 100644 --- a/src/test/modules/test_tidstore/test_tidstore.c +++ b/src/test/modules/test_tidstore/test_tidstore.c @@ -103,8 +103,7 @@ test_create(PG_FUNCTION_ARGS) { int tranche_id; - tranche_id = LWLockNewTrancheId(); - LWLockRegisterTranche(tranche_id, "test_tidstore"); + tranche_id = LWLockNewTrancheId("test_tidstore"); tidstore = TidStoreCreateShared(tidstore_max_size, tranche_id); -- 2.39.5 (Apple Git-154) --oZuO6jTSiLeu2qvQ Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename=v20-0002-Tests-for-LWLock-tranche-registration-improvemen.patch ^ permalink raw reply [nested|flat] 5+ messages in thread
* [PATCH v21 1/2] Move dynamically-allocated tranche names to shared memory. @ 2025-08-27 20:13 Nathan Bossart <[email protected]> 0 siblings, 0 replies; 5+ messages in thread From: Nathan Bossart @ 2025-08-27 20:13 UTC (permalink / raw) --- contrib/pg_prewarm/autoprewarm.c | 3 +- doc/src/sgml/xfunc.sgml | 15 +- src/backend/postmaster/launch_backend.c | 6 +- src/backend/storage/ipc/dsm_registry.c | 12 +- src/backend/storage/lmgr/lwlock.c | 218 +++++++++--------- src/include/storage/lwlock.h | 25 +- src/test/modules/test_dsa/test_dsa.c | 6 +- .../test_dsm_registry/test_dsm_registry.c | 3 +- .../modules/test_radixtree/test_radixtree.c | 9 +- src/test/modules/test_slru/test_slru.c | 6 +- .../modules/test_tidstore/test_tidstore.c | 3 +- 11 files changed, 131 insertions(+), 175 deletions(-) diff --git a/contrib/pg_prewarm/autoprewarm.c b/contrib/pg_prewarm/autoprewarm.c index c01b9c7e6a4..880e897796a 100644 --- a/contrib/pg_prewarm/autoprewarm.c +++ b/contrib/pg_prewarm/autoprewarm.c @@ -864,7 +864,7 @@ apw_init_state(void *ptr) { AutoPrewarmSharedState *state = (AutoPrewarmSharedState *) ptr; - LWLockInitialize(&state->lock, LWLockNewTrancheId()); + LWLockInitialize(&state->lock, LWLockNewTrancheId("autoprewarm")); state->bgworker_pid = InvalidPid; state->pid_using_dumpfile = InvalidPid; } @@ -883,7 +883,6 @@ apw_init_shmem(void) sizeof(AutoPrewarmSharedState), apw_init_state, &found); - LWLockRegisterTranche(apw_state->lock.tranche, "autoprewarm"); return found; } diff --git a/doc/src/sgml/xfunc.sgml b/doc/src/sgml/xfunc.sgml index f116d0648e5..da21ef56891 100644 --- a/doc/src/sgml/xfunc.sgml +++ b/doc/src/sgml/xfunc.sgml @@ -3759,7 +3759,7 @@ LWLockPadded *GetNamedLWLockTranche(const char *tranche_name) <literal>shmem_request_hook</literal>. To do so, first allocate a <literal>tranche_id</literal> by calling: <programlisting> -int LWLockNewTrancheId(void) +int LWLockNewTrancheId(const char *name) </programlisting> Next, initialize each LWLock, passing the new <literal>tranche_id</literal> as an argument: @@ -3777,17 +3777,8 @@ void LWLockInitialize(LWLock *lock, int tranche_id) </para> <para> - Finally, each backend using the <literal>tranche_id</literal> should - associate it with a <literal>tranche_name</literal> by calling: -<programlisting> -void LWLockRegisterTranche(int tranche_id, const char *tranche_name) -</programlisting> - </para> - - <para> - A complete usage example of <function>LWLockNewTrancheId</function>, - <function>LWLockInitialize</function>, and - <function>LWLockRegisterTranche</function> can be found in + A complete usage example of <function>LWLockNewTrancheId</function> and + <function>LWLockInitialize</function> can be found in <filename>contrib/pg_prewarm/autoprewarm.c</filename> in the <productname>PostgreSQL</productname> source tree. </para> diff --git a/src/backend/postmaster/launch_backend.c b/src/backend/postmaster/launch_backend.c index cd9547b03a3..de1b06df10d 100644 --- a/src/backend/postmaster/launch_backend.c +++ b/src/backend/postmaster/launch_backend.c @@ -101,7 +101,7 @@ typedef struct struct InjectionPointsCtl *ActiveInjectionPoints; #endif int NamedLWLockTrancheRequests; - NamedLWLockTranche *NamedLWLockTrancheArray; + char **NamedLWLockTrancheNames; int *LWLockCounter; LWLockPadded *MainLWLockArray; slock_t *ProcStructLock; @@ -761,7 +761,7 @@ save_backend_variables(BackendParameters *param, #endif param->NamedLWLockTrancheRequests = NamedLWLockTrancheRequests; - param->NamedLWLockTrancheArray = NamedLWLockTrancheArray; + param->NamedLWLockTrancheNames = NamedLWLockTrancheNames; param->LWLockCounter = LWLockCounter; param->MainLWLockArray = MainLWLockArray; param->ProcStructLock = ProcStructLock; @@ -1022,7 +1022,7 @@ restore_backend_variables(BackendParameters *param) #endif NamedLWLockTrancheRequests = param->NamedLWLockTrancheRequests; - NamedLWLockTrancheArray = param->NamedLWLockTrancheArray; + NamedLWLockTrancheNames = param->NamedLWLockTrancheNames; LWLockCounter = param->LWLockCounter; MainLWLockArray = param->MainLWLockArray; ProcStructLock = param->ProcStructLock; diff --git a/src/backend/storage/ipc/dsm_registry.c b/src/backend/storage/ipc/dsm_registry.c index ca12815f4a8..97130925106 100644 --- a/src/backend/storage/ipc/dsm_registry.c +++ b/src/backend/storage/ipc/dsm_registry.c @@ -299,8 +299,7 @@ GetNamedDSA(const char *name, bool *found) entry->type = DSMR_ENTRY_TYPE_DSA; /* Initialize the LWLock tranche for the DSA. */ - state->tranche = LWLockNewTrancheId(); - LWLockRegisterTranche(state->tranche, name); + state->tranche = LWLockNewTrancheId(name); /* Initialize the DSA. */ ret = dsa_create(state->tranche); @@ -321,9 +320,6 @@ GetNamedDSA(const char *name, bool *found) ereport(ERROR, (errmsg("requested DSA already attached to current process"))); - /* Initialize existing LWLock tranche for the DSA. */ - LWLockRegisterTranche(state->tranche, name); - /* Attach to existing DSA. */ ret = dsa_attach(state->handle); dsa_pin_mapping(ret); @@ -378,8 +374,7 @@ GetNamedDSHash(const char *name, const dshash_parameters *params, bool *found) entry->type = DSMR_ENTRY_TYPE_DSH; /* Initialize the LWLock tranche for the hash table. */ - dsh_state->tranche = LWLockNewTrancheId(); - LWLockRegisterTranche(dsh_state->tranche, name); + dsh_state->tranche = LWLockNewTrancheId(name); /* Initialize the DSA for the hash table. */ dsa = dsa_create(dsh_state->tranche); @@ -409,9 +404,6 @@ GetNamedDSHash(const char *name, const dshash_parameters *params, bool *found) ereport(ERROR, (errmsg("requested DSHash already attached to current process"))); - /* Initialize existing LWLock tranche for the hash table. */ - LWLockRegisterTranche(dsh_state->tranche, name); - /* Attach to existing DSA for the hash table. */ dsa = dsa_attach(dsh_state->dsa_handle); dsa_pin_mapping(dsa); diff --git a/src/backend/storage/lmgr/lwlock.c b/src/backend/storage/lmgr/lwlock.c index a4aecd1fbc3..7bd9aaf8377 100644 --- a/src/backend/storage/lmgr/lwlock.c +++ b/src/backend/storage/lmgr/lwlock.c @@ -126,8 +126,8 @@ StaticAssertDecl((LW_VAL_EXCLUSIVE & LW_FLAG_MASK) == 0, * in lwlocklist.h. We absorb the names of these tranches, too. * * 3. Extensions can create new tranches, via either RequestNamedLWLockTranche - * or LWLockRegisterTranche. The names of these that are known in the current - * process appear in LWLockTrancheNames[]. + * or LWLockNewTrancheId. These names are stored in shared memory and can be + * accessed via NamedLWLockTrancheNames. * * All these names are user-visible as wait event names, so choose with care * ... and do not forget to update the documentation's list of wait events. @@ -146,11 +146,12 @@ StaticAssertDecl(lengthof(BuiltinTrancheNames) == /* * This is indexed by tranche ID minus LWTRANCHE_FIRST_USER_DEFINED, and - * stores the names of all dynamically-created tranches known to the current - * process. Any unused entries in the array will contain NULL. + * points to the shared memory locations of the names of all + * dynamically-created tranches. Backends inherit the pointer by fork from the + * postmaster (except in the EXEC_BACKEND case, where we have special measures + * to pass it down). */ -static const char **LWLockTrancheNames = NULL; -static int LWLockTrancheNamesAllocated = 0; +char **NamedLWLockTrancheNames = NULL; /* * This points to the main array of LWLocks in shared memory. Backends inherit @@ -184,20 +185,22 @@ typedef struct NamedLWLockTrancheRequest } NamedLWLockTrancheRequest; static NamedLWLockTrancheRequest *NamedLWLockTrancheRequestArray = NULL; -static int NamedLWLockTrancheRequestsAllocated = 0; /* - * NamedLWLockTrancheRequests is both the valid length of the request array, - * and the length of the shared-memory NamedLWLockTrancheArray later on. - * This variable and NamedLWLockTrancheArray are non-static so that - * postmaster.c can copy them to child processes in EXEC_BACKEND builds. + * NamedLWLockTrancheRequests is the valid length of the request array. This + * variable is non-static so that postmaster.c can copy them to child processes + * in EXEC_BACKEND builds. */ int NamedLWLockTrancheRequests = 0; -/* points to data in shared memory: */ -NamedLWLockTranche *NamedLWLockTrancheArray = NULL; +/* shared memory counter of registered tranches */ int *LWLockCounter = NULL; +/* backend-local counter of registered tranches */ +static int LocalLWLockCounter; + +#define MAX_NAMED_TRANCHES 256 + static void InitializeLWLocks(void); static inline void LWLockReportWaitStart(LWLock *lock); static inline void LWLockReportWaitEnd(void); @@ -392,31 +395,28 @@ Size LWLockShmemSize(void) { Size size; - int i; int numLocks = NUM_FIXED_LWLOCKS; /* Calculate total number of locks needed in the main array. */ numLocks += NumLWLocksForNamedTranches(); - /* Space for dynamic allocation counter, plus room for alignment. */ - size = sizeof(int) + LWLOCK_PADDED_SIZE; + /* Space for dynamic allocation counter. */ + size = MAXALIGN(sizeof(int)); - /* Space for the LWLock array. */ - size = add_size(size, mul_size(numLocks, sizeof(LWLockPadded))); + /* Space for named tranches. */ + size = add_size(size, mul_size(MAX_NAMED_TRANCHES, sizeof(char *))); + size = add_size(size, mul_size(MAX_NAMED_TRANCHES, NAMEDATALEN)); - /* space for named tranches. */ - size = add_size(size, mul_size(NamedLWLockTrancheRequests, sizeof(NamedLWLockTranche))); - - /* space for name of each tranche. */ - for (i = 0; i < NamedLWLockTrancheRequests; i++) - size = add_size(size, strlen(NamedLWLockTrancheRequestArray[i].tranche_name) + 1); + /* Space for the LWLock array, plus room for cache line alignment. */ + size = add_size(size, LWLOCK_PADDED_SIZE); + size = add_size(size, mul_size(numLocks, sizeof(LWLockPadded))); return size; } /* * Allocate shmem space for the main LWLock array and all tranches and - * initialize it. We also register extension LWLock tranches here. + * initialize it. */ void CreateLWLocks(void) @@ -432,7 +432,16 @@ CreateLWLocks(void) /* Initialize the dynamic-allocation counter for tranches */ LWLockCounter = (int *) ptr; *LWLockCounter = LWTRANCHE_FIRST_USER_DEFINED; - ptr += sizeof(int); + ptr += MAXALIGN(sizeof(int)); + + /* Initialize tranche names */ + NamedLWLockTrancheNames = (char **) ptr; + ptr += MAX_NAMED_TRANCHES * sizeof(char *); + for (int i = 0; i < MAX_NAMED_TRANCHES; i++) + { + NamedLWLockTrancheNames[i] = ptr; + ptr += NAMEDATALEN; + } /* Ensure desired alignment of LWLock array */ ptr += LWLOCK_PADDED_SIZE - ((uintptr_t) ptr) % LWLOCK_PADDED_SIZE; @@ -441,11 +450,6 @@ CreateLWLocks(void) /* Initialize all LWLocks */ InitializeLWLocks(); } - - /* Register named extension LWLock tranches in the current process. */ - for (int i = 0; i < NamedLWLockTrancheRequests; i++) - LWLockRegisterTranche(NamedLWLockTrancheArray[i].trancheId, - NamedLWLockTrancheArray[i].trancheName); } /* @@ -454,7 +458,6 @@ CreateLWLocks(void) static void InitializeLWLocks(void) { - int numNamedLocks = NumLWLocksForNamedTranches(); int id; int i; int j; @@ -485,32 +488,18 @@ InitializeLWLocks(void) */ if (NamedLWLockTrancheRequests > 0) { - char *trancheNames; - - NamedLWLockTrancheArray = (NamedLWLockTranche *) - &MainLWLockArray[NUM_FIXED_LWLOCKS + numNamedLocks]; - - trancheNames = (char *) NamedLWLockTrancheArray + - (NamedLWLockTrancheRequests * sizeof(NamedLWLockTranche)); lock = &MainLWLockArray[NUM_FIXED_LWLOCKS]; for (i = 0; i < NamedLWLockTrancheRequests; i++) { NamedLWLockTrancheRequest *request; - NamedLWLockTranche *tranche; - char *name; + int tranche; request = &NamedLWLockTrancheRequestArray[i]; - tranche = &NamedLWLockTrancheArray[i]; - - name = trancheNames; - trancheNames += strlen(request->tranche_name) + 1; - strcpy(name, request->tranche_name); - tranche->trancheId = LWLockNewTrancheId(); - tranche->trancheName = name; + tranche = LWLockNewTrancheId(request->tranche_name); for (j = 0; j < request->num_lwlocks; j++, lock++) - LWLockInitialize(&lock->lock, tranche->trancheId); + LWLockInitialize(&lock->lock, tranche); } } } @@ -562,59 +551,47 @@ GetNamedLWLockTranche(const char *tranche_name) } /* - * Allocate a new tranche ID. + * Allocate a new tranche ID with the provided name. */ int -LWLockNewTrancheId(void) +LWLockNewTrancheId(const char *name) { int result; - /* We use the ShmemLock spinlock to protect LWLockCounter */ - SpinLockAcquire(ShmemLock); - result = (*LWLockCounter)++; - SpinLockRelease(ShmemLock); + if (!name) + ereport(ERROR, + (errcode(ERRCODE_INVALID_NAME), + errmsg("tranche name cannot be NULL"))); - return result; -} + if (strlen(name) >= NAMEDATALEN) + ereport(ERROR, + (errcode(ERRCODE_NAME_TOO_LONG), + errmsg("tranche name too long"), + errdetail("LWLock tranche names must be no longer than %d bytes.", + NAMEDATALEN - 1))); -/* - * Register a dynamic tranche name in the lookup table of the current process. - * - * This routine will save a pointer to the tranche name passed as an argument, - * so the name should be allocated in a backend-lifetime context - * (shared memory, TopMemoryContext, static constant, or similar). - * - * The tranche name will be user-visible as a wait event name, so try to - * use a name that fits the style for those. - */ -void -LWLockRegisterTranche(int tranche_id, const char *tranche_name) -{ - /* This should only be called for user-defined tranches. */ - if (tranche_id < LWTRANCHE_FIRST_USER_DEFINED) - return; - - /* Convert to array index. */ - tranche_id -= LWTRANCHE_FIRST_USER_DEFINED; + /* + * We use the ShmemLock spinlock to protect LWLockCounter and + * NamedLWLockTrancheNames. + */ + SpinLockAcquire(ShmemLock); - /* If necessary, create or enlarge array. */ - if (tranche_id >= LWLockTrancheNamesAllocated) + if (*LWLockCounter - LWTRANCHE_FIRST_USER_DEFINED >= MAX_NAMED_TRANCHES) { - int newalloc; + SpinLockRelease(ShmemLock); + ereport(ERROR, + (errmsg("maximum number of tranches already registered"), + errdetail("No more than %d tranches may be registered.", + MAX_NAMED_TRANCHES))); + } - newalloc = pg_nextpower2_32(Max(8, tranche_id + 1)); + result = (*LWLockCounter)++; + LocalLWLockCounter = *LWLockCounter; + strlcpy(NamedLWLockTrancheNames[result - LWTRANCHE_FIRST_USER_DEFINED], name, NAMEDATALEN); - if (LWLockTrancheNames == NULL) - LWLockTrancheNames = (const char **) - MemoryContextAllocZero(TopMemoryContext, - newalloc * sizeof(char *)); - else - LWLockTrancheNames = - repalloc0_array(LWLockTrancheNames, const char *, LWLockTrancheNamesAllocated, newalloc); - LWLockTrancheNamesAllocated = newalloc; - } + SpinLockRelease(ShmemLock); - LWLockTrancheNames[tranche_id] = tranche_name; + return result; } /* @@ -637,27 +614,33 @@ RequestNamedLWLockTranche(const char *tranche_name, int num_lwlocks) if (!process_shmem_requests_in_progress) elog(FATAL, "cannot request additional LWLocks outside shmem_request_hook"); + if (!tranche_name) + ereport(ERROR, + (errcode(ERRCODE_INVALID_NAME), + errmsg("tranche name cannot be NULL"))); + + if (strlen(tranche_name) >= NAMEDATALEN) + ereport(ERROR, + (errcode(ERRCODE_NAME_TOO_LONG), + errmsg("tranche name too long"), + errdetail("LWLock tranche names must be no longer than %d bytes.", + NAMEDATALEN - 1))); + if (NamedLWLockTrancheRequestArray == NULL) { - NamedLWLockTrancheRequestsAllocated = 16; NamedLWLockTrancheRequestArray = (NamedLWLockTrancheRequest *) MemoryContextAlloc(TopMemoryContext, - NamedLWLockTrancheRequestsAllocated + MAX_NAMED_TRANCHES * sizeof(NamedLWLockTrancheRequest)); } - if (NamedLWLockTrancheRequests >= NamedLWLockTrancheRequestsAllocated) - { - int i = pg_nextpower2_32(NamedLWLockTrancheRequests + 1); - - NamedLWLockTrancheRequestArray = (NamedLWLockTrancheRequest *) - repalloc(NamedLWLockTrancheRequestArray, - i * sizeof(NamedLWLockTrancheRequest)); - NamedLWLockTrancheRequestsAllocated = i; - } + if (NamedLWLockTrancheRequests >= MAX_NAMED_TRANCHES) + ereport(ERROR, + (errmsg("maximum number of tranches already registered"), + errdetail("No more than %d tranches may be registered.", + MAX_NAMED_TRANCHES))); request = &NamedLWLockTrancheRequestArray[NamedLWLockTrancheRequests]; - Assert(strlen(tranche_name) + 1 <= NAMEDATALEN); strlcpy(request->tranche_name, tranche_name, NAMEDATALEN); request->num_lwlocks = num_lwlocks; NamedLWLockTrancheRequests++; @@ -669,6 +652,9 @@ RequestNamedLWLockTranche(const char *tranche_name, int num_lwlocks) void LWLockInitialize(LWLock *lock, int tranche_id) { + /* verify the tranche_id is valid */ + (void) GetLWTrancheName(tranche_id); + pg_atomic_init_u32(&lock->state, LW_FLAG_RELEASE_OK); #ifdef LOCK_DEBUG pg_atomic_init_u32(&lock->nwaiters, 0); @@ -710,17 +696,29 @@ GetLWTrancheName(uint16 trancheId) return BuiltinTrancheNames[trancheId]; /* - * It's an extension tranche, so look in LWLockTrancheNames[]. However, - * it's possible that the tranche has never been registered in the current - * process, in which case give up and return "extension". + * We only ever add new entries to NamedLWLockTrancheNames, so most + * lookups can avoid taking the spinlock as long as the backend-local + * counter (LocalLWLockCounter) is greater than the requested tranche ID. + * Else, we need to first update the backend-local counter with ShmemLock + * held before attempting the lookup again. In practice, the latter case + * is probably rare. */ - trancheId -= LWTRANCHE_FIRST_USER_DEFINED; + if (trancheId >= LocalLWLockCounter) + { + SpinLockAcquire(ShmemLock); + LocalLWLockCounter = *LWLockCounter; + SpinLockRelease(ShmemLock); + + if (trancheId >= LocalLWLockCounter) + elog(ERROR, "tranche %d is not registered", trancheId); + } - if (trancheId >= LWLockTrancheNamesAllocated || - LWLockTrancheNames[trancheId] == NULL) - return "extension"; + /* + * It's an extension tranche, so look in NamedLWLockTrancheNames. + */ + trancheId -= LWTRANCHE_FIRST_USER_DEFINED; - return LWLockTrancheNames[trancheId]; + return NamedLWLockTrancheNames[trancheId]; } /* diff --git a/src/include/storage/lwlock.h b/src/include/storage/lwlock.h index f9cf57f8d26..16728521b9a 100644 --- a/src/include/storage/lwlock.h +++ b/src/include/storage/lwlock.h @@ -73,14 +73,7 @@ typedef union LWLockPadded extern PGDLLIMPORT LWLockPadded *MainLWLockArray; -/* struct for storing named tranche information */ -typedef struct NamedLWLockTranche -{ - int trancheId; - char *trancheName; -} NamedLWLockTranche; - -extern PGDLLIMPORT NamedLWLockTranche *NamedLWLockTrancheArray; +extern PGDLLIMPORT char **NamedLWLockTrancheNames; extern PGDLLIMPORT int NamedLWLockTrancheRequests; extern PGDLLIMPORT int *LWLockCounter; @@ -158,19 +151,11 @@ extern LWLockPadded *GetNamedLWLockTranche(const char *tranche_name); /* * There is another, more flexible method of obtaining lwlocks. First, call - * LWLockNewTrancheId just once to obtain a tranche ID; this allocates from - * a shared counter. Next, each individual process using the tranche should - * call LWLockRegisterTranche() to associate that tranche ID with a name. - * Finally, LWLockInitialize should be called just once per lwlock, passing - * the tranche ID as an argument. - * - * It may seem strange that each process using the tranche must register it - * separately, but dynamic shared memory segments aren't guaranteed to be - * mapped at the same address in all coordinating backends, so storing the - * registration in the main shared memory segment wouldn't work for that case. + * LWLockNewTrancheId to obtain a tranche ID; this allocates from a shared + * counter. Second, LWLockInitialize should be called just once per lwlock, + * passing the tranche ID as an argument. */ -extern int LWLockNewTrancheId(void); -extern void LWLockRegisterTranche(int tranche_id, const char *tranche_name); +extern int LWLockNewTrancheId(const char *name); extern void LWLockInitialize(LWLock *lock, int tranche_id); /* diff --git a/src/test/modules/test_dsa/test_dsa.c b/src/test/modules/test_dsa/test_dsa.c index cd24d0f4873..01d5c6fa67f 100644 --- a/src/test/modules/test_dsa/test_dsa.c +++ b/src/test/modules/test_dsa/test_dsa.c @@ -29,8 +29,7 @@ test_dsa_basic(PG_FUNCTION_ARGS) dsa_pointer p[100]; /* XXX: this tranche is leaked */ - tranche_id = LWLockNewTrancheId(); - LWLockRegisterTranche(tranche_id, "test_dsa"); + tranche_id = LWLockNewTrancheId("test_dsa"); a = dsa_create(tranche_id); for (int i = 0; i < 100; i++) @@ -70,8 +69,7 @@ test_dsa_resowners(PG_FUNCTION_ARGS) ResourceOwner childowner; /* XXX: this tranche is leaked */ - tranche_id = LWLockNewTrancheId(); - LWLockRegisterTranche(tranche_id, "test_dsa"); + tranche_id = LWLockNewTrancheId("test_dsa"); /* Create DSA in parent resource owner */ a = dsa_create(tranche_id); diff --git a/src/test/modules/test_dsm_registry/test_dsm_registry.c b/src/test/modules/test_dsm_registry/test_dsm_registry.c index 141c8ed1b34..4cc2ccdac3f 100644 --- a/src/test/modules/test_dsm_registry/test_dsm_registry.c +++ b/src/test/modules/test_dsm_registry/test_dsm_registry.c @@ -48,7 +48,7 @@ init_tdr_dsm(void *ptr) { TestDSMRegistryStruct *dsm = (TestDSMRegistryStruct *) ptr; - LWLockInitialize(&dsm->lck, LWLockNewTrancheId()); + LWLockInitialize(&dsm->lck, LWLockNewTrancheId("test_dsm_registry")); dsm->val = 0; } @@ -61,7 +61,6 @@ tdr_attach_shmem(void) sizeof(TestDSMRegistryStruct), init_tdr_dsm, &found); - LWLockRegisterTranche(tdr_dsm->lck.tranche, "test_dsm_registry"); if (tdr_dsa == NULL) tdr_dsa = GetNamedDSA("test_dsm_registry_dsa", &found); diff --git a/src/test/modules/test_radixtree/test_radixtree.c b/src/test/modules/test_radixtree/test_radixtree.c index 80ad0296164..787162c8793 100644 --- a/src/test/modules/test_radixtree/test_radixtree.c +++ b/src/test/modules/test_radixtree/test_radixtree.c @@ -124,10 +124,9 @@ test_empty(void) rt_iter *iter; uint64 key; #ifdef TEST_SHARED_RT - int tranche_id = LWLockNewTrancheId(); + int tranche_id = LWLockNewTrancheId("test_radix_tree"); dsa_area *dsa; - LWLockRegisterTranche(tranche_id, "test_radix_tree"); dsa = dsa_create(tranche_id); radixtree = rt_create(dsa, tranche_id); #else @@ -167,10 +166,9 @@ test_basic(rt_node_class_test_elem *test_info, int shift, bool asc) uint64 *keys; int children = test_info->nkeys; #ifdef TEST_SHARED_RT - int tranche_id = LWLockNewTrancheId(); + int tranche_id = LWLockNewTrancheId("test_radix_tree"); dsa_area *dsa; - LWLockRegisterTranche(tranche_id, "test_radix_tree"); dsa = dsa_create(tranche_id); radixtree = rt_create(dsa, tranche_id); #else @@ -304,10 +302,9 @@ test_random(void) int num_keys = 100000; uint64 *keys; #ifdef TEST_SHARED_RT - int tranche_id = LWLockNewTrancheId(); + int tranche_id = LWLockNewTrancheId("test_radix_tree"); dsa_area *dsa; - LWLockRegisterTranche(tranche_id, "test_radix_tree"); dsa = dsa_create(tranche_id); radixtree = rt_create(dsa, tranche_id); #else diff --git a/src/test/modules/test_slru/test_slru.c b/src/test/modules/test_slru/test_slru.c index 32750930e43..8c0367eeee4 100644 --- a/src/test/modules/test_slru/test_slru.c +++ b/src/test/modules/test_slru/test_slru.c @@ -232,11 +232,9 @@ test_slru_shmem_startup(void) (void) MakePGDirectory(slru_dir_name); /* initialize the SLRU facility */ - test_tranche_id = LWLockNewTrancheId(); - LWLockRegisterTranche(test_tranche_id, "test_slru_tranche"); + test_tranche_id = LWLockNewTrancheId("test_slru_tranche"); - test_buffer_tranche_id = LWLockNewTrancheId(); - LWLockRegisterTranche(test_tranche_id, "test_buffer_tranche"); + test_buffer_tranche_id = LWLockNewTrancheId("test_buffer_tranche"); TestSlruCtl->PagePrecedes = test_slru_page_precedes_logically; SimpleLruInit(TestSlruCtl, "TestSLRU", diff --git a/src/test/modules/test_tidstore/test_tidstore.c b/src/test/modules/test_tidstore/test_tidstore.c index eb16e0fbfa6..0c8f43867e5 100644 --- a/src/test/modules/test_tidstore/test_tidstore.c +++ b/src/test/modules/test_tidstore/test_tidstore.c @@ -103,8 +103,7 @@ test_create(PG_FUNCTION_ARGS) { int tranche_id; - tranche_id = LWLockNewTrancheId(); - LWLockRegisterTranche(tranche_id, "test_tidstore"); + tranche_id = LWLockNewTrancheId("test_tidstore"); tidstore = TidStoreCreateShared(tidstore_max_size, tranche_id); -- 2.39.5 (Apple Git-154) --+17XTdpDH5Q/lDP5 Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename=v21-0002-Tests-for-LWLock-tranche-registration-improvemen.patch ^ permalink raw reply [nested|flat] 5+ messages in thread
end of thread, other threads:[~2025-08-27 20:13 UTC | newest] Thread overview: 5+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2024-03-14 16:59 [PATCH v6 13/14] v7 Streaming Read API Melanie Plageman <[email protected]> 2025-08-27 20:13 [PATCH v16 2/2] Move dynamically-allocated tranche names to shared memory. Nathan Bossart <[email protected]> 2025-08-27 20:13 [PATCH v19 1/2] Move dynamically-allocated tranche names to shared memory. Nathan Bossart <[email protected]> 2025-08-27 20:13 [PATCH v20 1/2] Move dynamically-allocated tranche names to shared memory. Nathan Bossart <[email protected]> 2025-08-27 20:13 [PATCH v21 1/2] Move dynamically-allocated tranche names to shared memory. Nathan Bossart <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox