On Wed, Nov 16, 2022 at 5:24 PM Nathan Bossart wrote:
> On Wed, Nov 16, 2022 at 04:57:08PM +1300, Thomas Munro wrote:
> > On Tue, Nov 15, 2022 at 5:49 PM Nathan Bossart
> > wrote:
> >> Another option might be to just force initial reply/feedback messages when
> >> streaming starts. The attached
On Wed, Nov 16, 2022 at 04:57:08PM +1300, Thomas Munro wrote:
> On Tue, Nov 15, 2022 at 5:49 PM Nathan Bossart
> wrote:
>> Another option might be to just force initial reply/feedback messages when
>> streaming starts. The attached patch improves src/test/recovery test
>> runtime just like the p
On Tue, Nov 15, 2022 at 5:49 PM Nathan Bossart wrote:
> Another option might be to just force initial reply/feedback messages when
> streaming starts. The attached patch improves src/test/recovery test
> runtime just like the previous one I posted.
Yeah, looks good in my tests. I think we just
Another option might be to just force initial reply/feedback messages when
streaming starts. The attached patch improves src/test/recovery test
runtime just like the previous one I posted.
On Mon, Nov 14, 2022 at 11:01:27AM -0800, Nathan Bossart wrote:
> I wonder if we
> ought to do something sim
On Tue, Nov 15, 2022 at 12:14 PM Nathan Bossart
wrote:
> On Tue, Nov 15, 2022 at 09:42:26AM +1300, Thomas Munro wrote:
> > That works for 020_pg_receivewal. I wonder if there are also tests
> > that stream a bit of WAL first and then do wait_for_catchup that were
> > previously benefiting from th
On Tue, Nov 15, 2022 at 09:42:26AM +1300, Thomas Munro wrote:
> That works for 020_pg_receivewal. I wonder if there are also tests
> that stream a bit of WAL first and then do wait_for_catchup that were
> previously benefiting from the 100ms-after-startup message by
> scheduling luck (as in, that
On Tue, Nov 15, 2022 at 8:01 AM Nathan Bossart wrote:
> Is there any reason we should wait for 100ms before sending the initial
> reply? ISTM the previous behavior essentially caused the first reply (and
> feedback message) to be sent at the first opportunity after streaming
> begins. My instinc
On Mon, Nov 14, 2022 at 02:47:14PM +1300, Thomas Munro wrote:
> Ahh, I think I might have it. In the old coding, sendTime starts out
> as 0, which I think caused it to send its first reply message after
> the first 100ms sleep, and only after that fall into a cadence of
> wal_receiver_status_inter
On Mon, Nov 14, 2022 at 1:05 PM Nathan Bossart wrote:
> Yeah, I'm able to sort-of reproduce the problem by sending fewer replies,
> but it's not clear to me why that's the problem. AFAICT all of the code
> paths that write/flush are careful to send a reply shortly afterward, which
> should keep w
On Mon, Nov 14, 2022 at 12:51:14PM +1300, Thomas Munro wrote:
> On Mon, Nov 14, 2022 at 12:35 PM Thomas Munro wrote:
>> Maybe there is a better way to code this (I mean, who likes global
>> variables?) and I need to test some more, but I suspect the attached
>> is approximately what we missed.
>
On Mon, Nov 14, 2022 at 12:35 PM Thomas Munro wrote:
> Maybe there is a better way to code this (I mean, who likes global
> variables?) and I need to test some more, but I suspect the attached
> is approximately what we missed.
Erm, ENOCOFFEE, sorry that's not quite right, it needs the apply LSN,
On Mon, Nov 14, 2022 at 12:11 PM Thomas Munro wrote:
> On Mon, Nov 14, 2022 at 11:26 AM Nathan Bossart
> wrote:
> > On Sun, Nov 13, 2022 at 05:08:04PM -0500, Tom Lane wrote:
> > > There is something very seriously wrong with this patch.
> > >
> > > On my machine, running "make -j10 check-world" (
On Mon, Nov 14, 2022 at 11:26 AM Nathan Bossart
wrote:
> On Sun, Nov 13, 2022 at 05:08:04PM -0500, Tom Lane wrote:
> > There is something very seriously wrong with this patch.
> >
> > On my machine, running "make -j10 check-world" (with compilation
> > already done) has been taking right about 2 m
On Sun, Nov 13, 2022 at 05:08:04PM -0500, Tom Lane wrote:
> There is something very seriously wrong with this patch.
>
> On my machine, running "make -j10 check-world" (with compilation
> already done) has been taking right about 2 minutes for some time.
> Since this patch, it's taking around 2:45
On Mon, Nov 14, 2022 at 11:08 AM Tom Lane wrote:
> There is something very seriously wrong with this patch.
>
> On my machine, running "make -j10 check-world" (with compilation
> already done) has been taking right about 2 minutes for some time.
> Since this patch, it's taking around 2:45 --- I di
Thomas Munro writes:
> And with that change and a pgindent, pushed.
There is something very seriously wrong with this patch.
On my machine, running "make -j10 check-world" (with compilation
already done) has been taking right about 2 minutes for some time.
Since this patch, it's taking around 2:
On Wed, Nov 9, 2022 at 2:38 AM Nathan Bossart wrote:
>
> On Tue, Nov 08, 2022 at 09:45:40PM +1300, Thomas Munro wrote:
> > On Tue, Nov 8, 2022 at 9:20 PM Bharath Rupireddy
> > wrote:
> >> Thanks. Do we need a similar wakeup approach for logical replication
> >> workers in worker.c? Or is it okay
On Tue, Nov 08, 2022 at 09:45:40PM +1300, Thomas Munro wrote:
> On Tue, Nov 8, 2022 at 9:20 PM Bharath Rupireddy
> wrote:
>> Thanks. Do we need a similar wakeup approach for logical replication
>> workers in worker.c? Or is it okay that the nap time is 1sec there?
>
> Yeah, I think so. At least
On Tue, Nov 08, 2022 at 08:46:26PM +1300, Thomas Munro wrote:
> On Tue, Nov 8, 2022 at 3:20 PM Thomas Munro wrote:
>> This looks pretty good to me. Thanks for picking it up! I can live
>> with the change to use a global variable; it seems we couldn't quite
>> decide what the scope was for moving
On Tue, Nov 8, 2022 at 9:20 PM Bharath Rupireddy
wrote:
> Thanks. Do we need a similar wakeup approach for logical replication
> workers in worker.c? Or is it okay that the nap time is 1sec there?
Yeah, I think so. At least for its idle/nap case (waiting for flush
is also a technically interesti
On Tue, Nov 8, 2022 at 1:17 PM Thomas Munro wrote:
>
> And with that change and a pgindent, pushed.
Thanks. Do we need a similar wakeup approach for logical replication
workers in worker.c? Or is it okay that the nap time is 1sec there?
--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open
On Tue, Nov 8, 2022 at 3:20 PM Thomas Munro wrote:
> On Sun, Nov 6, 2022 at 12:01 PM Nathan Bossart
> wrote:
> > Here is a new version of the patch that addresses this feedback.
>
> This looks pretty good to me. Thanks for picking it up! I can live
> with the change to use a global variable; i
On Sun, Nov 6, 2022 at 12:01 PM Nathan Bossart wrote:
> Here is a new version of the patch that addresses this feedback.
This looks pretty good to me. Thanks for picking it up! I can live
with the change to use a global variable; it seems we couldn't quite
decide what the scope was for moving s
On Sat, Nov 05, 2022 at 03:00:55PM -0700, Nathan Bossart wrote:
> On Mon, Oct 17, 2022 at 03:21:18PM +0900, Kyotaro Horiguchi wrote:
>> Now that I see the fix for the implicit conversion:
>>
>> L527: + nap = Max(0, (nextWakeup - now + 999) /
>> 1000);
>> ..
>> L545: +
On Wed, Oct 26, 2022 at 03:05:20PM +0530, Bharath Rupireddy wrote:
> A comment on the patch:
> Isn't it better we track the soonest wakeup time or min of wakeup[]
> array (update the min whenever the array element is updated in
> WalRcvComputeNextWakeup()) instead of recomputing everytime by loopin
On Mon, Oct 17, 2022 at 03:21:18PM +0900, Kyotaro Horiguchi wrote:
> Now that I see the fix for the implicit conversion:
>
> L527: + nap = Max(0, (nextWakeup - now + 999) /
> 1000);
> ..
> L545: +
On Sun, Oct 16, 2022 at 9:29 AM Nathan Bossart wrote:
>
> Here's a different take. Instead of creating structs and altering function
> signatures, we can just make the wake-up times file-global, and we can skip
> the changes related to reducing the number of calls to
> GetCurrentTimestamp() for n
Thanks for taking this up.
At Sat, 15 Oct 2022 20:59:00 -0700, Nathan Bossart
wrote in
> On Thu, Oct 13, 2022 at 12:09:54PM -0700, Nathan Bossart wrote:
> > On Thu, Oct 13, 2022 at 12:37:39PM +0200, Alvaro Herrera wrote:
> >> The main reason is that it seems odd to have startpointTLI in the str
On Thu, Oct 13, 2022 at 12:09:54PM -0700, Nathan Bossart wrote:
> On Thu, Oct 13, 2022 at 12:37:39PM +0200, Alvaro Herrera wrote:
>> The main reason is that it seems odd to have startpointTLI in the struct
>> used in some places together with a file-global recvFileTLI which isn't.
>> The way one is
On Thu, Oct 13, 2022 at 12:37:39PM +0200, Alvaro Herrera wrote:
> I think in 0001 we should put more stuff in the state struct --
> specifically these globals:
>
> static intrecvFile = -1;
> static TimeLineID recvFileTLI = 0;
> static XLogSegNo recvSegNo = 0;
>
> The main reason is that it se
On Thu, Oct 13, 2022 at 03:35:14PM +0530, Bharath Rupireddy wrote:
> I think the hot standby feedback message gets sent too frequently
> without honouring the wal_receiver_status_interval because the 'now'
> is actually not the current time with your patch but 'now +
> XLogWalRcvSendReply()'s time'
I think in 0001 we should put more stuff in the state struct --
specifically these globals:
static int recvFile = -1;
static TimeLineID recvFileTLI = 0;
static XLogSegNo recvSegNo = 0;
The main reason is that it seems odd to have startpointTLI in the struct
used in some places together with
On Tue, Oct 11, 2022 at 11:22 PM Nathan Bossart
wrote:
>
> On Tue, Oct 11, 2022 at 09:34:25AM +0530, Bharath Rupireddy wrote:
> > now = t1;
> > XLogWalRcvSendReply() /* say it ran for a minute or so for whatever reasons
> > */
> > XLogWalRcvSendHSFeedback() /* with patch walrecevier sends hot sta
On Tue, Oct 11, 2022 at 09:34:25AM +0530, Bharath Rupireddy wrote:
> now = t1;
> XLogWalRcvSendReply() /* say it ran for a minute or so for whatever reasons */
> XLogWalRcvSendHSFeedback() /* with patch walrecevier sends hot standby
> feedback more often without properly honouring
> wal_receiver_st
On Tue, Oct 11, 2022 at 8:20 AM Nathan Bossart wrote:
>
> > AFICS, the aim of the patch isn't optimizing around
> > GetCurrentTimestamp() calls and the patch does seem to change quite a
> > bit of them which may cause a change of the behaviour. I think that
> > the GetCurrentTimestamp() optimizati
On Tue, Oct 11, 2022 at 07:01:26AM +0530, Bharath Rupireddy wrote:
> On Mon, Oct 10, 2022 at 11:21 PM Nathan Bossart
>> Outside of the code snippet you pointed out,
>> I think WalReceiverMain() has a similar problem. That being said, I'm
>> generally skeptical that this sort of thing is detrimenta
On Mon, Oct 10, 2022 at 11:21 PM Nathan Bossart
wrote:
>
Thanks for the updated patches.
> > 2. With the below change, the time walreceiver spends in
> > XLogWalRcvSendReply() is also included for XLogWalRcvSendHSFeedback
> > right? I think it's a problem given that XLogWalRcvSendReply() can
> >
On Mon, Oct 10, 2022 at 11:10:14AM -0700, Nathan Bossart wrote:
> On Mon, Oct 10, 2022 at 10:51:14AM -0700, Nathan Bossart wrote:
>>> +/* Find the soonest wakeup time, to limit our nap. */
>>> +nextWakeup = INT64_MAX;
>>> +for (int i = 0; i < NUM_WALR
On Mon, Oct 10, 2022 at 10:51:14AM -0700, Nathan Bossart wrote:
>> +/* Find the soonest wakeup time, to limit our nap. */
>> +nextWakeup = INT64_MAX;
>> +for (int i = 0; i < NUM_WALRCV_WAKEUPS; ++i)
>> +nextWakeup = Min(state.wakeu
On Mon, Oct 10, 2022 at 03:22:15PM +0530, Bharath Rupireddy wrote:
> Some comments on v4-0002:
Thanks for taking a look.
> 1. You might want to use PG_INT64_MAX instead of INT64_MAX for portability?
Yes, I used PG_INT64_MAX in v5.
> 2. With the below change, the time walreceiver spends in
> XLo
On Wed, Oct 5, 2022 at 11:38 PM Nathan Bossart wrote:
>
> I moved as much as I could to 0001 in v4.
Some comments on v4-0002:
1. You might want to use PG_INT64_MAX instead of INT64_MAX for portability?
2. With the below change, the time walreceiver spends in
XLogWalRcvSendReply() is also includ
Thanks for taking a look.
On Wed, Oct 05, 2022 at 09:57:00AM +0200, Alvaro Herrera wrote:
> On 2022-Oct-04, Nathan Bossart wrote:
>> * The creation of the struct for non-shared WAL receiver state is moved to
>> a prerequisite 0001 patch. This should help ease review of 0002 a bit.
>
> I think th
On 2022-Oct-04, Nathan Bossart wrote:
> Here is an updated patch set with the following changes:
>
> * The creation of the struct for non-shared WAL receiver state is moved to
> a prerequisite 0001 patch. This should help ease review of 0002 a bit.
I think that would be even better if you moved
Here is an updated patch set with the following changes:
* The creation of the struct for non-shared WAL receiver state is moved to
a prerequisite 0001 patch. This should help ease review of 0002 a bit.
* I updated the nap time calculation to round up to the next millisecond,
as discussed upthre
On Fri, Sep 30, 2022 at 05:04:43PM +1300, Thomas Munro wrote:
> Please go ahead! One thing I had in my notes to check with this patch
> is whether there might be a bug due to computing all times in
> microseconds, but sleeping for a number of milliseconds without
> rounding up (what I mean is that
On Fri, Sep 30, 2022 at 4:51 PM Nathan Bossart wrote:
> Thomas, are you planning to continue with this patch? If not, I don't mind
> picking it up.
Hi Nathan,
Please go ahead! One thing I had in my notes to check with this patch
is whether there might be a bug due to computing all times in
mic
I was surprised to see that this patch was never committed!
On Mon, Jan 31, 2022 at 04:28:11PM +0900, Kyotaro Horiguchi wrote:
> +static void
> +WalRcvComputeNextWakeup(WalRcvInfo *state,
> + WalRcvWakeupReason reason,
> +
At Fri, 28 Jan 2022 22:41:32 +1300, Thomas Munro wrote
in
> On Fri, Jan 28, 2022 at 8:26 PM Kyotaro Horiguchi
> wrote:
> > At Thu, 27 Jan 2022 23:50:04 +1300, Thomas Munro
> > wrote in
> The reason why I put the timeout computation into a function is
> because there are about 3 places you nee
On Fri, Jan 28, 2022 at 8:26 PM Kyotaro Horiguchi
wrote:
> At Thu, 27 Jan 2022 23:50:04 +1300, Thomas Munro
> wrote in
> > While working on WaitEventSet-ifying various codepaths, I found it
> > strange that walreceiver wakes up 10 times per second while idle.
> > Here's a draft patch to compute
Hello.
At Thu, 27 Jan 2022 23:50:04 +1300, Thomas Munro wrote
in
> While working on WaitEventSet-ifying various codepaths, I found it
> strange that walreceiver wakes up 10 times per second while idle.
> Here's a draft patch to compute the correct sleep time.
Agree to the objective. However I
Hi,
While working on WaitEventSet-ifying various codepaths, I found it
strange that walreceiver wakes up 10 times per second while idle.
Here's a draft patch to compute the correct sleep time.
From 553d2dae8f8e7bb46ac73ca739aac03862f473cc Mon Sep 17 00:00:00 2001
From: Thomas Munro
Date: Thu, 27
51 matches
Mail list logo
