Re: pg_ls_tmpdir to show directories and shared filesets (and pg_ls_*)

2021-11-22 Thread Bossart, Nathan
In an attempt to get this patch set off the ground again, I took a look at the first 5 patches. 0001: This one is a very small documentation update for pg_stat_file to point out that isdir will be true for symbolic links to directories. Given this is true, I think the patch looks good. 0002:

Re: Improving psql's \password command

2021-11-20 Thread Bossart, Nathan
On 11/20/21, 1:58 PM, "Tom Lane" wrote: > "Bossart, Nathan" writes: >> I did find some missing control-C handling in >> pg_receivewal/pg_recvlogical, though. Attached is a patch for those. > > Meh ... I'm inclined to fix those programs by just mo

Re: logical decoding/replication: new functions pg_ls_logicaldir and pg_ls_replslotdir

2021-11-19 Thread Bossart, Nathan
On 11/17/21, 11:39 PM, "Bharath Rupireddy" wrote: > Please review the attached v2. LGTM. I've marked this one as ready-for-committer. Nathan

Re: Pasword expiration warning

2021-11-19 Thread Bossart, Nathan
On 11/19/21, 7:56 AM, "Tom Lane" wrote: > That leads me to wonder about server-side solutions. It's easy > enough for the server to see that it's used a password with an > expiration N days away, but how could that be reported to the > client? The only idea that comes to mind that doesn't seem

Re: Should rename "startup process" to something else?

2021-11-19 Thread Bossart, Nathan
On 11/18/21, 12:24 PM, "Tom Lane" wrote: > Alvaro Herrera writes: >> If we change the name, and I support the idea that we do, I think a >> good name would be "wal replay". I think "recovery" is not great >> precisely because in a standby there is likely no crash that we're >> recovering from.

Re: Improving psql's \password command

2021-11-17 Thread Bossart, Nathan
On 11/17/21, 4:15 PM, "Tom Lane" wrote: > Pushed with a little further tweaking --- mostly, I felt that > explicitly referring to SIGINT in the API names was too > implementation-specific, so I renamed things. Thanks! > As you mentioned, there are several other simple_prompt() calls > that

Re: Add planner support function for starts_with()

2021-11-17 Thread Bossart, Nathan
On 10/9/21, 10:24 AM, "Tom Lane" wrote: > With the (admittedly later) introduction of planner support functions, > it's really quite easy to do better. The attached patch adds a planner > support function for starts_with(), with these benefits: The patch looks reasonable to me. Nathan

Re: logical decoding/replication: new functions pg_ls_logicaldir and pg_ls_replslotdir

2021-11-17 Thread Bossart, Nathan
On 10/30/21, 2:36 AM, "Bharath Rupireddy" wrote: > I've added 3 functions pg_ls_logicalsnapdir, pg_ls_logicalmapdir, > pg_ls_replslotdir, and attached the patch. The sample output looks > like [1]. Please review it further. I took a look at the patch. + charpath[MAXPGPATH +

Re: add recovery, backup, archive, streaming etc. activity messages to server logs along with ps display

