v2-0007-bufmgr-Move-relation-extension-handling-into-Bulk.patch
+static BlockNumber
+BulkExtendSharedRelationBuffered(Relation rel,
+ SMgrRelation
smgr,
+ bool
skip_extension_lock,
+ char
relpersistence,
+ ForkNumber
fork, ReadBufferMode mode,
+
BufferAccessStrategy strategy,
+ uint32
*num_pages,
+ uint32
num_locked_pages,
+ Buffer
*buffers)
Ugh, that's a lot of arguments, some are inputs and some are outputs. I
don't have any concrete suggestions, but could we simplify this somehow?
Needs a comment at least.
v2-0008-Convert-a-few-places-to-ExtendRelationBuffered.patch
diff --git a/src/backend/access/brin/brin.c b/src/backend/access/brin/brin.c
index de1427a1e0e..1810f7ebfef 100644
--- a/src/backend/access/brin/brin.c
+++ b/src/backend/access/brin/brin.c
@@ -829,9 +829,11 @@ brinbuild(Relation heap, Relation index, IndexInfo
*indexInfo)
* whole relation will be rolled back.
*/
- meta = ReadBuffer(index, P_NEW);
+ meta = ExtendRelationBuffered(index, NULL, true,
+
index->rd_rel->relpersistence,
+ MAIN_FORKNUM,
RBM_ZERO_AND_LOCK,
+ NULL);
Assert(BufferGetBlockNumber(meta) == BRIN_METAPAGE_BLKNO);
- LockBuffer(meta, BUFFER_LOCK_EXCLUSIVE);
brin_metapage_init(BufferGetPage(meta), BrinGetPagesPerRange(index),
BRIN_CURRENT_VERSION);
Since we're changing the API anyway, how about introducing a new
function for this case where we extend the relation but we what block
number we're going to get? This pattern of using P_NEW and asserting the
result has always felt awkward to me.
- buf = ReadBuffer(irel, P_NEW);
+ buf = ExtendRelationBuffered(irel, NULL, false,
+
irel->rd_rel->relpersistence,
+
MAIN_FORKNUM, RBM_ZERO_AND_LOCK,
+ NULL);
These new calls are pretty verbose, compared to ReadBuffer(rel, P_NEW).
I'd suggest something like:
buf = ExtendBuffer(rel);
Do other ReadBufferModes than RBM_ZERO_AND_LOCK make sense with
ExtendRelationBuffered?
Is it ever possible to call this without a relcache entry? WAL redo
functions do that with ReadBuffer, but they only extend a relation
implicitly, by replay a record for a particular block.
All of the above comments are around the BulkExtendRelationBuffered()
function's API. That needs a closer look and a more thought-out design
to make it nice. Aside from that, this approach seems valid.
- Heikki
