On Wed, Sep 20, 2023 at 04:48:00PM +0530, Ashutosh Bapat wrote:
> Commitfest entry "https://commitfest.postgresql.org/44/4536/ is in
> "Ready for committer" state. Is there something remaining here? We
> should probably set it as "committed".
Thanks, I've switched that to "Committed".
--
Michael
On Thu, Sep 14, 2023 at 2:41 PM Amit Kapila wrote:
>
> On Thu, Sep 14, 2023 at 7:20 AM Michael Paquier wrote:
> >
> > On Wed, Sep 13, 2023 at 04:20:37PM +0530, Amit Kapila wrote:
> > > The patch is updated as per recent discussion.
> >
> > WFM. Thanks for the updated version.
> >
>
> Pushed.
On Thu, Sep 14, 2023 at 7:20 AM Michael Paquier wrote:
>
> On Wed, Sep 13, 2023 at 04:20:37PM +0530, Amit Kapila wrote:
> > The patch is updated as per recent discussion.
>
> WFM. Thanks for the updated version.
>
Pushed.
--
With Regards,
Amit Kapila.
On Wed, Sep 13, 2023 at 04:20:37PM +0530, Amit Kapila wrote:
> The patch is updated as per recent discussion.
WFM. Thanks for the updated version.
--
Michael
signature.asc
Description: PGP signature
On Wed, Sep 13, 2023 at 12:45 PM Michael Paquier wrote:
>
> On Wed, Sep 13, 2023 at 12:07:12PM +0530, Amit Kapila wrote:
> > Consider if we move this call to bgwriter (aka flushing slots is no
> > longer part of a checkpoint), Would that be okay? Previously, I think
> > it was okay but not now. I
On Wed, Sep 13, 2023 at 12:07:12PM +0530, Amit Kapila wrote:
> Consider if we move this call to bgwriter (aka flushing slots is no
> longer part of a checkpoint), Would that be okay? Previously, I think
> it was okay but not now. I see an argument to keep that as it is as
> well because we have
On Wed, Sep 13, 2023 at 10:57 AM Michael Paquier wrote:
>
> On Tue, Sep 12, 2023 at 03:15:44PM +0530, Amit Kapila wrote:
> >>
> >> - Not sure that there is any point to mention the other code paths in
> >> the tree where ReplicationSlotSave() can be called, and a slot can be
> >> saved in other
On Tue, Sep 12, 2023 at 03:15:44PM +0530, Amit Kapila wrote:
> I don't think it will become more responsive in any way, not sure what
> made you think like that. The key idea is that after restart we want
> to ensure that all the WAL data up to the shutdown checkpoint record
> is sent to
On Tue, Sep 12, 2023 at 10:55 AM Michael Paquier wrote:
>
> On Mon, Sep 11, 2023 at 02:49:49PM +0530, Amit Kapila wrote:
>
> Please see the v11 attached, that rewords all the places of the patch
> that need clarifications IMO. I've found that the comment additions
> in
On Mon, Sep 11, 2023 at 02:49:49PM +0530, Amit Kapila wrote:
> I don't know if the difference is worth inventing a new BACKEND_TYPE_*
> but if you think so then we can probably discuss this in a new thread.
> I think we may want to improve some comments as a separate patch to
> make this evident.
On Mon, Sep 11, 2023 at 12:08 PM Michael Paquier wrote:
>
> On Fri, Sep 08, 2023 at 11:50:37AM +0530, Amit Kapila wrote:
> > On Fri, Sep 8, 2023 at 10:08 AM Michael Paquier wrote:
> >>
> >>
> >> +/*
> >> + * This is used to track the last persisted confirmed_flush LSN value
> >> to
> >>
On Fri, Sep 08, 2023 at 11:50:37AM +0530, Amit Kapila wrote:
> On Fri, Sep 8, 2023 at 10:08 AM Michael Paquier wrote:
>>
>>
>> +/*
>> + * This is used to track the last persisted confirmed_flush LSN value to
>> + * detect any divergence in the in-memory and on-disk values for the
>>
On Fri, Sep 8, 2023 at 10:08 AM Michael Paquier wrote:
>
>
> +/*
> + * This is used to track the last persisted confirmed_flush LSN value to
> + * detect any divergence in the in-memory and on-disk values for the
> same.
> + */
>
> "This value tracks is the last LSN where this
On Fri, Sep 08, 2023 at 09:04:43AM +0530, Amit Kapila wrote:
> I have added something on these lines and also changed the other
> comment pointed out by you. In the passing, I made minor cosmetic
> changes as well.
+ * We can flush dirty replication slots at regular intervals by any
+ *
On Thu, Sep 7, 2023 at 3:38 PM Ashutosh Bapat
wrote:
>
> On Thu, Sep 7, 2023 at 1:43 PM Amit Kapila wrote:
> >
> > On Thu, Sep 7, 2023 at 1:18 PM Michael Paquier wrote:
> > >
> > > On Thu, Sep 07, 2023 at 11:56:28AM +0530, Amit Kapila wrote:
> > > > Thanks, the patch looks good to me as well.
>
On Thu, Sep 7, 2023 at 4:30 PM Ashutosh Bapat
wrote:
>
> On Thu, Sep 7, 2023 at 4:11 PM Amit Kapila wrote:
> >
> > On Thu, Sep 7, 2023 at 3:38 PM Ashutosh Bapat
> > wrote:
> > >
> > > * This needn't actually be part of a checkpoint, but it's a convenient
> > > - * location.
> > > + *
On Thu, Sep 7, 2023 at 4:11 PM Amit Kapila wrote:
>
> On Thu, Sep 7, 2023 at 3:38 PM Ashutosh Bapat
> wrote:
> >
> > * This needn't actually be part of a checkpoint, but it's a convenient
> > - * location.
> > + * location. is_shutdown is true in case of a shutdown checkpoint.
> >
> > Relying
On Thu, Sep 7, 2023 at 3:38 PM Ashutosh Bapat
wrote:
>
> * This needn't actually be part of a checkpoint, but it's a convenient
> - * location.
> + * location. is_shutdown is true in case of a shutdown checkpoint.
>
> Relying on the first sentence, if we decide to not persist the
> replication
On Thu, Sep 7, 2023 at 1:43 PM Amit Kapila wrote:
>
> On Thu, Sep 7, 2023 at 1:18 PM Michael Paquier wrote:
> >
> > On Thu, Sep 07, 2023 at 11:56:28AM +0530, Amit Kapila wrote:
> > > Thanks, the patch looks good to me as well.
> >
> > + /* This is used to track the last saved confirmed_flush
On Thu, Sep 7, 2023 at 1:18 PM Michael Paquier wrote:
>
> On Thu, Sep 07, 2023 at 11:56:28AM +0530, Amit Kapila wrote:
> > Thanks, the patch looks good to me as well.
>
> + /* This is used to track the last saved confirmed_flush LSN value */
> + XLogRecPtr last_saved_confirmed_flush;
>
>
On Thu, Sep 07, 2023 at 11:56:28AM +0530, Amit Kapila wrote:
> Thanks, the patch looks good to me as well.
+ /* This is used to track the last saved confirmed_flush LSN value */
+ XLogRecPtr last_saved_confirmed_flush;
This does not feel sufficient, as the comment explaining what this
On Thu, Sep 7, 2023 at 10:13 AM vignesh C wrote:
>
> On Wed, 6 Sept 2023 at 12:09, Dilip Kumar wrote:
> >
> > Other than that the patch LGTM.
>
> pgindent reported that the new comments added need to be re-adjusted,
> here is an updated patch for the same.
>
Thanks, the patch looks good to me
On Wed, 6 Sept 2023 at 12:09, Dilip Kumar wrote:
>
> Other than that the patch LGTM.
pgindent reported that the new comments added need to be re-adjusted,
here is an updated patch for the same.
I also verified the following: a) patch applies neatly on HEAD b) make
check-world passes c) pgindent
On Wed, Sep 6, 2023 at 12:01 PM Amit Kapila wrote:
>
> On Wed, Sep 6, 2023 at 9:57 AM Dilip Kumar wrote:
> >
> > On Wed, Sep 6, 2023 at 9:47 AM Amit Kapila wrote:
> > >
> > > On Tue, Sep 5, 2023 at 6:00 PM Dilip Kumar wrote:
> >
> > >
> > > Right, it can change and in that case, the check
On Wed, Sep 6, 2023 at 9:57 AM Dilip Kumar wrote:
>
> On Wed, Sep 6, 2023 at 9:47 AM Amit Kapila wrote:
> >
> > On Tue, Sep 5, 2023 at 6:00 PM Dilip Kumar wrote:
>
> >
> > Right, it can change and in that case, the check related to
> > confirm_flush LSN will fail during the upgrade. However,
On Wed, Sep 6, 2023 at 9:47 AM Amit Kapila wrote:
>
> On Tue, Sep 5, 2023 at 6:00 PM Dilip Kumar wrote:
>
> Right, it can change and in that case, the check related to
> confirm_flush LSN will fail during the upgrade. However, the point is
> that if we don't take spinlock, we need to properly
On Tue, Sep 5, 2023 at 6:00 PM Dilip Kumar wrote:
>
> On Tue, Sep 5, 2023 at 5:04 PM Zhijie Hou (Fujitsu)
> wrote:
> >
>
> > > Can't we just have code like this? I mean we will have to make
> > > ReplicationSlotMarkDirty take slot as an argument or have another version
> > > which takes slot as
On Tue, Sep 5, 2023 at 5:04 PM Zhijie Hou (Fujitsu)
wrote:
>
> > Can't we just have code like this? I mean we will have to make
> > ReplicationSlotMarkDirty take slot as an argument or have another version
> > which takes slot as an argument and that would be called by us as well as by
> >
On Tuesday, September 5, 2023 4:15 PM Dilip Kumar wrote:
Hi,
>
> On Tue, Sep 5, 2023 at 10:12 AM Amit Kapila
> wrote:
> >
> > On Tue, Sep 5, 2023 at 7:54 AM Zhijie Hou (Fujitsu)
> > wrote:
> > >
> > > On Monday, September 4, 2023 6:15 PM vignesh C
> wrote:
> > > >
> > > > On Mon, 4 Sept
On Tue, Sep 5, 2023 at 10:12 AM Amit Kapila wrote:
>
> On Tue, Sep 5, 2023 at 7:54 AM Zhijie Hou (Fujitsu)
> wrote:
> >
> > On Monday, September 4, 2023 6:15 PM vignesh C wrote:
> > >
> > > On Mon, 4 Sept 2023 at 15:20, Amit Kapila wrote:
> > > >
> > > > On Fri, Sep 1, 2023 at 10:50 AM vignesh
On Tue, 5 Sept 2023 at 09:10, Dilip Kumar wrote:
>
> On Fri, Sep 1, 2023 at 12:16 PM vignesh C wrote:
> >
> > On Fri, 1 Sept 2023 at 10:06, Amit Kapila wrote:
> > >
> > > On Thu, Aug 31, 2023 at 6:12 PM Ashutosh Bapat
> > > wrote:
> > > >
> > > > On Thu, Aug 31, 2023 at 2:52 PM Amit Kapila
>
On Tue, Sep 5, 2023 at 9:58 AM Amit Kapila wrote:
>
> On Tue, Sep 5, 2023 at 9:10 AM Dilip Kumar wrote:
> >
> > The comments don't mention anything about why we are just flushing at
> > the shutdown checkpoint. I mean the checkpoint is not that frequent
> > and we already perform a lot of I/O
On Tue, Sep 5, 2023 at 9:10 AM Dilip Kumar wrote:
>
> On Fri, Sep 1, 2023 at 12:16 PM vignesh C wrote:
> >
> > On Fri, 1 Sept 2023 at 10:06, Amit Kapila wrote:
> > >
> > > On Thu, Aug 31, 2023 at 6:12 PM Ashutosh Bapat
> > > wrote:
> > > >
> > > > On Thu, Aug 31, 2023 at 2:52 PM Amit Kapila
On Fri, Sep 1, 2023 at 12:16 PM vignesh C wrote:
>
> On Fri, 1 Sept 2023 at 10:06, Amit Kapila wrote:
> >
> > On Thu, Aug 31, 2023 at 6:12 PM Ashutosh Bapat
> > wrote:
> > >
> > > On Thu, Aug 31, 2023 at 2:52 PM Amit Kapila
> > > wrote:
> > > >
> > > > All but one. Normally, the idea of
On Tue, Sep 5, 2023 at 7:54 AM Zhijie Hou (Fujitsu)
wrote:
>
> On Monday, September 4, 2023 6:15 PM vignesh C wrote:
> >
> > On Mon, 4 Sept 2023 at 15:20, Amit Kapila wrote:
> > >
> > > On Fri, Sep 1, 2023 at 10:50 AM vignesh C wrote:
> > > >
> > > > On Fri, 1 Sept 2023 at 10:06, Amit Kapila
On Monday, September 4, 2023 6:15 PM vignesh C wrote:
>
> On Mon, 4 Sept 2023 at 15:20, Amit Kapila wrote:
> >
> > On Fri, Sep 1, 2023 at 10:50 AM vignesh C wrote:
> > >
> > > On Fri, 1 Sept 2023 at 10:06, Amit Kapila wrote:
> > > >
> > > > On Thu, Aug 31, 2023 at 6:12 PM Ashutosh Bapat
> > >
On Mon, 4 Sept 2023 at 15:20, Amit Kapila wrote:
>
> On Fri, Sep 1, 2023 at 10:50 AM vignesh C wrote:
> >
> > On Fri, 1 Sept 2023 at 10:06, Amit Kapila wrote:
> > >
> > > On Thu, Aug 31, 2023 at 6:12 PM Ashutosh Bapat
> > > wrote:
> > > >
> > > > On Thu, Aug 31, 2023 at 2:52 PM Amit Kapila
>
On Fri, Sep 1, 2023 at 10:50 AM vignesh C wrote:
>
> On Fri, 1 Sept 2023 at 10:06, Amit Kapila wrote:
> >
> > On Thu, Aug 31, 2023 at 6:12 PM Ashutosh Bapat
> > wrote:
> > >
> > > On Thu, Aug 31, 2023 at 2:52 PM Amit Kapila
> > > wrote:
> > > >
> > > > All but one. Normally, the idea of
On Fri, Sep 1, 2023 at 1:11 PM Ashutosh Bapat
wrote:
>
> On Thu, Aug 31, 2023 at 7:28 PM Amit Kapila wrote:
> >
> > On Thu, Aug 31, 2023 at 6:12 PM Ashutosh Bapat
> > wrote:
> > >
> > > > + qr/Streaming transactions committing after ([A-F0-9]+\/[A-F0-9]+),
> > > > reading WAL from
On Thu, Aug 31, 2023 at 7:28 PM Amit Kapila wrote:
>
> On Thu, Aug 31, 2023 at 6:12 PM Ashutosh Bapat
> wrote:
> >
> > On Thu, Aug 31, 2023 at 2:52 PM Amit Kapila wrote:
> > > > > > I
> > > > > > think we should shut down subscriber, restart publisher and then
> > > > > > make this
> > > > > >
On Fri, 1 Sept 2023 at 10:06, Amit Kapila wrote:
>
> On Thu, Aug 31, 2023 at 6:12 PM Ashutosh Bapat
> wrote:
> >
> > On Thu, Aug 31, 2023 at 2:52 PM Amit Kapila wrote:
> > >
> > > All but one. Normally, the idea of marking dirty is to indicate that
> > > we will actually write/flush the
On Thu, Aug 31, 2023 at 6:12 PM Ashutosh Bapat
wrote:
>
> On Thu, Aug 31, 2023 at 2:52 PM Amit Kapila wrote:
> >
> > All but one. Normally, the idea of marking dirty is to indicate that
> > we will actually write/flush the contents at a later point (except
> > when required for correctness) as
On Thu, Aug 31, 2023 at 6:12 PM Ashutosh Bapat
wrote:
>
> On Thu, Aug 31, 2023 at 2:52 PM Amit Kapila wrote:
> > > > > I
> > > > > think we should shut down subscriber, restart publisher and then make
> > > > > this
> > > > > check based on the contents of the replication slot instead of server
On Thu, Aug 31, 2023 at 2:52 PM Amit Kapila wrote:
> > > > I
> > > > think we should shut down subscriber, restart publisher and then make
> > > > this
> > > > check based on the contents of the replication slot instead of server
> > > > log.
> > > > Shutting down subscriber will ensure that
On Thu, Aug 31, 2023 at 12:25 PM Ashutosh Bapat
wrote:
>
> On Thu, Aug 31, 2023 at 12:10 PM Amit Kapila wrote:
> >
> > > +
> > > +/*
> > > + * We won't ensure that the slot is persisted after the
> > > confirmed_flush
> > > + * LSN is updated as that could lead to frequent writes.
On Thu, Aug 31, 2023 at 12:10 PM Amit Kapila wrote:
>
> > +
> > +/*
> > + * We won't ensure that the slot is persisted after the confirmed_flush
> > + * LSN is updated as that could lead to frequent writes. However, we
> > need
> > + * to ensure that we do persist the slots at
On Wed, Aug 30, 2023 at 6:33 PM Ashutosh Bapat
wrote:
>
> On Tue, Aug 29, 2023 at 5:40 PM Ashutosh Bapat
> wrote:
> >
> > I am looking at it. If you can wait till the end of the week, that
> > will be great.
>
> /*
> * Successfully wrote, unset dirty bit, unless somebody dirtied again
On Tue, Aug 29, 2023 at 5:40 PM Ashutosh Bapat
wrote:
>
> I am looking at it. If you can wait till the end of the week, that
> will be great.
/*
* Successfully wrote, unset dirty bit, unless somebody dirtied again
- * already.
+ * already and remember the confirmed_flush LSN
On Wed, Aug 30, 2023 at 9:03 AM Julien Rouhaud wrote:
>
> On Tue, Aug 29, 2023 at 02:21:15PM +0530, Amit Kapila wrote:
> > On Tue, Aug 29, 2023 at 10:16 AM vignesh C wrote:
> > >
> > > That makes sense. The attached v6 version has the changes for the
> > > same, apart from this I have also fixed
Hi,
On Tue, Aug 29, 2023 at 02:21:15PM +0530, Amit Kapila wrote:
> On Tue, Aug 29, 2023 at 10:16 AM vignesh C wrote:
> >
> > That makes sense. The attached v6 version has the changes for the
> > same, apart from this I have also fixed a) pgindent issues b) perltidy
> > issues c) one variable
On Tue, Aug 29, 2023 at 2:21 PM Amit Kapila wrote:
>
> On Tue, Aug 29, 2023 at 10:16 AM vignesh C wrote:
> >
> > That makes sense. The attached v6 version has the changes for the
> > same, apart from this I have also fixed a) pgindent issues b) perltidy
> > issues c) one variable change
On Tue, Aug 29, 2023 at 10:16 AM vignesh C wrote:
>
> That makes sense. The attached v6 version has the changes for the
> same, apart from this I have also fixed a) pgindent issues b) perltidy
> issues c) one variable change (flush_lsn_changed to
> confirmed_flush_has_changed) d) corrected few
On Mon, 28 Aug 2023 at 18:56, Amit Kapila wrote:
>
> On Thu, Aug 24, 2023 at 11:44 AM vignesh C wrote:
> >
>
> The patch looks mostly good to me. I have made minor changes which are
> as follows: (a) removed the autovacuum =off and
> wal_receiver_status_interval = 0 setting as those doesn't seem
On Thu, Aug 24, 2023 at 11:44 AM vignesh C wrote:
>
The patch looks mostly good to me. I have made minor changes which are
as follows: (a) removed the autovacuum =off and
wal_receiver_status_interval = 0 setting as those doesn't seem to be
required for the test; (b) changed a few comments and
Dear hackers,
I also tested for logical slots on the physical standby. PSA the script.
confirmed_flush_lsn for such slots were successfully persisted.
# Topology
In this test nodes are connected each other.
node1 --(physical replication)-->node2--(logical replication)-->node3
# Test method
On Fri, 25 Aug 2023 at 17:40, vignesh C wrote:
>
> On Sat, 19 Aug 2023 at 11:53, Amit Kapila wrote:
> >
> > It's entirely possible for a logical slot to have a confirmed_flush
> > LSN higher than the last value saved on disk while not being marked as
> > dirty. It's currently not a problem to
On Sat, 19 Aug 2023 at 11:53, Amit Kapila wrote:
>
> It's entirely possible for a logical slot to have a confirmed_flush
> LSN higher than the last value saved on disk while not being marked as
> dirty. It's currently not a problem to lose that value during a clean
> shutdown / restart cycle but
On Wed, 23 Aug 2023 at 14:21, Hayato Kuroda (Fujitsu)
wrote:
>
> Dear Vignesh,
>
> > Here is a patch to persist to disk logical slots during a shutdown
> > checkpoint if the updated confirmed_flush_lsn has not yet been
> > persisted.
>
> Thanks for making the patch with different approach! Here
On Wed, 23 Aug 2023 at 14:21, Hayato Kuroda (Fujitsu)
wrote:
>
> Dear Vignesh,
>
> > Here is a patch to persist to disk logical slots during a shutdown
> > checkpoint if the updated confirmed_flush_lsn has not yet been
> > persisted.
>
> Thanks for making the patch with different approach! Here
Dear Vignesh,
> Here is a patch to persist to disk logical slots during a shutdown
> checkpoint if the updated confirmed_flush_lsn has not yet been
> persisted.
Thanks for making the patch with different approach! Here are comments.
01. RestoreSlotFromDisk
```
On Tue, 22 Aug 2023 at 15:42, Amit Kapila wrote:
>
> On Tue, Aug 22, 2023 at 2:56 PM Ashutosh Bapat
> wrote:
> >
> > On Tue, Aug 22, 2023 at 9:48 AM Amit Kapila wrote:
> > > >
> > > > Another idea is to record the confirm_flush_lsn at the time of
> > > > persisting the slot. We can use it in
Dear hackers,
Thanks for forking the thread! I think we would choose another design, but I
wanted
to post the updated version once with the current approach. All comments came
from the parent thread [1].
> 1. GENERAL -- git apply
>
> The patch fails to apply cleanly. There are whitespace
On Tue, Aug 22, 2023 at 3:42 PM Amit Kapila wrote:
> >
> > Once we have last_persisted_confirm_flush_lsn, (1) is just an
> > optimization on top of that. With that we take the opportunity to
> > persist confirmed_flush_lsn which is much farther than the current
> > persisted value and thus
On Tue, Aug 22, 2023 at 2:56 PM Ashutosh Bapat
wrote:
>
> On Tue, Aug 22, 2023 at 9:48 AM Amit Kapila wrote:
> > >
> > > Another idea is to record the confirm_flush_lsn at the time of
> > > persisting the slot. We can use it in two different ways 1. to mark a
> > > slot dirty and persist if the
On Tue, Aug 22, 2023 at 9:48 AM Amit Kapila wrote:
> >
> > Another idea is to record the confirm_flush_lsn at the time of
> > persisting the slot. We can use it in two different ways 1. to mark a
> > slot dirty and persist if the last confirm_flush_lsn when slot was
> > persisted was too far from
On Mon, Aug 21, 2023 at 6:36 PM Ashutosh Bapat
wrote:
>
> On Sun, Aug 20, 2023 at 8:40 AM Amit Kapila wrote:
> >
> > The other possibility is that we introduce yet another dirty flag for
> > slots, say dirty_for_shutdown_checkpoint which will be set when we
> > update confirmed_flush LSN. The
On Sun, Aug 20, 2023 at 8:40 AM Amit Kapila wrote:
>
> On Sat, Aug 19, 2023 at 12:46 PM Julien Rouhaud wrote:
> >
> > On Sat, 19 Aug 2023, 14:16 Amit Kapila, wrote:
> >>
> >
> >> The idea discussed in the thread [1] is to always persist logical
> >> slots to disk during the shutdown checkpoint.
On Sat, Aug 19, 2023 at 12:46 PM Julien Rouhaud wrote:
>
> On Sat, 19 Aug 2023, 14:16 Amit Kapila, wrote:
>>
>
>> The idea discussed in the thread [1] is to always persist logical
>> slots to disk during the shutdown checkpoint. I have extracted the
>> patch to achieve the same from that thread
On Sat, 19 Aug 2023, 14:16 Amit Kapila, wrote:
> It's entirely possible for a logical slot to have a confirmed_flush
> LSN higher than the last value saved on disk while not being marked as
> dirty. It's currently not a problem to lose that value during a clean
> shutdown / restart cycle but to
69 matches
Mail list logo
