On Wed, Oct 13, 2021 at 12:54 PM Justin Pryzby wrote:
> It seems unfortunate if names from log messages qualified with datname were
> now
> rejected. Like this one:
>
> | automatic analyze of table "ts.child.cdrs_2021_10_12"...
That's a good argument, IM
ler is WAL replay, it will know where the aborted
Saying that "perhaps" a contrecord piece was lost seems to imply that
other explanations are possible as well, but I'm not sure what.
--
Robert Haas
EDB: http://www.enterprisedb.com
an.one.part would give an error
> about too many dotted parts rather than simply trying to exclude the last
> "part" and silently ignoring the prefix, which I think is what v13's
> pg_dumpall would do. --exclude-database=db?? would work to exclude four
> charact
only being applied for *tables* and not for
*functions*? I guess that could be true, but if so, it sure seems
inconsistent.
--
Robert Haas
EDB: http://www.enterprisedb.com
ogInsertAllowed() checks (noting that the return value is reversed).
If we did that, then I think RecoveryInProgress() could also NOT call
InitXLOGAccess(), and that could be done only by XLogInsertAllowed(),
which seems like it might be better. But I haven't really tried to
code all of this up, so I'm not really sure how it all works out.
--
Robert Haas
EDB: http://www.enterprisedb.com
e for basebackup_lz4.c to
use. It would be a lot better if we could instead get LZ4 to allocate
memory using palloc(), but a quick Google search suggests that you
can't accomplish that without recompiling liblz4, and that's not
workable since we don't want to require a liblz4 built specifically
for PostgreSQL. Do you see any other solution?
--
Robert Haas
EDB: http://www.enterprisedb.com
ly as if the user querying the view were running them, and
partly as if the user owning the view were one of them. It seems much
more logical for it to be one or the other.
--
Robert Haas
EDB: http://www.enterprisedb.com
ther.
I also think that, as a followup action item, we need to reassess
parallel_tuple_cost.
--
Robert Haas
EDB: http://www.enterprisedb.com
ion. Maybe that makes a
> good proposal for LZ4 library ;-).
> I cannot think of another solution to it right away.
OK. Will give it some thought.
--
Robert Haas
EDB: http://www.enterprisedb.com
ad a view if it will
> always fail due to lack of execute privilege?
An excellent question.
--
Robert Haas
EDB: http://www.enterprisedb.com
well, but that seems like a pretty good one.
I'm not very convinced that using the LSN for any of this is a good
idea. Something that changes most of the time but not all the time
seems more like it could hurt by masking fuzzy thinking more than it
helps anything.
--
Robert Haas
EDB: http://www.enterprisedb.com
On Thu, Sep 30, 2021 at 5:08 PM Robert Haas wrote:
> Any thoughts on the patch I attached?
Apparently not, but here's a v2 anyway. In this version I made
enable_timeout_every() a three-argument function, so that the caller
can specify both the first time at which the timeout routine s
de could seem
like a much less safe choice by the time we get a committed feature
here than it does today - even if somehow that were to happen for v15.
I expect there are people out there trying to break it even as I write
these words, and it seems likely that they will eventually succeed,
but as to when, who can say?
--
Robert Haas
EDB: http://www.enterprisedb.com
entirely
trivial. And this is just a quick test, and just for one of the three
things that get initialized here.
On the other hand, just moving it to XLogInsertAllowed() isn't
risk-free either and would likely require adjusting some of the same
places I found with this test. So I guess
d is a hateful
thing to many people here, but software has to serve the user base,
not just the developers. Lots of people use archive_command and rely
on it -- and are not interested in installing yet another piece of
out-of-core software to do what $OTHERDB has built in.
--
Robert Haas
EDB: http://www.enterprisedb.com
On Fri, Sep 24, 2021 at 12:28 PM Robert Haas wrote:
> On Thu, Sep 16, 2021 at 7:26 PM Bossart, Nathan wrote:
> > What do you think?
>
> I think this is committable. I also went back and looked at your
> previous proposal to do files in batches, and I think that's also
&g
s there can be weird cases if it gets loaded
into some backends but not others and things like that.
And here we seem to have an opportunity to improve the interface by
not depending on it.
--
Robert Haas
EDB: http://www.enterprisedb.com
On Tue, Oct 19, 2021 at 11:46 AM Sasasu wrote:
> As there are so many threat models, I propose to do the TDE feature by a
> set of hooks.
This is too invasive to do using hooks. We are inevitably going to
need to make significant changes in core.
--
Robert Haas
EDB: http://www.enterprisedb.com
entOpen */
sendTimeLineIsHistoric = false;
-sendTimeLine = ThisTimeLineID;
+sendTimeLine = rand() % 10;
if (cmd->kind == REPLICATION_KIND_PHYSICAL)
{
And in fact, that passes make check-world. :-(
--
Robert Haas
EDB: http://www.enterprisedb.com
t's not a bad idea; it'll help discover bogus code. Obviously, some
> additional tests wouldn't harm -- we have a lot more coverage now than
> in embarrasingly recent past, but it can still be improved.
+1.
--
Robert Haas
EDB: http://www.enterprisedb.com
"archive recovery complete" message is now logged after rather
than before writing and archiving a timeline history file. I think
that's likely an improvement.
That's all I have on 0001. Is this kind of review helpful?
Thanks,
--
Robert Haas
EDB: http://www.enterprisedb.com
this change *and also*
resolve the role self-administration problem, then it can also work in
cases where a more privileged user needs to enforce event trigger
firing against a less-privileged user.
--
Robert Haas
EDB: http://www.enterprisedb.com
[1] http://postgr.es/m/214052.1627331...@sss.pgh.pa.us
[2] http://postgr.es/m/216038.1627333...@sss.pgh.pa.us
local variable here is that that's what
RecoveryInProgress() updates. It's just two mutually-reinforcing bad
decisions.
--
Robert Haas
EDB: http://www.enterprisedb.com
0001-Remove-useless-code-from-CreateReplicationSlot.patch
Description: Binary data
> The stock answer for that has been to do
>
> select reltuples from pg_class
> where relname = 'foo';
>
> But that is unsatisfying because the problem is often with some
> benchmark or another that cannot be changed.
I would also expect it to almost al
what was being discussed here in regards to
archive_command.
Also, more to the point, when there's a need to break backward
compatibility in order to get some improvement, it's worth
considering, but here there just isn't.
--
Robert Haas
EDB: http://www.enterprisedb.com
an approximation it
may be good enough, but I don't think it will often be exactly correct
- probably only if the table is small and rarely modified.
--
Robert Haas
EDB: http://www.enterprisedb.com
nces you mention: query can't be changed, exact answer not
actually required, whole table being counted. But I am not here to
call you a liar either. If you run across users in that situation all
the time, then you do.
--
Robert Haas
EDB: http://www.enterprisedb.com
we
think of additional benefits that we cannot obtain without
incompatibilities, then we can consider that situation when it arises.
In the meantime, there's no need to go looking for reasons to break
stuff that works in existing releases.
--
Robert Haas
EDB: http://www.enterprisedb.com
t Just Worked. But I suspect that even if that did happen in the
lab, reality wouldn't often be so kind.
--
Robert Haas
EDB: http://www.enterprisedb.com
d_libraries.
I was imagining something like what logical decoding does. In that
case, you make a _PG_output_plugin_init function and it returns a
table of callbacks. Then the core code invokes those callbacks at the
appropriate times.
--
Robert Haas
EDB: http://www.enterprisedb.com
ld make 9.0 or 9.2 or
something compile and talk to it than if I had to use v15 and hope
that held together somehow. It doesn't really make sense to try to
keep compatibility of any sort with versions we can no longer test
against.
--
Robert Haas
EDB: http://www.enterprisedb.com
y fine) or enable
-O0 locally. I routinely do that when I hit problems on older
branches, and it helps a lot, but the way I see it, that's such an
easy change that there's little reason to make it in the source code.
What's a lot more annoying is if the compile fails altogether, or you
can't even get past the configure step.
--
Robert Haas
EDB: http://www.enterprisedb.com
small adjustments to allow us to build old
> versions on modern platforms, but if we do that then we should probably
> have some buildfarm support for it.
Yeah, I think having a small number of buildfarm animals testing very
old versions would be nice. Perhaps we can call th
chines with not so much
> disk. Do we really need to maintain a separate checkout for each
> branch? It seems like a fresh checkout from the repo would be
> little more expensive than the current copy-a-checkout process.)
I suppose it would be useful if we had the ability to do new runs only
when the source code has changed...
--
Robert Haas
EDB: http://www.enterprisedb.com
e's
only one place where we set LocalXLogInsertAllowed = 0, and I don't
know that we'll ever have another one.
--
Robert Haas
EDB: http://www.enterprisedb.com
On Thu, Oct 21, 2021 at 3:29 PM Robert Haas wrote:
> I think the correct fix for this particular problem is just to delete
> the initialization, as in the attached patch. I spent a long time
> studying this today and eventually convinced myself that there's just
> no way these in
I don't think it's even the default.
Oh, OK. I wonder how that plays with the buildfarm status page's
desire to drop old results that are more than 30 days old. I guess
you'd just need to force a run at least every 28 days or something.
--
Robert Haas
EDB: http://www.enterprisedb.com
d-enable_timeout_every-to-fire-the-same-timeout.patch' is
> the same as Robert's v2 patch. I have rebased my patch on top of this
> and it is 'v19-0002-startup-progress.patch'.
This version looks fine, so I have committed it (and my
enable_timeout_every patch also,
teger values assigned to the enums in WaitEventIO. I know of 2
extensions that are affected by this. I think that it should be
reverted in v14 and kept only in master.
--
Robert Haas
EDB: http://www.enterprisedb.com
eral, GUCs whose legal values depend on
the values of other GUCs don't end up working out well. I think what
should happen instead is that if archive_library=shell then
archive_command does whatever it does; otherwise archive_command is
without effect.
--
Robert Haas
EDB: http://www.enterprisedb.com
ble whether or not anything that some
extension is doing ought to be deemed an acceptable use of the
facilities provided by core, and how much stability ought to be
guaranteed. But while I agree it's good to remove unused stuff in the
master, it doesn't seem like we really need to back
That doesn't seem too terrible to me, but it was
> something I was trying to avoid.
Hmm. That doesn't seem like a terrible goal, but I think we should try
to find some way of achieving it that looks tidier than this does.
--
Robert Haas
EDB: http://www.enterprisedb.com
n't be the case that user X can cause event
trigger E to run as user Y unless X can become Y, because to do so
would allow X to usurp Y's privileges, except in the corner case where
Y never does anything that can trigger an event trigger. But if X has
to be able to become Y in order to f
anyway without having to do any special steps.
But also, Noah writes: "Also, it's currently a best practice for only
non-LOGIN roles to have members. The proposed approach invites folks
to abandon that best practice."
The kind of mechanism you're proposing here doesn't seem to make that
any less likely.
--
Robert Haas
EDB: http://www.enterprisedb.com
also removed the GUC check hooks as previously
> discussed.
I would need to spend more time on this to have a detailed opinion on
all of it, but I agree that part looks better this way.
--
Robert Haas
EDB: http://www.enterprisedb.com
y when it is used.
Hmm, I guess I could have foreseen that, had I been a little bit
smarter than I am. I have committed a change to initialize it to 0 as
you propose.
--
Robert Haas
EDB: http://www.enterprisedb.com
thing else.
#1 and #2 could certainly be done, but I'm not sure what the use case
is, and I think it's a separate proposal from what we did here. So I
think a new thread would be appropriate if you want to make a new
proposal.
--
Robert Haas
EDB: http://www.enterprisedb.com
seems like it
might be a bit confusing. Then "single" would end up being an alias
for "virtual" which I don't suppose is what anyone is expecting.
--
Robert Haas
EDB: http://www.enterprisedb.com
t, we're going
to go into an infinite loop scanning that bucket, but if we're holding
a buffer lock while we do it, there's no way to escape short of
bouncing the entire server. That's pretty bad.
Undetected deadlock is something to think about, too.
--
Robert Haas
EDB: http://www.enterprisedb.com
On Mon, Oct 25, 2021 at 3:25 PM Robert Haas wrote:
> But also, Noah writes: "Also, it's currently a best practice for only
> non-LOGIN roles to have members. The proposed approach invites folks
> to abandon that best practice."
>
> The kind of mechanism you're
or privilege checking.
>
> Please consider this for the upcoming commitfest.
I am not sure I understand what the advantage of this is supposed to be.
--
Robert Haas
EDB: http://www.enterprisedb.com
e fully
timeline-aware, but that's a bit of a project and this isn't really
broken, so what I'd like to do for right now is change it to use
non-bogus TLIs that we have available in local variables rather than a
possibly-nonsensical value from a global variable. Patch for that
at
On Mon, Oct 25, 2021 at 11:56 AM Robert Haas wrote:
> This version looks fine, so I have committed it (and my
> enable_timeout_every patch also, as a necessary prerequisite).
I was fooling around with a test setup today, working on an unrelated
problem, and this happened:
2021-10-28
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 correct to me.
--
Robert Haas
from
the right timeline, but there's nothing similar here. I think that
would likely have to be fixed in order for decoding to work on
standbys, but maybe I'm missing something.
--
Robert Haas
EDB: http://www.enterprisedb.com
On Thu, Oct 28, 2021 at 3:52 PM Bossart, Nathan wrote:
> When I apply the patch, it changes the PANIC message in the
> XLOG_CHECKPOINT_ONLINE section, not the XLOG_END_OF_RECOVERY one.
Well that's a good point. *facepalm*
--
Robert Haas
EDB: http://www.enterprisedb.com
Hmm, OK, perhaps I mis-spoke, but I think we're talking about the same
thing. read_local_xlog_page() has this:
* RecoveryInProgress() will update ThisTimeLineID when it first
* notices recovery finishes, so we only have to maintain it for the
* local process until recovery ends.
*/
if (!RecoveryInProgress())
read_upto = GetFlushRecPtr();
else
read_upto = GetXLogReplayRecPtr(&ThisTimeLineID);
tli = ThisTimeLineID;
That's a bulletproof guarantee that "tli" and "ThisTimeLineID" are up
to date. The other function has nothing similar.
--
Robert Haas
EDB: http://www.enterprisedb.com
wrong. If we did that, the previous timer could fire
right after we set startup_progress_timer_expired = false, and before
we reschedule the timeout. It seems annoying to have to disable the
timeout and immediately turn around and re-enable it, but I don't see
how to avoid the race condition other
e rather dubious assumption
that timelines somehow don't matter, I think changing the error
message would make things more confusing rather than less.
> As a whole, it looks fine to me.
Great. Thank you.
--
Robert Haas
EDB: http://www.enterprisedb.com
s any
> error before the backup could finish up normally.
>
> I have attached a patch to fix this.
Yes, this is the right fix. Apologies for the oversight.
--
Robert Haas
EDB: http://www.enterprisedb.com
nor) mistake.
> Your patch seems correct to me.
Thanks, committed.
--
Robert Haas
EDB: http://www.enterprisedb.com
> timeout and immediately turn around and re-enable it, but I don't see
> > how to avoid the race condition otherwise.
>
> Right. There is a possibility of race conditions. In that case the
> above changes look good to me.
Committed.
--
Robert Haas
EDB: http://www.enterprisedb.com
On Fri, Oct 29, 2021 at 6:56 PM Jeff Davis wrote:
> On Wed, 2021-10-27 at 16:10 -0400, Robert Haas wrote:
> > But the thing that defines a tenant need not be a role. It can be
> > some
> > other kind of object. Suppose we invent a CREATE TENANT command.
>
> Would it be
could be done here, but I think this is a
fairly respectable start.
Opinions welcome.
Thanks,
--
Robert Haas
EDB: http://www.enterprisedb.com
0003-walsender.c-Don-t-rely-on-the-global-variable-ThisTi.patch
Description: Binary data
0004-walreceiver.c-Don-t-rely-on-the-global-variable-This.patch
t and they probably wouldn't be doing that unless they'd
found some case where it's a big win. But this example seems extremely
unconvincing.
--
Robert Haas
EDB: http://www.enterprisedb.com
ment statement for that variable and it's here:
/* Save the selected TimeLineID in shared memory, too */
XLogCtl->ThisTimeLineID = ThisTimeLineID;
XLogCtl->PrevTimeLineID = PrevTimeLineID;
That's after the main redo loop completes.
--
Robert Haas
EDB: http://www.enterprisedb.com
u probably need to investigate is also supporting client-side
LZ4 compression. I think that is probably a really desirable addition
to your patch set, since people might find it odd if that were
exclusively a server-side option. Hopefully it's not that much work.
One minor nitpick in ter
more or less given up hope on getting away with
anything else.
--
Robert Haas
EDB: http://www.enterprisedb.com
anything. If
you want to change to a different module, you probably have to bounce
the whole server instead of just changing the GUC and letting it load
the new module when you run 'pg_ctl reload'.
Blech! :-)
--
Robert Haas
EDB: http://www.enterprisedb.com
hink that's probably not
good. If there's reason to think that, barring unusual circumstances,
changes will be noticed within a few minutes, I think that's fine.
--
Robert Haas
EDB: http://www.enterprisedb.com
u
wouldn't need to incur the expense of having the system enforce them.
The latter is unexpected and basically undiscoverable with default
settings.
--
Robert Haas
EDB: http://www.enterprisedb.com
ad the new one, and call any
startup hook for that one.
That wouldn't work with a hook-based approach.
--
Robert Haas
EDB: http://www.enterprisedb.com
that switching archive modules without restarting is more
> important, I believe we will need to take on a few restrictions.
I guess I'm failing to understand what the problem is. You can set
GUCs of the form foo.bar in postgresql.conf anyway, right?
--
Robert Haas
EDB: http://www.enterprisedb.com
I agree. Also, I think there's actually a file format called "zlib"
which is slightly different from the "gzip" format, and you have to be
careful not to generate the wrong one.
--
Robert Haas
EDB: http://www.enterprisedb.com
On Tue, Nov 2, 2021 at 10:32 AM Robert Haas wrote:
> Looks pretty good. I think you should work on stuff like documentation
> and tests, and I need to do some work on that stuff, too. Also, I
> think you should try to figure out how to support different
> compression levels.
On se
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.
It seems like PGC_POSTMASTER isn't very desirable anyway. Woul
would benefit from a lower setting
than 10 minutes, just to get more information about what's happening.
But IMHO, making the default as low as 1 minute is a bit much.
--
Robert Haas
EDB: http://www.enterprisedb.com
ng time. In those kinds of cases what
you really need to know is that there was a vacuum on a certain table,
and how long it took, and when that happened.
--
Robert Haas
EDB: http://www.enterprisedb.com
the fsyncs, I don't much care whether
it shows up in the log or not. If it wrote 4GB of data, or if it took
15 seconds to complete the fsyncs, I care. That's easily enough to
account for some problem that somebody had.
I'm not sure whether there are any other interesting criteria.
--
Robert Haas
EDB: http://www.enterprisedb.com
air to say that for the vast
majority of users, this isn't a real problem. And for those few for
whom it *is* a real problem, they can still shut off log_checkpoints.
It's not like anybody is proposing to remove the option.
--
Robert Haas
EDB: http://www.enterprisedb.com
On Tue, Nov 2, 2021 at 7:46 PM Bruce Momjian wrote:
> I realize in this thread that Robert Haas mentioned foreign key
> violations that would like also appear in logs, but those are
> ERROR/WARNING etc. messages that can be filtered out with
> log_min_message.
I think that's p
ave so little practical impact - especially considering that at least
some packagers are already putting log rotation in place
automatically, in a way consistent with distro policies.
--
Robert Haas
EDB: http://www.enterprisedb.com
nt file" - just a "log file".
> Can't GetWALInsertionTimeLine() called instead? I guess the reason is
> to avoid the unnecessary overhead involved in the function call. (Same
> at a few other places.)
That, plus to avoid failing the assertion in that function. As w
uld be the thing that would answer the question: are there any
parameters used under this node that are not also set under this node?
But I seem to recall that neither seemed to be answering precisely
that question, and the lousy naming of those fields and limited
documentation of their intended purpose did not help.
--
Robert Haas
EDB: http://www.enterprisedb.com
and the rest are parallel-safe. The best plan
might be to do all the other joins under a Gather and then perform the
parallel-restricted join above it.
But I found it very hard to figure out how to rejigger the logic that
places InitPlans to be more intelligent, and eventually gave up.
--
Robert Haas
EDB: http://www.enterprisedb.com
On Wed, Nov 3, 2021 at 6:56 PM Mark Dilger wrote:
> Done that way.
I agree with what others have said: this looks fine.
But, is it plausible to add test coverage for the new checks, or is
that going to be too much of a pain?
--
Robert Haas
EDB: http://www.enterprisedb.com
ine documentation changes contains two spelling
mistakes, and you've used // comments which are not project style in
two places. It's a good idea to check over your patches for such
simple mistakes before submitting them.
--
Robert Haas
EDB: http://www.enterprisedb.com
utput. There's no reason
why initdb can't pass -c log_checkpoints=off. See backend_options in
initdb.c.
I didn't spot any other problems on a quick read-through.
--
Robert Haas
EDB: http://www.enterprisedb.com
it that this is OK from a performance perspective. The pointers can
change from execution to execution, but not within an individual
execution, or so I think. So it doesn't need to be resolved every
time, if somehow that can be avoided. But maybe CPUs are sufficiently
well-optimized for computi
fic to propose, which I realize is kind of
unhelpful ... but I don't like this, either.
--
Robert Haas
EDB: http://www.enterprisedb.com
s it goes, but maybe there's a
way to be a little less ambitious and still get most of the benefit.
--
Robert Haas
EDB: http://www.enterprisedb.com
On Thu, Nov 4, 2021 at 5:25 PM Jeff Davis wrote:
> On Thu, 2021-11-04 at 12:37 -0400, Robert Haas wrote:
> > I don't have anything specific to propose, which I realize is kind of
> > unhelpful ... but I don't like this, either.
>
> We can go back to having a pg_che
;t
prevent you from successfully executing a checkpoint. With this
approach, it might.
--
Robert Haas
EDB: http://www.enterprisedb.com
o on. CHECKPOINT is
one of the few commands that has no target.
--
Robert Haas
EDB: http://www.enterprisedb.com
stic,
> cross platform manner with a compact, easy to maintain regression test has
> eluded me and is not included here.
OK, I've committed this version.
--
Robert Haas
EDB: http://www.enterprisedb.com
rbage is the right solution.
--
Robert Haas
EDB: http://www.enterprisedb.com
ent policy of generating a
paper trail long enough to reach Alpha Centauri, so maybe we ought to
try harder to reach some sort of consensus.
--
Robert Haas
EDB: http://www.enterprisedb.com
issingOK or
noError flag, for example. Maybe the fact that I don't quite see how
to use this effectively is just lack of imagination on my part
--
Robert Haas
EDB: http://www.enterprisedb.com
On Tue, Nov 2, 2021 at 10:32 AM Robert Haas wrote:
> Meanwhile, I think it's probably OK for me to go ahead and commit
> 0001-0003 from my patches at this point, since it seems we have pretty
> good evidence that the abstraction basically works, and there doesn't
> seem to b
y machine will work everywhere, unless someone tests it.
--
Robert Haas
EDB: http://www.enterprisedb.com
601 - 700 of 8046 matches
Mail list logo
