Skip to content

Commit

Permalink
Use smgrexists() instead of access() to enforce uniqueness of generat…
Browse files Browse the repository at this point in the history
…ed relfilenumber (#7992)

## Problem

Postgres is using `access()` function in `GetNewRelFileNumber` to check
if assigned relfilenumber is not used for any other relation. This check
will not work in Neon, because we do not have all files in local
storage.

## Summary of changes

Use smgrexists() instead which will check at page server if such
relfilenode is used.

## Checklist before requesting a review

- [ ] I have performed a self-review of my code.
- [ ] If it is a core feature, I have added thorough tests.
- [ ] Do we need to implement analytics? if so did you add the relevant
metrics to the dashboard?
- [ ] If this PR requires public announcement, mark it with
/release-notes label and add several sentences in this section.

## Checklist before merging

- [ ] Do not forget to reformat commit message to not include the above
checklist

---------

Co-authored-by: Konstantin Knizhnik <[email protected]>
  • Loading branch information
knizhnik and Konstantin Knizhnik authored Jul 23, 2024
1 parent 1a4c1eb commit 563d73d
Show file tree
Hide file tree
Showing 8 changed files with 83 additions and 6 deletions.
10 changes: 10 additions & 0 deletions pageserver/src/pgdatadir_mapping.rs
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,16 @@ impl Timeline {
if let Some(_nblocks) = self.get_cached_rel_size(&tag, version.get_lsn()) {
return Ok(true);
}
// then check if the database was already initialized.
// get_rel_exists can be called before dbdir is created.
let buf = version.get(self, DBDIR_KEY, ctx).await?;
let dbdirs = match DbDirectory::des(&buf).context("deserialization failure") {
Ok(dir) => Ok(dir.dbdirs),
Err(e) => Err(PageReconstructError::from(e)),
}?;
if !dbdirs.contains_key(&(tag.spcnode, tag.dbnode)) {
return Ok(false);
}
// fetch directory listing
let key = rel_dir_to_key(tag.spcnode, tag.dbnode);
let buf = version.get(self, key, ctx).await?;
Expand Down
6 changes: 6 additions & 0 deletions pgxn/neon_test_utils/neon_test_utils--1.3.sql
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,12 @@ AS 'MODULE_PATHNAME', 'test_consume_xids'
LANGUAGE C STRICT
PARALLEL UNSAFE;

CREATE FUNCTION test_consume_oids(oid int)
RETURNS VOID
AS 'MODULE_PATHNAME', 'test_consume_oids'
LANGUAGE C STRICT
PARALLEL UNSAFE;

CREATE FUNCTION test_consume_cpu(seconds int)
RETURNS VOID
AS 'MODULE_PATHNAME', 'test_consume_cpu'
Expand Down
16 changes: 16 additions & 0 deletions pgxn/neon_test_utils/neontest.c
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ PG_MODULE_MAGIC;
extern void _PG_init(void);

PG_FUNCTION_INFO_V1(test_consume_xids);
PG_FUNCTION_INFO_V1(test_consume_oids);
PG_FUNCTION_INFO_V1(test_consume_cpu);
PG_FUNCTION_INFO_V1(test_consume_memory);
PG_FUNCTION_INFO_V1(test_release_memory);
Expand Down Expand Up @@ -74,6 +75,21 @@ _PG_init(void)

#define neon_read_at_lsn neon_read_at_lsn_ptr

/*
* test_consume_oids(int4), for rapidly consuming OIDs, to test wraparound.
* Unlike test_consume_xids which is passed number of xids to be consumed,
* this function is given the target Oid.
*/
Datum
test_consume_oids(PG_FUNCTION_ARGS)
{
int32 oid = PG_GETARG_INT32(0);

while (oid != GetNewObjectId());

PG_RETURN_VOID();
}

/*
* test_consume_xids(int4), for rapidly consuming XIDs, to test wraparound.
*/
Expand Down
45 changes: 45 additions & 0 deletions test_runner/regress/test_oid_overflow.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
from fixtures.log_helper import log
from fixtures.neon_fixtures import NeonEnvBuilder


def test_oid_overflow(neon_env_builder: NeonEnvBuilder):
env = neon_env_builder.init_start()

endpoint = env.endpoints.create_start("main")

conn = endpoint.connect()
cur = conn.cursor()

cur.execute("CREATE EXTENSION neon_test_utils")

cur.execute("CREATE TABLE t1(x integer)")
cur.execute("INSERT INTO t1 values (1)")
cur.execute("CREATE TABLE t2(x integer)")
cur.execute("INSERT INTO t2 values (2)")

cur.execute("SELECT x from t1")
assert cur.fetchone() == (1,)
cur.execute("SELECT x from t2")
assert cur.fetchone() == (2,)

cur.execute("VACUUM FULL t1")
cur.execute("VACUUM FULL t1")
cur.execute("vacuum pg_class")
cur.execute("SELECT relfilenode FROM pg_class where relname='t1'")
oid = cur.fetchall()[0][0]
log.info(f"t1.relfilenode={oid}")

cur.execute("set statement_timeout=0")
cur.execute(f"select test_consume_oids({oid-1})")
cur.execute("VACUUM FULL t2")

cur.execute("SELECT relfilenode FROM pg_class where relname='t2'")
oid = cur.fetchall()[0][0]
log.info(f"t2.relfilenode={oid}")

cur.execute("SELECT clear_buffer_cache()")

cur.execute("SELECT x from t1")
assert cur.fetchone() == (1,)
cur.execute("SELECT x from t2")
assert cur.fetchone() == (2,)
2 changes: 1 addition & 1 deletion vendor/postgres-v14
2 changes: 1 addition & 1 deletion vendor/postgres-v15
2 changes: 1 addition & 1 deletion vendor/postgres-v16
6 changes: 3 additions & 3 deletions vendor/revisions.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"v16": ["16.3", "b810fdfcbb59afea7ea7bbe0cf94eaccb55a2ea2"],
"v15": ["15.7", "4874c8e52ed349a9f8290bbdcd91eb92677a5d24"],
"v14": ["14.12", "ad73770c446ea361f43e4f0404798b7e5e7a62d8"]
"v16": ["16.3", "b39f316137fdd29e2da15d2af2fdd1cfd18163be"],
"v15": ["15.7", "035b73a9c5998f9a0ef35cc8df1bae680bf770fc"],
"v14": ["14.12", "dbd0e6428b9274d72a10ac29bd3e3162faf109d4"]
}

1 comment on commit 563d73d

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3214 tests run: 3080 passed, 1 failed, 133 skipped (full report)


Failures on Postgres 16

  • test_multixid_wraparound_import: release
# Run all failed tests locally:
scripts/pytest -vv -n $(nproc) -k "test_multixid_wraparound_import[release-pg16]"

Test coverage report is not available

The comment gets automatically updated with the latest test results
563d73d at 2024-07-23T17:14:34.254Z :recycle:

Please sign in to comment.