Skip to content

Commit e152506

Browse files
michaelpqmichaelpq
committed
Revert pg_relation_check_pages()
This reverts the following set of commits, following complaints about the lack of portability of the central part of the code in bufmgr.c as well as the use of partition mapping locks during page reads: c780a7a f2b8839 b787d4c ce7f772 60a51c6 Per discussion with Andres Freund, Robert Haas and myself. Bump catalog version. Discussion: https://postgr.es/m/20201029181729.2nrub47u7yqncsv7@alap3.anarazel.de
1 parent 90851d1 commit e152506

File tree

14 files changed

+2
-745
lines changed

14 files changed

+2
-745
lines changed

‎doc/src/sgml/func.sgml

-56
Original file line numberDiff line numberDiff line change
@@ -26171,62 +26171,6 @@ SELECT convert_from(pg_read_binary_file('file_in_utf8.txt'), 'UTF8');
2617126171

2617226172
</sect2>
2617326173

26174-
<sect2 id="functions-data-sanity">
26175-
<title>Data Sanity Functions</title>
26176-
26177-
<para>
26178-
The functions shown in <xref linkend="functions-data-sanity-table"/>
26179-
provide ways to check the sanity of data files in the cluster.
26180-
</para>
26181-
26182-
<table id="functions-data-sanity-table">
26183-
<title>Data Sanity Functions</title>
26184-
<tgroup cols="1">
26185-
<thead>
26186-
<row>
26187-
<entry role="func_table_entry"><para role="func_signature">
26188-
Function
26189-
</para>
26190-
<para>
26191-
Description
26192-
</para></entry>
26193-
</row>
26194-
</thead>
26195-
26196-
<tbody>
26197-
<row>
26198-
<entry role="func_table_entry"><para role="func_signature">
26199-
<indexterm>
26200-
<primary>pg_relation_check_pages</primary>
26201-
</indexterm>
26202-
<function>pg_relation_check_pages</function> ( <parameter>relation</parameter> <type>regclass</type> [, <parameter>fork</parameter> <type>text</type> ] )
26203-
<returnvalue>setof record</returnvalue>
26204-
( <parameter>path</parameter> <type>text</type>,
26205-
<parameter>failed_block_num</parameter> <type>bigint</type> )
26206-
</para>
26207-
<para>
26208-
Checks the pages of the specified relation to see if they are valid
26209-
enough to safely be loaded into the server's shared buffers. If
26210-
given, <parameter>fork</parameter> specifies that only the pages of
26211-
the given fork are to be verified. <parameter>fork</parameter> can
26212-
be <literal>main</literal> for the main data
26213-
fork, <literal>fsm</literal> for the free space
26214-
map, <literal>vm</literal> for the visibility map,
26215-
or <literal>init</literal> for the initialization fork. The
26216-
default of <literal>NULL</literal> means that all forks of the
26217-
relation should be checked. The function returns a list of block
26218-
numbers that appear corrupted along with the path names of their
26219-
files. Use of this function is restricted to superusers by
26220-
default, but access may be granted to others
26221-
using <command>GRANT</command>.
26222-
</para></entry>
26223-
</row>
26224-
</tbody>
26225-
</tgroup>
26226-
</table>
26227-
26228-
</sect2>
26229-
2623026174
</sect1>
2623126175

2623226176
<sect1 id="functions-trigger">

‎src/backend/catalog/system_views.sql

-9
Original file line numberDiff line numberDiff line change
@@ -1303,14 +1303,6 @@ LANGUAGE INTERNAL
13031303
STRICT VOLATILE
13041304
AS 'pg_create_logical_replication_slot';
13051305

