Re: How to accurately determine when a relation should use local buffers?

From: Aleksander Alekseev <aleksander(at)timescale(dot)com>
To: pgsql-hackers(at)lists(dot)postgresql(dot)org
Cc: Давыдов Виталий <v(dot)davydov(at)postgrespro(dot)ru>, Vitaly Davydov <vitprof(at)gmail(dot)com>
Subject: Re: How to accurately determine when a relation should use local buffers?
Date: 2023-11-21 15:01:30
Message-ID: CAJ7c6TOVSupn0tvLzwxCUfjPqPUy8L0L4xKAws62r8rQiMVrHA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

> Furthermore, there is some possible inconsistency in the code show below (REL_16_STABLE) in bufmgr.c file:
>
> FlushRelationBuffers, PrefetchBuffer uses RelationUsesLocalBuffers(rel).
> ExtendBufferedRel_common finally use BufferManagerRelation.relpersistence which is actually rd_rel->relpersistence, works like RelationUsesLocalBuffers.
> ReadBuffer_common uses isLocalBuf = SmgrIsTemp(smgr), that checks rlocator.backend for InvalidBackendId.

I didn't do a deep investigation of the code in this particular aspect
but that could be a fair point. Would you like to propose a
refactoring that unifies the way we check if the relation is
temporary?

> I would like to clarify, do we completely refuse the use of temporary tables in other contexts than in backends or there is some work-in-progress to allow some other usage contexts? If so, the check of rd_rel->relpersistence is enough. Not sure why we use SmgrIsTemp instead of RelationUsesLocalBuffers in ReadBuffer_common.

According to the comments in relfilelocator.h:

```
/*
* Augmenting a relfilelocator with the backend ID provides all the information
* we need to locate the physical storage. The backend ID is InvalidBackendId
* for regular relations (those accessible to more than one backend), or the
* owning backend's ID for backend-local relations. Backend-local relations
* are always transient and removed in case of a database crash; they are
* never WAL-logged or fsync'd.
*/
typedef struct RelFileLocatorBackend
{
RelFileLocator locator;
BackendId backend;
} RelFileLocatorBackend;

#define RelFileLocatorBackendIsTemp(rlocator) \
((rlocator).backend != InvalidBackendId)
```

And this is what ReadBuffer_common() and other callers of SmgrIsTemp()
are using. So no, you can't have a temporary table without an assigned
RelFileLocatorBackend.backend.

It is my understanding that SmgrIsTemp() and
RelationUsesLocalBuffers() are equivalent except the fact that the
first macro works with SMgrRelation objects and the second one - with
Relation objects.

--
Best regards,
Aleksander Alekseev

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Eric Ridge 2023-11-21 15:14:32 Re: Hide exposed impl detail of wchar.c
Previous Message Peter Eisentraut 2023-11-21 14:59:20 Re: Annoying build warnings from latest Apple toolchain