On Wed, Feb 8, 2023 at 11:13 PM Tom Lane wrote:
>
> Bharath Rupireddy writes:
> > Thanks a lot Robert for taking care of this. The patch is committed on
> > HEAD and reverted on v15. Now that the minor version branches are
> > stamped, is it time for us to get this to v15? I can then close the CF
Bharath Rupireddy writes:
> Thanks a lot Robert for taking care of this. The patch is committed on
> HEAD and reverted on v15. Now that the minor version branches are
> stamped, is it time for us to get this to v15? I can then close the CF
> entry - https://commitfest.postgresql.org/42/4012/.
No
On Mon, Feb 6, 2023 at 9:39 PM Robert Haas wrote:
>
> On Mon, Feb 6, 2023 at 11:07 AM Tom Lane wrote:
> > Umm ... is this really the sort of patch to be committing on a
> > release wrap day?
>
> Oh, shoot, I wasn't thinking about that. Would you like me to revert
> it in v15 for now?
Thanks a lo
On Mon, Feb 6, 2023 at 11:15 AM Tom Lane wrote:
> Robert Haas writes:
> > On Mon, Feb 6, 2023 at 11:07 AM Tom Lane wrote:
> >> Umm ... is this really the sort of patch to be committing on a
> >> release wrap day?
>
> > Oh, shoot, I wasn't thinking about that. Would you like me to revert
> > it i
Robert Haas writes:
> On Mon, Feb 6, 2023 at 11:07 AM Tom Lane wrote:
>> Umm ... is this really the sort of patch to be committing on a
>> release wrap day?
> Oh, shoot, I wasn't thinking about that. Would you like me to revert
> it in v15 for now?
Yeah, seems like the safest course. I wouldn'
On Mon, Feb 6, 2023 at 11:07 AM Tom Lane wrote:
> Umm ... is this really the sort of patch to be committing on a
> release wrap day?
Oh, shoot, I wasn't thinking about that. Would you like me to revert
it in v15 for now?
--
Robert Haas
EDB: http://www.enterprisedb.com
Robert Haas writes:
> On Thu, Feb 2, 2023 at 11:22 PM Bharath Rupireddy
> wrote:
>> I took the v4 patch, added some comments and attached it as the v7
>> patch here. Please find it.
> Committed and back-patched to v15.
Umm ... is this really the sort of patch to be committing on a
release wrap
On Thu, Feb 2, 2023 at 11:22 PM Bharath Rupireddy
wrote:
> I took the v4 patch, added some comments and attached it as the v7
> patch here. Please find it.
Committed and back-patched to v15.
--
Robert Haas
EDB: http://www.enterprisedb.com
On Fri, Feb 3, 2023 at 2:29 AM Robert Haas wrote:
>
Thanks for looking at this.
> On Tue, Nov 22, 2022 at 6:05 AM Bharath Rupireddy
> wrote:
> > If we place just the Assert(!StandbyMode); in
> > enable_startup_progress_timeout(), it fails for
> > begin_startup_progress_phase() in ResetUnloggedR
On Tue, Nov 22, 2022 at 6:05 AM Bharath Rupireddy
wrote:
> If we place just the Assert(!StandbyMode); in
> enable_startup_progress_timeout(), it fails for
> begin_startup_progress_phase() in ResetUnloggedRelations() because the
> InitWalRecovery(), that sets StandbyMode to true, is called before
>
On Mon, Nov 21, 2022 at 10:37 PM Robert Haas wrote:
>
> On Sun, Nov 20, 2022 at 9:20 PM Kyotaro Horiguchi
> wrote:
> > I prefer Robert's approach as it is more robust for future changes and
> > simple. I prefer to avoid this kind of piggy-backing and it doesn't
> > seem to be needed in this case.
On Sun, Nov 20, 2022 at 9:20 PM Kyotaro Horiguchi
wrote:
> I prefer Robert's approach as it is more robust for future changes and
> simple. I prefer to avoid this kind of piggy-backing and it doesn't
> seem to be needed in this case. XLogShutdownWalRcv() looks like a
> similar case to me and hones
At Fri, 18 Nov 2022 15:55:00 +0530, Bharath Rupireddy
wrote in
> On Fri, Nov 18, 2022 at 12:42 AM Robert Haas wrote:
> >
> > On Thu, Nov 17, 2022 at 2:22 AM Bharath Rupireddy
> > wrote:
> > > Duplication is a problem that I agree with and I have an idea here -
> > > how about introducing a new
On Fri, Nov 18, 2022 at 12:42 AM Robert Haas wrote:
>
> On Thu, Nov 17, 2022 at 2:22 AM Bharath Rupireddy
> wrote:
> > Duplication is a problem that I agree with and I have an idea here -
> > how about introducing a new function, say EnableStandbyMode() that
> > sets StandbyMode to true and disab
On Thu, Nov 17, 2022 at 2:22 AM Bharath Rupireddy
wrote:
> Duplication is a problem that I agree with and I have an idea here -
> how about introducing a new function, say EnableStandbyMode() that
> sets StandbyMode to true and disables the startup progress timeout,
> something like the attached?
On Thu, Nov 17, 2022 at 12:21 AM Robert Haas wrote:
>
> On Wed, Nov 16, 2022 at 1:47 AM Bharath Rupireddy
> wrote:
> > That can be done, only if we can disable the timeout in another place
> > when the StandbyMode is set to true in ReadRecord(), that is, after
> > the standby server finishes cras
On Thu, Nov 17, 2022 at 7:51 AM Robert Haas wrote:
+ * up, since standby mode is a state that is intendeded to persist
typo
Otherwise LGTM.
On Wed, Nov 16, 2022 at 1:47 AM Bharath Rupireddy
wrote:
> That can be done, only if we can disable the timeout in another place
> when the StandbyMode is set to true in ReadRecord(), that is, after
> the standby server finishes crash recovery and enters standby mode.
Oh, interesting. I didn't re
On Wed, Nov 16, 2022 at 2:28 PM Simon Riggs
wrote:
>
> On Wed, 16 Nov 2022 at 06:47, Bharath Rupireddy
> wrote:
> >
> > On Tue, Nov 15, 2022 at 10:55 PM Robert Haas wrote:
> > >
> > > On Tue, Nov 15, 2022 at 8:33 AM Bharath Rupireddy
> > > wrote:
> > > > Please review the v2 patch.
> > >
> > >
On Wed, 16 Nov 2022 at 06:47, Bharath Rupireddy
wrote:
>
> On Tue, Nov 15, 2022 at 10:55 PM Robert Haas wrote:
> >
> > On Tue, Nov 15, 2022 at 8:33 AM Bharath Rupireddy
> > wrote:
> > > Please review the v2 patch.
> >
> > It seems to me that this will call disable_startup_progress_timeout
> > on
On Tue, Nov 15, 2022 at 10:55 PM Robert Haas wrote:
>
> On Tue, Nov 15, 2022 at 8:33 AM Bharath Rupireddy
> wrote:
> > Please review the v2 patch.
>
> It seems to me that this will call disable_startup_progress_timeout
> once per WAL record, which seems like an unnecessary expense. How
> about le
On Tue, Nov 15, 2022 at 8:33 AM Bharath Rupireddy
wrote:
> Please review the v2 patch.
It seems to me that this will call disable_startup_progress_timeout
once per WAL record, which seems like an unnecessary expense. How
about leaving the code inside the loop just as we have it, and putting
if (S
On Tue, 15 Nov 2022 at 13:33, Bharath Rupireddy
wrote:
>
> On Mon, Nov 14, 2022 at 9:31 PM Robert Haas wrote:
> >
> > On Mon, Nov 14, 2022 at 7:37 AM Simon Riggs
> > wrote:
> > > > Whilte at it, I noticed that we report redo progress for PITR, but we
> > > > don't report when standby enters arch
On Mon, Nov 14, 2022 at 9:31 PM Robert Haas wrote:
>
> On Mon, Nov 14, 2022 at 7:37 AM Simon Riggs
> wrote:
> > > Whilte at it, I noticed that we report redo progress for PITR, but we
> > > don't report when standby enters archive recovery mode, say due to a
> > > failure in the connection to pri
On Mon, Nov 14, 2022 at 7:37 AM Simon Riggs
wrote:
> > Whilte at it, I noticed that we report redo progress for PITR, but we
> > don't report when standby enters archive recovery mode, say due to a
> > failure in the connection to primary or after the promote signal is
> > found. Isn't it useful t
On Tue, 8 Nov 2022 at 12:33, Bharath Rupireddy
wrote:
>
> On Tue, Nov 8, 2022 at 4:35 PM Thomas Munro wrote:
> >
> > On Sat, Oct 30, 2021 at 7:44 AM Robert Haas wrote:
> > > Committed.
> >
> > Is it expected that an otherwise idle standby's recovery process
> > receives SIGALRM every N seconds,
On Tue, Nov 8, 2022 at 4:35 PM Thomas Munro wrote:
>
> On Sat, Oct 30, 2021 at 7:44 AM Robert Haas wrote:
> > Committed.
>
> Is it expected that an otherwise idle standby's recovery process
> receives SIGALRM every N seconds, or should the timer be canceled at
> that point, as there is no further
On Sat, Oct 30, 2021 at 7:44 AM Robert Haas wrote:
> Committed.
Is it expected that an otherwise idle standby's recovery process
receives SIGALRM every N seconds, or should the timer be canceled at
that point, as there is no further progress to report?
On Fri, Oct 29, 2021 at 9:10 AM Nitin Jadhav
wrote:
> > I think you're 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
> I think you're 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
On Fri, Oct 29, 2021 at 7:37 AM Nitin Jadhav
wrote:
> ereport_startup_progress() logs a message. So I feel just setting
> 'startup_progress_timer_expired' to false in
> begin_startup_progress_phase() would work. Please correct me if I am
> wrong.
I think you're wrong. If we did that, the previous
> I was fooling around with a test setup today, working on an unrelated
> problem, and this happened:
>
> 2021-10-28 14:21:23.145 EDT [92010] LOG: resetting unlogged relations
> (init), elapsed time: 0.00 s, current path: base/13020
Nice catch and interesting case.
> That's not supposed to happe
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 14:21:23.145
On Tue, Oct 26, 2021 at 4:19 AM Bharath Rupireddy
wrote:
> Can we also log the total time the startup process took to recover,
> and also the total time each stage of the recovery/redo processing
> took: 1) into a file or 2) emitting that info via a new hook 3) into a
> system catalog table (assum
On Mon, Oct 25, 2021 at 9:26 PM Robert Haas wrote:
>
> On Tue, Oct 19, 2021 at 9:06 AM Nitin Jadhav
> wrote:
> > Thanks for sharing the patch. Overall approach looks good to me. But
> > just one concern about using enable_timeout_every() functionality. As
> > per my understanding the caller shoul
On Tue, Oct 19, 2021 at 9:06 AM Nitin Jadhav
wrote:
> Thanks for sharing the patch. Overall approach looks good to me. But
> just one concern about using enable_timeout_every() functionality. As
> per my understanding the caller should calculate the first scheduled
> timeout (now + interval) and p
> 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 should be
> called and the interval between them, instead of only the latter. That
> seems to be mor
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 should be
cal
On Wed, Oct 13, 2021 at 9:06 AM Amul Sul wrote:
> I think the "elapsed time" part can be implicitly added to the error
> message inside ereport_startup_progress() which is common to all
> calls.
Not if it means having to call psprintf there!
If there's some way we could do it with macro tricks,
On Wed, Sep 29, 2021 at 11:10 PM Robert Haas wrote:
>
> On Wed, Sep 29, 2021 at 10:08 AM Nitin Jadhav
> wrote:
> > Sorry. There was a misunderstanding about this and for the patch
> > shared on September 27th, I had tested for the value '0' and observed
> > that no progress messages were getting
On Thu, Sep 30, 2021 at 05:08:17PM -0400, Robert Haas wrote:
> It's certainly less of an issue than it used to be back in my day.
>
> Any thoughts on the patch I attached?
I don't know. Anyway, this is actively worked on, so I have taken the
liberty to move that to the next CF.
--
Michael
sign
On Thu, Sep 30, 2021 at 3:10 PM Tom Lane wrote:
> That would be lovely, certainly. But aren't you moving the goalposts
> rather far? I don't think we make any promises about such things
> today, so why has the issue suddenly gotten more pressing?
Yeah, perhaps it's best to not to worry about it
Robert Haas writes:
> ... When I say I want my handler to be
> fired in 3 s, I don't mean that I want it to be fired when the system
> time is 3 seconds greater than it is right now. I mean I want it to be
> fired in 3 actual seconds, regardless of what dumb thing the system
> clock may choose to
On Wed, Sep 29, 2021 at 5:12 PM Tom Lane wrote:
> I didn't claim there are any other places that could use the feature
> *today*. But once we've got one, it seems like there could be more
> tomorrow. In any case, I dislike keeping timeout state data outside
> timeout.c, because it's so likely to
Robert Haas writes:
> On Wed, Sep 29, 2021 at 2:06 PM Tom Lane wrote:
>> The real comment I'd have here, though, is that writing one-off
>> code for this purpose is bad. If we have a need for a repetitive
>> timeout, it'd be better to add the feature to timeout.c explicitly.
>> That would probab
On Wed, Sep 29, 2021 at 2:06 PM Tom Lane wrote:
> The real comment I'd have here, though, is that writing one-off
> code for this purpose is bad. If we have a need for a repetitive
> timeout, it'd be better to add the feature to timeout.c explicitly.
> That would probably also remove the need for
On Wed, Sep 29, 2021 at 1:52 PM Alvaro Herrera wrote:
> I think one person casting an opinion on one aspect does not set that
> aspect in stone.
Of course not. I was just explaining that how the patch ended up like it did.
--
Robert Haas
EDB: http://www.enterprisedb.com
Alvaro Herrera writes:
> On 2021-Sep-29, Robert Haas wrote:
>> Well, this was my suggestion, because if you don't do this, you get
>> drift, which I think looks weird. Like the timestamps will be:
>>
>> 13:41:05.012456
>> 13:41:15.072484
>> 13:41:25.149632
>>
>> ...and it gets further and furthe
On 2021-Sep-29, Justin Pryzby wrote:
> Robert requested the current behavior here.
> https://www.postgresql.org/message-id/CA%2BTgmoYkS1ZeWdSMFMBecMNxWonHk6J5eoX4FEQrpKtvEbXqGQ%40mail.gmail.com
>
> It's confusing (at least) to get these kind of requests to change the behavior
> back and forth.
W
On 2021-Sep-29, Robert Haas wrote:
> Well, this was my suggestion, because if you don't do this, you get
> drift, which I think looks weird. Like the timestamps will be:
>
> 13:41:05.012456
> 13:41:15.072484
> 13:41:25.149632
>
> ...and it gets further and further off as it goes on.'
Right ...
On Wed, Sep 29, 2021 at 02:36:14PM -0300, Alvaro Herrera wrote:
> Why is it that we set the next timeout to fire not at "now + interval"
> but at "when-it-should-have-fired-but-didn't + interval"? As a user, if
> I request a message to be logged every N milliseconds, and one
> of them is a little
On Wed, Sep 29, 2021 at 1:36 PM Alvaro Herrera wrote:
> Why is it that we set the next timeout to fire not at "now + interval"
> but at "when-it-should-have-fired-but-didn't + interval"? As a user, if
> I request a message to be logged every N milliseconds, and one
> of them is a little bit delay
On Wed, Sep 29, 2021 at 10:08 AM Nitin Jadhav
wrote:
> Sorry. There was a misunderstanding about this and for the patch
> shared on September 27th, I had tested for the value '0' and observed
> that no progress messages were getting logged, probably the time at
> which 'enable_timeout_at' is getti
So, I've wondered about this part all along:
> +/*
> + * Calculates the timestamp at which the next timer should expire and enables
> + * the timer accordingly.
> + */
> +static void
> +reset_startup_progress_timeout(TimestampTz now)
> +{
> + TimestampTz next_timeout;
> +
> + next_timeout
> It is common to mention what the default is as part of the
> documentation of a GUC. I don't see why this one should be an
> exception, especially since not mentioning it reduces the length of
> the documentation by exactly one word.
>
> I don't mind the sentence you added at the end to clarify t
On Tue, Sep 28, 2021 at 8:06 AM Nitin Jadhav
wrote:
> I thought mentioning the unit in milliseconds and the example in
> seconds would confuse the user, so I changed the example to
> milliseconds.The message behind the above description looks good to me
> however I feel some sentences can be expla
> That's really not what I'm complaining about. I think if we're going
> to give an example at all, 10ms is a better example than 100ms,
> because 10s is a value that people are more likely to find useful. But
> I'm not sure that it's necessary to mention a specific value, and if
> it is, I think i
> +/*
> + * Decides whether to log the startup progress or not based on whether the
> + * timer is expired or not. Returns FALSE if the timer is not expired,
> otherwise
> + * calculates the elapsed time and sets the respective out parameters secs
> and
> + * usecs. Enables the timer for the next
On Mon, Sep 27, 2021 at 9:32 AM Justin Pryzby wrote:
> >>It also looks pretty silly to say that if you set the value to 10s,
> >>something
> >>will happen every 10s. What else would anyone expect to happen?
>
> @Robert: that's consistent with existing documentation, even though it might
> seem ob
On Mon, Sep 27, 2021 at 7:26 AM Nitin Jadhav
wrote:
> > I really don't know what to say about this. You say that the time is
> > measured in milliseconds, and then immediately turn around and say
> > "For example, if you set it to 10s". Now we do expect that most people
> > will set it to interval
On Mon, Sep 27, 2021 at 04:57:20PM +0530, Nitin Jadhav wrote:
> > It is also not logical to define 0 as meaning that "all messages for
> > such operations are logged". What does that even mean? It makes sense
> > for something like log_autovacuum_min_duration, because there we are
> > talking about
> I really don't know what to say about this. You say that the time is
> measured in milliseconds, and then immediately turn around and say
> "For example, if you set it to 10s". Now we do expect that most people
> will set it to intervals that are measured in seconds rather than
> milliseconds, bu
On Wed, Sep 22, 2021 at 10:28 AM Nitin Jadhav
wrote:
> Thanks Justin for the detailed explanation. Done the necessary changes.
Not really. The documentation here does not make a ton of sense:
+ Sets the time interval between each progress update of the operations
+ performed by
> Michael is right. You updated some of the units based on Robert's suggestion
> to use MS, but didn't update all of the corresponding parts of the patch.
> guc.c says that the units are in MS, which means that unqualified values are
> interpretted as such. But postgresql.conf.sample still says "
> I think you're confusing discussions.
>
> Robert was talking about initdb/bootstrap/single, and I separately and
> independently asked about hot standbys. It seems like Robert and I agreed
> about the desired behavior and there was no further discussion.
Sorry for including 2 separate points in
On Tue, Sep 07, 2021 at 03:07:15PM +0530, Nitin Jadhav wrote:
> > Looking over this version, I realized something I (or you) should have
> > noticed sooner: you've added the RegisterTimeout call to
> > InitPostgres(), but that's for things that are used by all backends,
> > and this is only used by
> Looking over this version, I realized something I (or you) should have
> noticed sooner: you've added the RegisterTimeout call to
> InitPostgres(), but that's for things that are used by all backends,
> and this is only used by the startup process. So it seems to me that
> the right place is Star
> I also agree that this is the better place to do it. Hence modified
> the patch accordingly. The condition "!AmStartupProcess()" is added to
> differentiate whether the call is done from a startup process or some
> other process. Actually StartupXLOG() gets called in 2 places. one in
> StartupPro
On Fri, Sep 03, 2021 at 09:23:27PM -0500, Justin Pryzby wrote:
> On Fri, Sep 03, 2021 at 01:23:56PM +0530, Nitin Jadhav wrote:
> > Please find the updated patch attached.
>
> Please check
> CA+TgmoZtbqxaOLdpNkBcDbz=41tWALA8kpH4M=rwtpyhc7-...@mail.gmail.com
>
> I agree with Robert that a standby
On Fri, Sep 03, 2021 at 01:23:56PM +0530, Nitin Jadhav wrote:
> Please find the updated patch attached.
Please check CA+TgmoZtbqxaOLdpNkBcDbz=41tWALA8kpH4M=rwtpyhc7-...@mail.gmail.com
I agree with Robert that a standby server should not continuously show timing
messages for WAL replay.
Some doc
> > Anything that's not used in other files should be declared static in
> > the file itself, and not present in the header. Once you fix this for
> > reset_startup_progress_timeout, the header won't need to include
> > datatype/timestamp.h any more, which is good, because we don't want
> > header
> Anything that's not used in other files should be declared static in
> the file itself, and not present in the header. Once you fix this for
> reset_startup_progress_timeout, the header won't need to include
> datatype/timestamp.h any more, which is good, because we don't want
> header files to d
On Sat, Aug 14, 2021 at 5:47 PM Justin Pryzby wrote:
> Should this feature distinguish between crash recovery and archive recovery on
> a hot standby ? Otherwise the standby will display this all the time.
>
> 2021-08-14 16:13:33.139 CDT startup[11741] LOG: redo in progress, elapsed
> time: 124
Should this feature distinguish between crash recovery and archive recovery on
a hot standby ? Otherwise the standby will display this all the time.
2021-08-14 16:13:33.139 CDT startup[11741] LOG: redo in progress, elapsed
time: 124.42 s, current LSN: 0/EEE2100
If so, I think maybe you'd check
On Thu, Aug 12, 2021 at 7:40 AM Nitin Jadhav
wrote:
> > I suggest making the GUC GUC_UNIT_MS rather than GUC_UNIT_S, but
> > expressing the default in postgresl.conf.sample as 10s rather than
> > 1ms. I tried values measured in milliseconds just for testing
> > purposes and didn't initially un
> startup_progress_timer_expired should be declared as sig_atomic_t like
> we do in other cases (see interrupt.c).
Fixed.
> The default value of the new GUC is 10s in postgresql.conf.sample, but
> -1 in guc.c. They should both be 10s, and the one in
> postgresql.conf.sample should be commented ou
On Tue, Aug 10, 2021 at 9:28 AM Nitin Jadhav
wrote:
> Please find the updated patch attached.
I think this is getting close. The output looks nice. However, I still
see a bunch of issues.
You mentioned previously that you would add documentation, but I do
not see it here.
startup_progress_timer
> I'd really like to avoid this. I don't see why it's necessary. You say
> it causes a problem, but you don't explain what problem it causes.
> enable_timeout_at() will disable the timer if not already done. I
> think all we need to do is set scheduled_startup_progress_timeout = 0
> before calling
On Mon, Aug 9, 2021 at 11:20 AM Nitin Jadhav
wrote:
> Modified the reset_startup_progress_timeout() to take the second
> parameter which indicates whether it is called for initialization or
> for resetting. Without this parameter there is a problem if we call
> init_startup_progress() more than on
Modified the reset_startup_progress_timeout() to take the second
parameter which indicates whether it is called for initialization or
for resetting. Without this parameter there is a problem if we call
init_startup_progress() more than one time if there is no call to
ereport_startup_progress() in b
On Thu, Aug 5, 2021 at 7:41 AM Nitin Jadhav
wrote:
> This seems a little confusing. As we are setting
> last_startup_progress_timeout = now and then calling
> reset_startup_progress_timeout() which will calculate the next_time
> based on the value of last_startup_progress_timeout initially and
> c
Thanks for the detailed explanation.
> +elapsed_ms = (seconds * 1000) + (useconds / 1000);
> +interval_in_ms = log_startup_progress_interval * 1000;
> +enable_timeout_after(LOG_STARTUP_PROGRESS_TIMEOUT,
> + (interval_in_ms - (elapsed_ms % interval_in_ms)));
>
>
On Tue, Aug 3, 2021 at 2:48 AM Nitin Jadhav
wrote:
> Implemented the above approach and verified the patch. Kindly have a
> look and share your thoughts.
+LogStartupProgressTimerExpired(bool force, long *secs, int *usecs)
The name of this function begins with "log" but it does not log
anything,
Two issues that I saw:
The first syncfs message is not output, because it's before
InitStartupProgress() is called:
2021-08-03 07:53:02.176 CDT startup[9717] LOG: database system was
interrupted; last known up at 2021-08-03 07:52:15 CDT
2021-08-03 07:53:02.733 CDT startup[9717] LOG: data direc
> Thanks. Can you please have a look at what I suggested down toward the
> bottom of
> http://postgr.es/m/ca+tgmoap2wefsktmcgwt9lxuz7y99hnduyshpk7qnfuqb98...@mail.gmail.com
?
Implemented the above approach and verified the patch. Kindly have a
look and share your thoughts.
Thanks & Regards,
Niti
> Thanks. Can you please have a look at what I suggested down toward the
> bottom of
> http://postgr.es/m/ca+tgmoap2wefsktmcgwt9lxuz7y99hnduyshpk7qnfuqb98...@mail.gmail.com
> ?
>
> If we're going to go the route of combining the functions, I agree
> that a Boolean is the way to go; I think we have
On Thu, Jul 29, 2021 at 4:56 AM Nitin Jadhav
wrote:
> Thanks for sharing the information. I have done the necessary changes
> to show the logs during the latter case (postgres --single) and
> verified the log messages.
Thanks. Can you please have a look at what I suggested down toward the
bottom
> The InitPostgres() case occurs when the server is started in bootstrap
> mode (during initdb) or in single-user mode (postgres --single). I do
> not see any reason why we shouldn't produce progress messages in at
> least the latter case. I suspect that someone who is in the rather
> desperate sce
On Wed, Jul 28, 2021 at 11:25 AM Bharath Rupireddy
wrote:
> > Perhaps during initdb we don't want messages, but I'm not sure that we
> > need to do anything about that here. None of the messages that the
> > server normally produces show up when you run initdb, so I guess they
> > are getting redi
On Wed, Jul 28, 2021 at 7:02 PM Robert Haas wrote:
>
> On Wed, Jul 28, 2021 at 5:24 AM Nitin Jadhav
> wrote:
> > I also agree that this is the better place to do it. Hence modified
> > the patch accordingly. The condition "!AmStartupProcess()" is added to
> > differentiate whether the call is don
On Wed, Jul 28, 2021 at 5:24 AM Nitin Jadhav
wrote:
> I also agree that this is the better place to do it. Hence modified
> the patch accordingly. The condition "!AmStartupProcess()" is added to
> differentiate whether the call is done from a startup process or some
> other process. Actually Start
> > > I saw that you fixed it by calling InitStartupProgress() after the
> > > walkdir()
> > > calls which do pre_sync_fname. So then walkdir is calling
> > > LogStartupProgress(STARTUP_PROCESS_OP_FSYNC) even when it's not doing
> > > fsync,
> > > and then LogStartupProgress() is returning becau
On Mon, Jul 26, 2021 at 11:30 AM Justin Pryzby wrote:
> > Maybe I'm missing something here, but I don't understand the purpose
> > of this. You can always combine two functions into one, but it's only
> > worth doing if you end up with less code, which doesn't seem to be the
> > case here.
>
> 4
On Mon, Jul 26, 2021 at 10:13:09AM -0400, Robert Haas wrote:
> I don't think walkdir() has any business calling LogStartupProgress()
> at all. It's supposed to be a generic function, not one that is only
> available to be called from the startup process, or has different
> behavior there. From my p
On Sun, Jul 25, 2021 at 1:56 PM Justin Pryzby wrote:
> On Fri, Jul 23, 2021 at 04:09:47PM +0530, Nitin Jadhav wrote:
> > > I think walkdir() should only call LogStartupProgress(FSYNC_IN_PROGRESS,
> > > path);
> > > when action == datadir_fsync_fname.
> >
> > I agree and fixed it.
>
> I saw that y
On Fri, Jul 23, 2021 at 04:09:47PM +0530, Nitin Jadhav wrote:
> > I think walkdir() should only call LogStartupProgress(FSYNC_IN_PROGRESS,
> > path);
> > when action == datadir_fsync_fname.
>
> I agree and fixed it.
I saw that you fixed it by calling InitStartupProgress() after the walkdir()
cal
> I still don't see the need for two functions LogStartupProgress and
> LogStartupProgressComplete. Most of the code is duplicate. I think we
> can just do it with a single function something like [1]:
Initially I had written a common function for these 2. You can see
that in the earlier version o
I think walkdir() should only call LogStartupProgress(FSYNC_IN_PROGRESS, path);
when action == datadir_fsync_fname.
ResetUnloggedRelations() is calling
ResetUnloggedRelationsInTablespaceDir("base", op);
before calling InitStartupProgress().
This shows StartupXLOG() calling ResetUnloggedRelations(
On Wed, Jul 21, 2021 at 12:52 PM Nitin Jadhav
wrote:
> Please find the v5 patch attached. Kindly let me know your comments.
Thanks for the patch. Here are some comments on v5:
1) I still don't see the need for two functions LogStartupProgress and
LogStartupProgressComplete. Most of the code is du
> I am not sure I am getting the code flow correctly. From
> CloseStartupProgress()
> naming it seems, it corresponds to InitStartupProgress() but what it is doing
> is similar to LogStartupProgress(). I think it should be renamed to be inlined
> with LogStartupProgress(), IMO.
Renamed CloseStart
1 - 100 of 142 matches
Mail list logo
