public inbox for [email protected]
help / color / mirror / Atom feed[PATCH 03/18] pg_restore: "must be specified" and --list
2+ messages / 2 participants
[nested] [flat]
* [PATCH 03/18] pg_restore: "must be specified" and --list
@ 2020-12-06 04:43 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 2+ messages in thread
From: Justin Pryzby @ 2020-12-06 04:43 UTC (permalink / raw)
This was discussed here, but the idea got lost.
https://www.postgresql.org/message-id/flat/20190612170201.GA11881%40alvherre.pgsql#2984347ab074e6f19...
---
src/bin/pg_dump/pg_restore.c | 2 +-
src/bin/pg_dump/t/001_basic.pl | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/bin/pg_dump/pg_restore.c b/src/bin/pg_dump/pg_restore.c
index 589b4aed53..f6e6e41329 100644
--- a/src/bin/pg_dump/pg_restore.c
+++ b/src/bin/pg_dump/pg_restore.c
@@ -305,7 +305,7 @@ main(int argc, char **argv)
/* Complain if neither -f nor -d was specified (except if dumping TOC) */
if (!opts->cparams.dbname && !opts->filename && !opts->tocSummary)
{
- pg_log_error("one of -d/--dbname and -f/--file must be specified");
+ pg_log_error("one of -d/--dbname, -f/--file, or -l/--list must be specified");
exit_nicely(1);
}
diff --git a/src/bin/pg_dump/t/001_basic.pl b/src/bin/pg_dump/t/001_basic.pl
index 083fb3ad08..8280914c2a 100644
--- a/src/bin/pg_dump/t/001_basic.pl
+++ b/src/bin/pg_dump/t/001_basic.pl
@@ -63,8 +63,8 @@ command_fails_like(
command_fails_like(
['pg_restore'],
- qr{\Qpg_restore: error: one of -d/--dbname and -f/--file must be specified\E},
- 'pg_restore: error: one of -d/--dbname and -f/--file must be specified');
+ qr{\Qpg_restore: error: one of -d/--dbname, -f/--file, or -l/--list must be specified\E},
+ 'pg_restore: error: one of -d/--dbname, -f/--file, or -l/--list must be specified');
command_fails_like(
[ 'pg_restore', '-s', '-a', '-f -' ],
--
2.17.0
--lc9FT7cWel8HagAv
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="0004-typos-in-master.patch"
^ permalink raw reply [nested|flat] 2+ messages in thread
* Fix bank selection logic in SLRU
@ 2024-12-10 12:39 Yura Sokolov <[email protected]>
0 siblings, 0 replies; 2+ messages in thread
From: Yura Sokolov @ 2024-12-10 12:39 UTC (permalink / raw)
To: [email protected] <[email protected]>; +Cc: Alvaro Herrera <[email protected]>; Dilip Kumar <[email protected]>; Andrey M. Borodin <[email protected]>
Good day, hackers.
Due to long discussion about SLRU size configuration [1] and code
evolution, non-serious bug were introduced:
- intermediate versions assumed cache size is always power of 2,
- therefore to determine bank simple binary-and with mask were used:
bankno = pageno & ctl->bank_mask;
- final merged version allows arbitrary cache size for every cache
type, but code for bankno were not fixed.
It is not critical bug, since it doesn't hurt correctness just
performance. In worst case only one bank will be used.
I attach the patch, that changes SlruCtlData->bank_mask to ->nbanks,
and changes calculation to modulo operation.
bankno = pageno % ctl->nbanks;
Probably, instead of modulo operation could be used multiplication:
bankno = ((uint64) murmurhash32(pageno) * ctl->nbanks) >> 32;
But I didn't bother to measure does it pay for or not.
[1]
https://www.postgresql.org/message-id/flat/CAFiTN-vzDvNz%3DExGXz6gdyjtzGixKSqs0mKHMmaQ8sOSEFZ33A%40m...
Regards,
Yura Sokolov aka funny-falcon
Attachments:
[text/x-patch] v1-0001-Fix-SLRU-bank-selection.patch (2.4K, ../../[email protected]/2-v1-0001-Fix-SLRU-bank-selection.patch)
download | inline diff:
From 4b438e67a79d77bf2caf3e5a0386bb7700d329ba Mon Sep 17 00:00:00 2001
From: Yura Sokolov <[email protected]>
Date: Tue, 10 Dec 2024 15:38:02 +0300
Subject: [PATCH v1] Fix SLRU bank selection.
Due to code evolution, nbanks is not power of two any more. But still
binary & were used to obtain bankno.
---
src/backend/access/transam/slru.c | 6 +++---
src/include/access/slru.h | 6 +++---
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/src/backend/access/transam/slru.c b/src/backend/access/transam/slru.c
index e7f73bf4275..afedb5c039f 100644
--- a/src/backend/access/transam/slru.c
+++ b/src/backend/access/transam/slru.c
@@ -343,7 +343,7 @@ SimpleLruInit(SlruCtl ctl, const char *name, int nslots, int nlsns,
ctl->shared = shared;
ctl->sync_handler = sync_handler;
ctl->long_segment_names = long_segment_names;
- ctl->bank_mask = (nslots / SLRU_BANK_SIZE) - 1;
+ ctl->nbanks = nbanks;
strlcpy(ctl->Dir, subdir, sizeof(ctl->Dir));
}
@@ -606,7 +606,7 @@ SimpleLruReadPage_ReadOnly(SlruCtl ctl, int64 pageno, TransactionId xid)
{
SlruShared shared = ctl->shared;
LWLock *banklock = SimpleLruGetBankLock(ctl, pageno);
- int bankno = pageno & ctl->bank_mask;
+ int bankno = pageno % ctl->nbanks;
int bankstart = bankno * SLRU_BANK_SIZE;
int bankend = bankstart + SLRU_BANK_SIZE;
@@ -1177,7 +1177,7 @@ SlruSelectLRUPage(SlruCtl ctl, int64 pageno)
int bestinvalidslot = 0; /* keep compiler quiet */
int best_invalid_delta = -1;
int64 best_invalid_page_number = 0; /* keep compiler quiet */
- int bankno = pageno & ctl->bank_mask;
+ int bankno = pageno % ctl->nbanks;
int bankstart = bankno * SLRU_BANK_SIZE;
int bankend = bankstart + SLRU_BANK_SIZE;
diff --git a/src/include/access/slru.h b/src/include/access/slru.h
index 97e612cd100..fabea220290 100644
--- a/src/include/access/slru.h
+++ b/src/include/access/slru.h
@@ -129,9 +129,9 @@ typedef struct SlruCtlData
SlruShared shared;
/*
- * Bitmask to determine bank number from page number.
+ * Number of banks to determine bank number from page number.
*/
- bits16 bank_mask;
+ bits16 nbanks;
/*
* If true, use long segment file names. Otherwise, use short file names.
@@ -179,7 +179,7 @@ SimpleLruGetBankLock(SlruCtl ctl, int64 pageno)
{
int bankno;
- bankno = pageno & ctl->bank_mask;
+ bankno = pageno % ctl->nbanks;
return &(ctl->shared->bank_locks[bankno].lock);
}
--
2.43.0
^ permalink raw reply [nested|flat] 2+ messages in thread
end of thread, other threads:[~2024-12-10 12:39 UTC | newest]
Thread overview: 2+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-12-06 04:43 [PATCH 03/18] pg_restore: "must be specified" and --list Justin Pryzby <[email protected]>
2024-12-10 12:39 Fix bank selection logic in SLRU Yura Sokolov <[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