2021-11-16 Thread Bossart, Nathan
On 11/15/21, 7:14 PM, "Michael Paquier" wrote: > + ereport(DEBUG1, > + (errmsg_internal("streaming %X/%X", > + > LSN_FORMAT_ARGS(sentPtr; > Anyway, those two ones are going to make the logs much more noisy,

Re: add recovery, backup, archive, streaming etc. activity messages to server logs along with ps display

2021-11-15 Thread Bossart, Nathan
On 11/14/21, 4:18 AM, "Bharath Rupireddy" wrote: > Thanks. Attaching the v2 to avoid that by directly using the message > in ereport instead of activitymsg. +ereport(DEBUG1, +(errmsg_internal("waiting for %s", xlogfname))); As a general comment, I think we

Re: [PATCH v2] use has_privs_for_role for predefined roles

2021-11-15 Thread Bossart, Nathan
On 11/12/21, 12:34 PM, "Joshua Brindle" wrote: > All of these and also adminpack.sgml updated. I think that is all of > them but docs broken across lines and irregular wording makes it > difficult. LGTM. I've marked this as ready-for-committer. Nathan

Re: RecoveryInProgress() has critical side effects

2021-11-15 Thread Bossart, Nathan
On 11/15/21, 1:30 PM, "Robert Haas" wrote: > Here's a new version that does it that way. Any other opinions? LGTM > The best thing I could come up with for a test case for this was to > try repeatedly making a new connection and running "SELECT > txid_current()", which will cause just one WAL

Re: Improving psql's \password command

2021-11-12 Thread Bossart, Nathan
On 11/12/21, 11:58 AM, "Tom Lane" wrote: > "Bossart, Nathan" writes: >> I haven't started on the SIGINT stuff yet, but I did extract the >> PQuser() fix so that we can hopefully get that one out of the way. I >> made some small adjustments in an attempt

Re: RecoveryInProgress() has critical side effects

2021-11-12 Thread Bossart, Nathan
On 11/11/21, 9:20 AM, "Robert Haas" wrote: > memory, and not much is lost if they are set up and not used. So, in > 0001, I propose to move the call to InitXLogInsert() from > InitXLOGAccess() to BaseInit(). That means every backend will do it > the first time it starts up. It also means that

Re: Patch abstracts in the Commitfest app

2021-11-12 Thread Bossart, Nathan
On 11/12/21, 4:53 AM, "Daniel Gustafsson" wrote: > While reading through and working with the hundreds of patches in the CF app a > small feature/process request struck me: it would be really helpful if the > patch had a brief abstract outlining what it aims to add or fix (or summary, >

Re: .ready and .done files considered harmful

2021-11-11 Thread Bossart, Nathan
On 11/11/21, 12:23 PM, "Robert Haas" wrote: > On Thu, Nov 11, 2021 at 2:49 PM Robert Haas wrote: >> Somehow I didn't see your October 19th response previously. The >> threading in gmail seems to have gotten broken, which may have >> contributed. > > And actually I also missed the September 27th

Re: Improving psql's \password command

2021-11-11 Thread Bossart, Nathan
On 10/29/21, 5:07 PM, "Tom Lane" wrote: > "Bossart, Nathan" writes: >> Well, as of bf6b9e9, "ALTER ROLE nathan PASSWORD ''" is effectively >> the same as "ALTER ROLE nathan PASSWORD NULL". I agree about the >> user-unfriendliness

Re: .ready and .done files considered harmful

2021-11-11 Thread Bossart, Nathan
On 10/19/21, 7:53 AM, "Bossart, Nathan" wrote: > On 10/19/21, 5:59 AM, "Robert Haas" wrote: >> Nathan, I just realized we never closed the loop on this. Do you have >> any thoughts? > > IMO the patch is in decent shape. Happy to address any feedbac

Re: add recovery, backup, archive, streaming etc. activity messages to server logs along with ps display

2021-11-11 Thread Bossart, Nathan
On 11/11/21, 4:38 AM, "Alvaro Herrera" wrote: > Yeah. If we had some sort of ring buffer in which to store these > messages, the user could examine them through a view; they would still > be accessible in a running server, but they would not be written to the > server log. That's an interesting

Re: add recovery, backup, archive, streaming etc. activity messages to server logs along with ps display

2021-11-11 Thread Bossart, Nathan
On 11/10/21, 9:51 PM, "Bharath Rupireddy" wrote: > But for the snprintf(activitymsg, sizeof(activitymsg), "archiving %s", > xlog); we have elog(DEBUG1, "archived write-ahead log file \"%s\"", > xlog); after the archiving command. It is also good to have a similar > debug message before archive

Re: Deduplicate code updating ControleFile's DBState.

2021-11-10 Thread Bossart, Nathan
On 10/1/21, 10:40 PM, "Michael Paquier" wrote: > On Fri, Oct 01, 2021 at 05:47:45PM +0000, Bossart, Nathan wrote: >> I'm inclined to agree that anything that calls update_controlfile() >> should update the timestamp. > > pg_control.h tells that: > pg_time_t tim

Re: Isn't it better with "autovacuum worker...." instead of "worker took too long to start; canceled" specific to "auto

2021-11-10 Thread Bossart, Nathan
On 10/28/21, 5:42 AM, "Bharath Rupireddy" wrote: > Thanks all for reviewing this. Here's the CF entry - > https://commitfest.postgresql.org/35/3378/ I've marked this one as ready-for-committer. Nathan

Re: archive modules

2021-11-10 Thread Bossart, Nathan
On 11/10/21, 10:42 AM, "David Steele" wrote: > OK, I haven't had to go over the patch in detail so I didn't realize the > module was not backwards compatible. I'll have a closer look soon. It's backward-compatible in the sense that you'd be able to switch archive_library to "shell" to continue

Re: add recovery, backup, archive, streaming etc. activity messages to server logs along with ps display

2021-11-10 Thread Bossart, Nathan
On 11/10/21, 9:43 AM, "Bharath Rupireddy" wrote: > As discussed in [1], isn't it a better idea to add some of activity > messages [2] such as recovery, archive, backup, streaming etc. to > server logs at LOG level? They are currently being set into ps display > which is good if the postgres is

Re: archive modules

2021-11-10 Thread Bossart, Nathan
On 11/10/21, 8:10 AM, "David Steele" wrote: > On 11/7/21 1:04 AM, Fujii Masao wrote: >> It's helpful if you share how much this approach reduces >> the amount of overhead. > > FWIW we have a test for this in pgBackRest. Running > `archive_command=pgbackrest archive-push ...` 1000 times via

Re: [PATCH v2] use has_privs_for_role for predefined roles

2021-11-10 Thread Bossart, Nathan
On 11/8/21, 2:19 PM, "Joshua Brindle" wrote: > Thanks for the review, attached is an update with that comment fixed > and also sgml documentation changes that I missed earlier. I think there are a number of documentation changes that are still missing. I did a quick scan and saw the "is member

Re: partial heap only tuples

2021-11-10 Thread Bossart, Nathan
On 11/4/21, 3:24 AM, "Daniel Gustafsson" wrote: > As no update has been posted, the patch still doesn't apply. I'm marking this > patch Returned with Feedback, feel free to open a new entry for an updated > patch. Thanks. I have been working on this intermittently, and I hope to post a more

Re: Predefined role pg_maintenance for VACUUM, ANALYZE, CHECKPOINT.

2021-11-02 Thread Bossart, Nathan
On 11/2/21, 11:27 AM, "Stephen Frost" wrote: > * Bossart, Nathan (bossa...@amazon.com) wrote: >> The approach in the patch looks alright to me, but another one could >> be to build a SelectStmt when parsing CHECKPOINT. I think that'd >> simplify the standard_Proce

Re: Predefined role pg_maintenance for VACUUM, ANALYZE, CHECKPOINT.

2021-11-02 Thread Bossart, Nathan
On 11/2/21, 10:29 AM, "Jeff Davis" wrote: > Great idea! Patch attached. > > This feels like a good pattern that we might want to use elsewhere, if > the need arises. The approach in the patch looks alright to me, but another one could be to build a SelectStmt when parsing CHECKPOINT. I think

Re: archive modules

2021-11-02 Thread Bossart, Nathan
On 11/2/21, 9:46 AM, "Robert Haas" wrote: > On Tue, Nov 2, 2021 at 12:39 PM Bossart, Nathan wrote:> >> On 11/2/21, 9:17 AM, "Robert Haas" wrote: >> You could still introduce GUCs in _PG_init(), but they couldn't be >> defined as PGC_POSTMASTER.

Re: archive modules

2021-11-02 Thread Bossart, Nathan
On 11/2/21, 9:17 AM, "Robert Haas" wrote: > On Tue, Nov 2, 2021 at 12:10 PM Bossart, Nathan wrote: >> Yes, that seems doable. My point is that I've intentionally chosen to >> preload the libraries at the moment so that it's possible to define >>

Re: archive modules

2021-11-02 Thread Bossart, Nathan
On 11/2/21, 8:57 AM, "Robert Haas" wrote: > On Tue, Nov 2, 2021 at 11:26 AM Bossart, Nathan wrote: >> Well, the current patch does require a reload since the modules are >> preloaded, but maybe there is some way to avoid that. > > I think we could set things up so

Re: inefficient loop in StandbyReleaseLockList()

2021-11-02 Thread Bossart, Nathan
On 11/2/21, 8:36 AM, "Tom Lane" wrote: > I've pushed the SyncPostCheckpoint change, and I think I'm going > to stop here. It's not clear that the remaining list_delete_first > callers have any real problem; and changing them would be complex. > We can revisit the question if we find out there is

Re: archive modules

2021-11-02 Thread Bossart, Nathan
On 11/2/21, 8:11 AM, "Robert Haas" wrote: > Why in the world would you want a plain hook rather than something > closer to the way logical replication works? > > Plain hooks are annoying to use. If you load things at the wrong time, > it silently doesn't work. It's also impossible to unload

Re: archive modules

2021-11-02 Thread Bossart, Nathan
I've just realized I forgot to CC the active participants on the last thread to this one, so I've attempted to do that now. I didn't intentionally leave anyone out, but I'm sorry if I missed someone. On 11/1/21, 10:24 PM, "Michael Paquier" wrote: > It seems to me that this patch is not moving

Re: archive modules

2021-11-02 Thread Bossart, Nathan
On 11/1/21, 9:44 PM, "Fujii Masao" wrote: > What is the main motivation of this patch? I was thinking that > it's for parallelizing WAL archiving. But as far as I read > the patch very briefly, WAL file name is still passed to > the archive callback function one by one. The main motivation is

Re: parallelizing the archiver

2021-11-01 Thread Bossart, Nathan
On 11/1/21, 10:57 AM, "Stephen Frost" wrote: > Definitely interested and plan to look at this more shortly, and > generally this all sounds good, but maybe we should have it be posted > under a new thread as it's moved pretty far from the subject and folks > might not appreciate what this is

Re: Predefined role pg_maintenance for VACUUM, ANALYZE, CHECKPOINT.

2021-11-01 Thread Bossart, Nathan
On 11/1/21, 10:43 AM, "Stephen Frost" wrote: > Folks playing around in the catalog can break lots of things, I don't > really see this as an argument against the idea. > > I do wonder if we should put a bit more effort into preventing people > from messing with functions and such in pg_catalog.

Re: inefficient loop in StandbyReleaseLockList()

2021-11-01 Thread Bossart, Nathan
On 11/1/21, 10:34 AM, "Tom Lane" wrote: > Thanks for looking! Here's an expanded patch that also takes care > of the other two easy-to-fix cases, nodeAgg.c and llvmjit.c. > AFAICS, llvm_release_context is like StandbyReleaseLockList > in that we don't need to worry about whether the data

Re: Predefined role pg_maintenance for VACUUM, ANALYZE, CHECKPOINT.

2021-11-01 Thread Bossart, Nathan
On 11/1/21, 9:51 AM, "Stephen Frost" wrote: > I don't really buy off on the "because it's been around a long time" as > a reason to invent a predefined role for an individual command that > doesn't take any options and could certainly just be a function. > Applications developed to run as a

Re: inefficient loop in StandbyReleaseLockList()

2021-11-01 Thread Bossart, Nathan
On 11/1/21, 9:58 AM, "Tom Lane" wrote: > "Bossart, Nathan" writes: >> Should there be a list_free(trgmNFA->queue) at the end of >> transformGraph()? > > There could be, but that's visibly invoked only once per > createTrgmNFAInternal call, so I di

Re: inefficient loop in StandbyReleaseLockList()

2021-11-01 Thread Bossart, Nathan
On 10/31/21, 1:55 PM, "Tom Lane" wrote: > 1. Attached is a proposed patch to get rid of the calls in trgm_regexp.c. > I'm not certain that those lists could get long enough to be a problem, > given the existing complexity limits in that file (MAX_EXPANDED_STATES > etc). But I'm not certain they

Re: inefficient loop in StandbyReleaseLockList()

2021-11-01 Thread Bossart, Nathan
On 10/31/21, 12:39 PM, "Tom Lane" wrote: > Yeah, there's no expectation that this data structure needs to be kept > consistent after an error; and I'm not real sure that the existing > code could claim to satisfy such a requirement if we did need it. > (There's at least a short window where the

Re: Predefined role pg_maintenance for VACUUM, ANALYZE, CHECKPOINT.

2021-10-30 Thread Bossart, Nathan
On 10/30/21, 11:14 AM, "Jeff Davis" wrote: > On Sat, 2021-10-30 at 13:24 +0530, Bharath Rupireddy wrote: >> IMHO, moving away from SQL command "CHECKPOINT" to function >> "pg_checkpoint()" isn't nice as the SQL command has been there for a >> long time and all the applications or services that

Re: Improving psql's \password command

2021-10-29 Thread Bossart, Nathan
On 10/29/21, 12:47 PM, "Tom Lane" wrote: > While testing that, I noticed another bit of user-unfriendliness: > there's no obvious way to get out of it if you realize you are > setting the wrong user's password. simple_prompt() ignores > control-C, and when you give up and press return, you'll

Re: Add support for ALTER INDEX .. ALTER [COLUMN] col_num {SET, RESET}

2021-10-29 Thread Bossart, Nathan
On 10/29/21, 2:14 AM, "Michael Paquier" wrote: > By the way, while looking at this area of the code (particularly > tsvector and gist), I was under the impression that it is safe to > assume that we don't particularly need to rebuild the index when > changing its attoptions this way, but I have

Re: Skip vacuum log report code in lazy_scan_heap() if possible

2021-10-29 Thread Bossart, Nathan
On 10/29/21, 3:54 AM, "Greg Nancarrow" wrote: > When recently debugging a vacuum-related issue, I noticed that there > is some log-report processing/formatting code at the end of > lazy_scan_heap() that, unless the vacuum VERBOSE option is specified, > typically results in no log output (as the

Re: inefficient loop in StandbyReleaseLockList()

2021-10-29 Thread Bossart, Nathan
On 10/28/21, 11:53 PM, "Michael Paquier" wrote: > On Thu, Oct 28, 2021 at 04:52:48PM -0700, Andres Freund wrote: >> I suspect the reverse lock order release could be tad faster. But I probably >> wouldn't change it either - I was more thinking of some of the other cases >> that deleted the first

Re: inefficient loop in StandbyReleaseLockList()

2021-10-28 Thread Bossart, Nathan
On 10/28/21, 3:25 PM, "Tom Lane" wrote: > Andres Freund writes: >> Which leads to to wonder whether the better fix would be to switch to >> deleting >> the last element, but still use the while (!empty) style. That should convert >> the O(n^2) due to 1cff1b9 back to O(n). It might or might not

Re: inefficient loop in StandbyReleaseLockList()

2021-10-28 Thread Bossart, Nathan
On 10/28/21, 3:15 PM, "Andres Freund" wrote: > Which leads to to wonder whether the better fix would be to switch to deleting > the last element, but still use the while (!empty) style. That should convert > the O(n^2) due to 1cff1b9 back to O(n). It might or might not be faster/slower > than

Re: Correct error message for end-of-recovery record TLI

2021-10-28 Thread Bossart, Nathan
On 10/28/21, 12:52 PM, "Bossart, Nathan" wrote: > On 10/28/21, 12:23 PM, "Robert Haas" wrote: >> On Thu, Oct 28, 2021 at 8:59 AM Amul Sul wrote: >>> In xlog_redo, for end-of-recovery case error message describes the >>> record as a checkpoint recor

Re: Correct error message for end-of-recovery record TLI

2021-10-28 Thread Bossart, Nathan
On 10/28/21, 12:23 PM, "Robert Haas" wrote: > On Thu, Oct 28, 2021 at 8:59 AM Amul Sul wrote: >> In xlog_redo, for end-of-recovery case error message describes the >> record as a checkpoint record which seems to be incorrect; the >> attached patch corrects that. > > The analysis and patch appear

Re: inefficient loop in StandbyReleaseLockList()

2021-10-28 Thread Bossart, Nathan
On 10/28/21, 12:58 AM, "Michael Paquier" wrote: > On Thu, Oct 28, 2021 at 03:57:51PM +0900, Kyotaro Horiguchi wrote: >> However, I'm fine with fixing only StandbyRelaseLockList(), which >> actually suffers from list_delete_first(). > > I can also see a large gap between one technique and the

Re: [PATCH v2] use has_privs_for_role for predefined roles

2021-10-27 Thread Bossart, Nathan
On 10/27/21, 2:19 PM, "Robert Haas" wrote: > I am not sure I understand what the advantage of this is supposed to be. There are a couple of other related threads with some additional context:

Re: Isn't it better with "autovacuum worker...." instead of "worker took too long to start; canceled" specific to "auto

2021-10-27 Thread Bossart, Nathan
On 10/27/21, 9:29 AM, "Bharath Rupireddy" wrote: > Is there a specific reason that we have a generic WARNING "worker took > too long to start; canceled" for an autovacuum worker? Isn't it better > with "autovacuum worker took too long to start; canceled"? It is > confusing to see the generic

Re: [PATCH] Conflation of member/privs for predefined roles

2021-10-27 Thread Bossart, Nathan
On 10/27/21, 10:28 AM, "Joshua Brindle" wrote: > I'm new to contributing here but I've been told that the string > changes get taken care of later, is that not true? I will sometimes leave out tests and docs until I get buy-in on the approach. But for serious consideration, I think the patch

Re: [PATCH] remove is_member_of_role() from header, add can_set_role()

2021-10-27 Thread Bossart, Nathan
On 10/27/21, 10:22 AM, "Joshua Brindle" wrote: > On Wed, Oct 27, 2021 at 1:12 PM Mark Dilger > wrote: >> I don't understand the purpose of this. You are defining >> can_set_role(member,role) as a simple wrapper around >> is_member_of_role(member,role). Couldn't the comment: >> >> + * >> + *

Re: [PATCH] Conflation of member/privs for predefined roles

2021-10-27 Thread Bossart, Nathan
On 10/26/21, 3:50 PM, "Joshua Brindle" wrote: > Generally if a role is granted membership to another role with NOINHERIT > they must use SET ROLE to access the privileges of that role, however > with predefined roles the membership and privilege is conflated, as > demonstrated by: I think it

Re: Predefined role pg_maintenance for VACUUM, ANALYZE, CHECKPOINT.

2021-10-26 Thread Bossart, Nathan
On 10/26/21, 2:04 PM, "Jeff Davis" wrote: > Should we just add a builtin function pg_checkpoint(), and deprecate > the syntax? That seems reasonable to me. Nathan

Re: Predefined role pg_maintenance for VACUUM, ANALYZE, CHECKPOINT.

2021-10-26 Thread Bossart, Nathan
On 10/25/21, 6:48 PM, "Jeff Davis" wrote: > On Tue, 2021-10-26 at 00:07 +0000, Bossart, Nathan wrote: >> It feels a bit excessive to introduce a new predefined role just for >> this. Perhaps this could be accomplished with a new function that >> could b

Re: Predefined role pg_maintenance for VACUUM, ANALYZE, CHECKPOINT.

2021-10-25 Thread Bossart, Nathan
On 10/25/21, 4:40 PM, "Jeff Davis" wrote: > On Mon, 2021-10-25 at 17:55 -0300, Alvaro Herrera wrote: >> Maybe you just need pg_checkpointer. > > Fair enough. Attached simpler patch that only covers checkpoint, and > calls the role pg_checkpointer. It feels a bit excessive to introduce a new

Re: Allow pg_signal_backend members to use pg_log_backend_memory_stats().

2021-10-25 Thread Bossart, Nathan
On 10/25/21, 4:29 PM, "Jeff Davis" wrote: > On Mon, 2021-10-25 at 14:30 -0700, Andres Freund wrote: >> I don't get the reasoning behind the "except ..." logic. What does >> this >> actually protect against? A reasonable use case for this feature is >> is to >> monitor memory usage of all

Re: Allow pg_signal_backend members to use pg_log_backend_memory_stats().

2021-10-25 Thread Bossart, Nathan
On 10/25/21, 1:43 PM, "Jeff Davis" wrote: > On Mon, 2021-10-25 at 16:10 +0900, Michael Paquier wrote: >> Hmm. Why don't you split the patch into two parts that can be >> discussed separately then? There would be one to remove all the >> superuser() checks you can think of, and a potential

Re: parallelizing the archiver

2021-10-25 Thread Bossart, Nathan
On 10/25/21, 1:29 PM, "Robert Haas" wrote: > On Mon, Oct 25, 2021 at 3:45 PM Bossart, Nathan wrote: >> Alright, here is an attempt at that. With this revision, archive >> libraries are preloaded (and _PG_init() is called), and the archiver >> is responsible for

Re: [Patch] ALTER SYSTEM READ ONLY

2021-10-25 Thread Bossart, Nathan
On 10/25/21, 1:33 PM, "Robert Haas" wrote: > On Mon, Oct 25, 2021 at 3:14 PM Bossart, Nathan wrote: >> My compiler is complaining about oldXLogAllowed possibly being used >> uninitialized in CreateCheckPoint(). AFAICT it can just be initially >> set to zero

Re: [Patch] ALTER SYSTEM READ ONLY

2021-10-25 Thread Bossart, Nathan
On 10/25/21, 7:50 AM, "Robert Haas" wrote: > I've pushed 0001 and 0002 but I reversed the order of them and made a > few other edits. My compiler is complaining about oldXLogAllowed possibly being used uninitialized in CreateCheckPoint(). AFAICT it can just be initially set to zero to silence

Re: parallelizing the archiver

2021-10-25 Thread Bossart, Nathan
On 10/25/21, 10:18 AM, "Robert Haas" wrote: > On Mon, Oct 25, 2021 at 1:14 PM Bossart, Nathan wrote: >> IIUC this would mean that archive modules that need to define GUCs or >> register background workers would have to separately define a >> _PG_init() and be load

Re: parallelizing the archiver

2021-10-25 Thread Bossart, Nathan
On 10/25/21, 10:02 AM, "Robert Haas" wrote: > I don't see why this patch should need to make any changes to > internal_load_library(), PostmasterMain(), SubPostmasterMain(), or any > other central point of control, and I don't think it should. > pgarch_archiveXlog() can just load the library at

Re: Allow pg_signal_backend members to use pg_log_backend_memory_stats().

2021-10-25 Thread Bossart, Nathan
On 10/25/21, 2:21 AM, "Bharath Rupireddy" wrote: > On Mon, Oct 25, 2021 at 12:40 PM Michael Paquier wrote: >> Hmm. Why don't you split the patch into two parts that can be >> discussed separately then? There would be one to remove all the >> superuser() checks you can think of, and a

Re: Predefined role pg_maintenance for VACUUM, ANALYZE, CHECKPOINT.

2021-10-25 Thread Bossart, Nathan
On 10/24/21, 11:13 PM, "Jeff Davis" wrote: > On Sun, 2021-10-24 at 21:32 +0000, Bossart, Nathan wrote: >> My initial reaction was that members of pg_maintenance should be able >> to do all of these things (VACUUM, ANALYZE, CLUSTER, REINDEX, and >> CHECKPOINT). >

Re: Predefined role pg_maintenance for VACUUM, ANALYZE, CHECKPOINT.

2021-10-24 Thread Bossart, Nathan
On 10/24/21, 10:20 AM, "Jeff Davis" wrote: > On Sun, 2021-10-24 at 20:19 +0530, Bharath Rupireddy wrote: >> Are there any other database activities that fall under the >> "maintenance" category? How about CLUSTER, REINDEX? I didn't check >> the >> code for their permissions. > > I looked around

Re: Allow pg_signal_backend members to use pg_log_backend_memory_stats().

2021-10-24 Thread Bossart, Nathan
On 10/24/21, 9:51 AM, "Jeff Davis" wrote: > On Sat, 2021-10-23 at 20:42 +0000, Bossart, Nathan wrote: >> The predefined roles documentation notes >> that members of pg_signal_backend cannot signal superuser-owned >> backends, but AFAICT pg_log_backend_memory_contex

Re: Allow pg_signal_backend members to use pg_log_backend_memory_stats().

2021-10-23 Thread Bossart, Nathan
On 10/23/21, 12:57 PM, "Jeff Davis" wrote: > pg_signal_backend seems like the appropriate predefined role, because > pg_log_backend_memory_contexts() is implemented by a sending signal. This seems reasonable to me. The stated reason in the original commit message for keeping it restricted to

Re: pg_dump handling of ALTER DEFAULT PRIVILEGES IN SCHEMA

2021-10-22 Thread Bossart, Nathan
On 9/5/21, 11:33 PM, "Bossart, Nathan" wrote: > Attached is an attempt at cleaning the patch up and adding an > informative comment and a test case. For future reference, there was another thread for this bug [0], and a fix was committed [1]. Nathan [0] https://postgr

Re: Make mesage at end-of-recovery less scary.

2021-10-22 Thread Bossart, Nathan
On 3/4/21, 10:50 PM, "Kyotaro Horiguchi" wrote: > As the result, the following messages are emitted with the attached. I'd like to voice my support for this effort, and I intend to help review the patch. It looks like the latest patch no longer applies, so I've marked the commitfest entry [0]

Re: parallelizing the archiver

2021-10-22 Thread Bossart, Nathan
On 10/22/21, 7:43 AM, "Magnus Hagander" wrote: > I still like the idea of loading the library via a special > parameter, archive_library or such. My first attempt [0] added a GUC like this, so I can speak to some of the interesting design decisions that follow. The simplest thing we could do

Re: should we allow users with a predefined role to access pg_backend_memory_contexts view and pg_log_backend_memory_contexts function?

2021-10-22 Thread Bossart, Nathan
On 10/21/21, 6:51 PM, "Bharath Rupireddy" wrote: > Here's v3 for further review. I've marked this as ready-for-committer. The only other feedback I would offer is nitpicking at the test code to clean it up a little bit, but I don't think it is necessary to block on that. Nathan

Re: Experimenting with hash tables inside pg_dump

2021-10-21 Thread Bossart, Nathan
On 10/21/21, 4:14 PM, "Bossart, Nathan" wrote: > On 10/21/21, 3:29 PM, "Tom Lane" wrote: >> (b) I couldn't measure any change in performance at all. I tried >> it on the regression database and on a toy DB with 1 simple >> tables. Maybe on a rea

Re: Experimenting with hash tables inside pg_dump

2021-10-21 Thread Bossart, Nathan
On 10/21/21, 3:29 PM, "Tom Lane" wrote: > (b) I couldn't measure any change in performance at all. I tried > it on the regression database and on a toy DB with 1 simple > tables. Maybe on a really large DB you'd notice some difference, > but I'm not very optimistic now. I wonder how many

Re: CREATEROLE and role ownership hierarchies

2021-10-21 Thread Bossart, Nathan
On 10/20/21, 11:46 AM, "Mark Dilger" wrote: > The purpose of these patches is to fix the CREATEROLE escalation > attack vector misfeature. (Not everyone will see CREATEROLE that > way, but the perceived value of the patch set likely depends on how > much you see CREATEROLE in that light.)

Re: Fixing WAL instability in various TAP tests

2021-10-21 Thread Bossart, Nathan
On 9/28/21, 8:17 PM, "Michael Paquier" wrote: > On Tue, Sep 28, 2021 at 03:00:13PM -0400, Tom Lane wrote: >> Should we back-patch 0002? I'm inclined to think so. Should >> we then also back-patch enablement of the bloom test? Less >> sure about that, but I'd lean to doing so. A test that

Re: should we allow users with a predefined role to access pg_backend_memory_contexts view and pg_log_backend_memory_contexts function?

2021-10-21 Thread Bossart, Nathan
On 10/20/21, 11:44 PM, "Bharath Rupireddy" wrote: > I would like to confine this thread to allowing non-superusers with a > predefined role (earlier suggestion was to use pg_read_all_stats) to > access views pg_backend_memory_contexts and pg_shmem_allocations and > functions

Re: ALTER INDEX .. RENAME allows to rename tables/views as well

2021-10-19 Thread Bossart, Nathan
On 10/19/21, 3:13 PM, "Alvaro Herrera" wrote: > On 2021-Oct-19, Bossart, Nathan wrote: > >> I did consider this, but I figured it might be better to keep the lock >> level consistent for a given object type no matter what the statement >> type is. I don't have a

Re: ALTER INDEX .. RENAME allows to rename tables/views as well

2021-10-19 Thread Bossart, Nathan
On 10/19/21, 1:36 PM, "Alvaro Herrera" wrote: > I was about to push this when it occurred to me that it seems a bit > pointless to release AEL in order to retry with the lighter lock; once > we have AEL, let's just keep it and proceed. So how about the attached? I did consider this, but I

Re: pg_upgrade test chatter

2021-10-19 Thread Bossart, Nathan
On 10/19/21, 12:37 PM, "Tom Lane" wrote: > Actually ... why shouldn't we suppress that by running the command > with client_min_messages = warning? This would have to be a change > to pg_regress, but I'm having a hard time thinking of cases where > quieting that message would be a problem. I

Re: parallelizing the archiver

2021-10-19 Thread Bossart, Nathan
On 10/19/21, 6:39 AM, "David Steele" wrote: > On 10/19/21 8:50 AM, Robert Haas wrote: >> I am not quite sure why we wouldn't just compile the functions into >> the server. Functions pointers can point to core functions as surely >> as loadable modules. The present design isn't too congenial to

Re: .ready and .done files considered harmful

2021-10-19 Thread Bossart, Nathan
On 10/19/21, 5:59 AM, "Robert Haas" wrote: > Nathan, I just realized we never closed the loop on this. Do you have > any thoughts? IMO the patch is in decent shape. Happy to address any feedback you might have on the latest patch [0]. Nathan [0]

Re: BUG #17220: ALTER INDEX ALTER COLUMN SET (..) with an optionless opclass makes index and table unusable

2021-10-19 Thread Bossart, Nathan
On 10/18/21, 11:49 PM, "Matthijs van der Vleuten" wrote: > The test case doesn't seem entirely correct to me? The index being > dropped (btree_tall_tbl_idx2) doesn't exist. This was fixed before it was committed [0]. > Also, I don't believe this tests the case of dropping the index when > it

Re: BUG #17220: ALTER INDEX ALTER COLUMN SET (..) with an optionless opclass makes index and table unusable

2021-10-18 Thread Bossart, Nathan
On 10/18/21, 7:09 PM, "Michael Paquier" wrote: > Thanks for double-checking. Applied and back-patched, with a small > conflict regarding the error message in ~14. Thanks! Nathan

Re: ALTER INDEX .. RENAME allows to rename tables/views as well

2021-10-18 Thread Bossart, Nathan
On 10/18/21, 4:56 PM, "Alvaro Herrera" wrote: > Now, what is the worst that can happen if we rename a table under SUE > and somebody else is using the table concurrently? Is there any way to > cause a backend crash or something like that? As far as I can see, > because we grab a fresh catalog

Re: relation OID in ReorderBufferToastReplace error message

2021-10-18 Thread Bossart, Nathan
On 10/18/21, 3:31 AM, "Amit Kapila" wrote: > I can take care of backpatching this in the next few days unless there > is any objection. Thanks! Nathan

Re: BUG #17220: ALTER INDEX ALTER COLUMN SET (..) with an optionless opclass makes index and table unusable

2021-10-18 Thread Bossart, Nathan
On 10/18/21, 12:47 AM, "Michael Paquier" wrote: > I have reviewed the last patch posted upthread, and while testing > partitioned indexes I have noticed that we don't need to do a custom > check as part of ATExecSetOptions(), because we have already that in > ATSimplePermissions() with details on

Re: relation OID in ReorderBufferToastReplace error message

2021-10-14 Thread Bossart, Nathan
On 9/23/21, 11:26 AM, "Alvaro Herrera" wrote: > On 2021-Sep-23, Jeremy Schneider wrote: > >> On 9/22/21 20:11, Amit Kapila wrote: >> > >> > On Thu, Sep 23, 2021 at 3:06 AM Jeremy Schneider >> > wrote: >> >> >> >> Any chance of back-patching this? >> > >> > Normally, we don't back-patch code

Re: BUG #17220: ALTER INDEX ALTER COLUMN SET (..) with an optionless opclass makes index and table unusable

2021-10-12 Thread Bossart, Nathan
On 10/12/21, 5:31 PM, "Vik Fearing" wrote: > On 10/13/21 2:06 AM, Bossart, Nathan wrote: >> Moving to pgsql-hackers@. >> >> At first glance, it looks like ALTER INDEX .. ALTER COLUMN ... SET >> uses the wrong validation function. I've attached a

Re: should we allow users with a predefined role to access pg_backend_memory_contexts view and pg_log_backend_memory_contexts function?gr

2021-10-12 Thread Bossart, Nathan
On 10/12/21, 6:26 PM, "Michael Paquier" wrote: > On Tue, Oct 12, 2021 at 08:33:19PM -0400, Stephen Frost wrote: >> I would think we would do both…. That is- move to using GRANT/REVOKE, and >> then just include a GRANT to pg_read_all_stats. >> >> Or not. I can see the argument that, because it

Re: should we allow users with a predefined role to access pg_backend_memory_contexts view and pg_log_backend_memory_contexts function?

2021-10-12 Thread Bossart, Nathan
On 10/9/21, 2:12 AM, "Bharath Rupireddy" wrote: > Here's the v1, please review it further. Thanks for the patch. - /* Only allow superusers to log memory contexts. */ - if (!superuser()) + /* +* Only superusers or members of pg_read_all_stats can log memory contexts.

Re: should we allow users with a predefined role to access pg_backend_memory_contexts view and pg_log_backend_memory_contexts function?

2021-10-07 Thread Bossart, Nathan
On 10/7/21, 10:42 AM, "Bharath Rupireddy" wrote: > In a typical production environment, the user (not necessarily a > superuser) sometimes wants to analyze the memory usage via > pg_backend_memory_contexts view or pg_log_backend_memory_contexts > function which are accessible to only superusers.

Re: ALTER INDEX .. RENAME allows to rename tables/views as well

2021-10-06 Thread Bossart, Nathan
On 10/6/21, 3:44 PM, "Tom Lane" wrote: > "Bossart, Nathan" writes: >> Here's a patch that ERRORs if the object type and statement type do >> not match. Interestingly, some of the regression tests were relying >> on this behavior. > &

Re: Parallel vacuum workers prevent the oldest xmin from advancing

2021-10-06 Thread Bossart, Nathan
On 10/6/21, 12:13 AM, "Masahiko Sawada" wrote: > A customer reported that during parallel index vacuum, the oldest xmin > doesn't advance. Normally, the calculation of oldest xmin > (ComputeXidHorizons()) ignores xmin/xid of processes having > PROC_IN_VACUUM flag in MyProc->statusFlags. But since

Re: Pre-allocating WAL files

2021-10-06 Thread Bossart, Nathan
On 10/6/21, 5:20 AM, "Maxim Orlov" wrote: > We've looked through the code and everything looks good except few minor > things: > 1). Using dedicated bg worker seems not optimal, it introduces a lot of > redundant code for little single action. > We'd join initial proposal of Andres to

<    1   2   3   4   5   >