1306-
CREATE OR REPLACE FUNCTION pg_relation_check_pages(
1307-
IN relation regclass, IN fork text DEFAULT NULL,
1308-
OUT path text, OUT failed_block_num bigint)
1309-
RETURNS SETOF record
1310-
LANGUAGE internal
1311-
VOLATILE PARALLEL RESTRICTED
1312-
AS 'pg_relation_check_pages';
1313-
13141306
CREATE OR REPLACE FUNCTION
13151307
make_interval(years int4 DEFAULT 0, months int4 DEFAULT 0, weeks int4 DEFAULT 0,
13161308
days int4 DEFAULT 0, hours int4 DEFAULT 0, mins int4 DEFAULT 0,
@@ -1455,7 +1447,6 @@ AS 'unicode_is_normalized';
14551447
-- can later change who can access these functions, or leave them as only
14561448
-- available to superuser / cluster owner, if they choose.
14571449
--
1458-
REVOKE EXECUTE ON FUNCTION pg_relation_check_pages(regclass, text) FROM public;
14591450
REVOKE EXECUTE ON FUNCTION pg_start_backup(text, boolean, boolean) FROM public;
14601451
REVOKE EXECUTE ON FUNCTION pg_stop_backup() FROM public;
14611452
REVOKE EXECUTE ON FUNCTION pg_stop_backup(boolean, boolean) FROM public;

‎src/backend/storage/buffer/bufmgr.c

-92
Original file line numberDiff line numberDiff line change
@@ -4585,95 +4585,3 @@ TestForOldSnapshot_impl(Snapshot snapshot, Relation relation)
45854585
(errcode(ERRCODE_SNAPSHOT_TOO_OLD),
45864586
errmsg("snapshot too old")));
45874587
}
4588-
4589-
4590-
/*
4591-
* CheckBuffer
4592-
*
4593-
* Check the state of a buffer without loading it into the shared buffers. To
4594-
* avoid torn pages and possible false positives when reading data, a shared
4595-
* LWLock is taken on the target buffer pool partition mapping, and we check
4596-
* if the page is in shared buffers or not. An I/O lock is taken on the block
4597-
* to prevent any concurrent activity from happening.
4598-
*
4599-
* If the page is found as dirty in the shared buffers, it is ignored as
4600-
* it will be flushed to disk either before the end of the next checkpoint
4601-
* or during recovery in the event of an unsafe shutdown.
4602-
*
4603-
* If the page is found in the shared buffers but is not dirty, we still
4604-
* check the state of its data on disk, as it could be possible that the
4605-
* page stayed in shared buffers for a rather long time while the on-disk
4606-
* data got corrupted.
4607-
*
4608-
* If the page is not found in shared buffers, the block is read from disk
4609-
* while holding the buffer pool partition mapping LWLock.
4610-
*
4611-
* The page data is stored in a private memory area local to this function
4612-
* while running the checks.
4613-
*/
4614-
bool
4615-
CheckBuffer(SMgrRelation smgr, ForkNumber forknum, BlockNumber blkno)
4616-
{
4617-
char buffer[BLCKSZ];
4618-
BufferTag buf_tag; /* identity of requested block */
4619-
uint32 buf_hash; /* hash value for buf_tag */
4620-
LWLock *partLock; /* buffer partition lock for the buffer */
4621-
BufferDesc *bufdesc;
4622-
int buf_id;
4623-
4624-
Assert(smgrexists(smgr, forknum));
4625-
4626-
/* create a tag so we can look after the buffer */
4627-
INIT_BUFFERTAG(buf_tag, smgr->smgr_rnode.node, forknum, blkno);
4628-
4629-
/* determine its hash code and partition lock ID */
4630-
buf_hash = BufTableHashCode(&buf_tag);
4631-
partLock = BufMappingPartitionLock(buf_hash);
4632-
4633-
/* see if the block is in the buffer pool or not */
4634-
LWLockAcquire(partLock, LW_SHARED);
4635-
buf_id = BufTableLookup(&buf_tag, buf_hash);
4636-
if (buf_id >= 0)
4637-
{
4638-
uint32 buf_state;
4639-
4640-
/*
4641-
* Found it. Now, retrieve its state to know what to do with it, and
4642-
* release the pin immediately. We do so to limit overhead as much as
4643-
* possible. We keep the shared LWLock on the target buffer mapping
4644-
* partition for now, so this buffer cannot be evicted, and we acquire
4645-
* an I/O Lock on the buffer as we may need to read its contents from
4646-
* disk.
4647-
*/
4648-
bufdesc = GetBufferDescriptor(buf_id);
4649-
4650-
LWLockAcquire(BufferDescriptorGetIOLock(bufdesc), LW_SHARED);
4651-
buf_state = LockBufHdr(bufdesc);
4652-
UnlockBufHdr(bufdesc, buf_state);
4653-
4654-
/* If the page is dirty or invalid, skip it */
4655-
if ((buf_state & BM_DIRTY) != 0 || (buf_state & BM_TAG_VALID) == 0)
4656-
{
4657-
LWLockRelease(BufferDescriptorGetIOLock(bufdesc));
4658-
LWLockRelease(partLock);
4659-
return true;
4660-
}
4661-
4662-
/* Read the buffer from disk, with the I/O lock still held */
4663-
smgrread(smgr, forknum, blkno, buffer);
4664-
LWLockRelease(BufferDescriptorGetIOLock(bufdesc));
4665-
}
4666-
else
4667-
{
4668-
/*
4669-
* Simply read the buffer. There's no risk of modification on it as
4670-
* we are holding the buffer pool partition mapping lock.
4671-
*/
4672-
smgrread(smgr, forknum, blkno, buffer);
4673-
}
4674-
4675-
/* buffer lookup done, so now do its check */
4676-
LWLockRelease(partLock);
4677-
4678-
return PageIsVerifiedExtended(buffer, blkno, PIV_REPORT_STAT);
4679-
}

‎src/backend/utils/adt/Makefile

-1
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,6 @@ OBJS = \
6969
oid.o \
7070
oracle_compat.o \
7171
orderedsetaggs.o \
72-
pagefuncs.o \
7372
partitionfuncs.o \
7473
pg_locale.o \
7574
pg_lsn.o \

0 commit comments

Comments
 (0)