On Wed, 26 Oct 2022 at 02:08, Bharath Rupireddy
wrote:
>
> I'm attaching the v9 patch set herewith after rebasing. Please review
> it further.
It looks like neither reviewer has been really convinced this is the
direction they want to go and I think that's why the thread has been
pretty dead sinc
On Mon, Oct 24, 2022 at 1:00 PM Michael Paquier wrote:
>
> On Wed, Oct 19, 2022 at 09:07:04PM +0530, Bharath Rupireddy wrote:
> > XLogBackupResetRunning() seemed better. +1 for above function names.
>
> I see what you are doing here. XLogCtl would still live in xlog.c,
> but we want to have funct
On Wed, Oct 19, 2022 at 09:07:04PM +0530, Bharath Rupireddy wrote:
> XLogBackupResetRunning() seemed better. +1 for above function names.
I see what you are doing here. XLogCtl would still live in xlog.c,
but we want to have functions that are able to manipulate some of its
fields. I am not sure
On Wed, Oct 19, 2022 at 6:30 PM Alvaro Herrera wrote:
>
> 0001 seems mostly OK, but I don't like some of these new function names.
> I see you've named them so that they are case-consistent with the name
> of the struct member that they affect, but I don't think that's a good
> criterion. I propo
0001 seems mostly OK, but I don't like some of these new function names.
I see you've named them so that they are case-consistent with the name
of the struct member that they affect, but I don't think that's a good
criterion. I propose
SetrunningBackups -> XLogBackupSetRunning()
ResetXLogBackupAc
On Wed, Oct 19, 2022 at 4:26 PM Bharath Rupireddy
wrote:
>
> On Wed, Oct 19, 2022 at 4:23 PM Alvaro Herrera
> wrote:
>
> > Thanks for looking. Pushed now.
>
> Thanks. I will rebase and post the other patches soon.
Please review the attached v7 patch-set.
--
Bharath Rupireddy
PostgreSQL Contr
On Wed, Oct 19, 2022 at 4:23 PM Alvaro Herrera wrote:
>
> On 2022-Oct-19, Bharath Rupireddy wrote:
>
> > When the standby is in recovery, calls to XLogInsertRecord() or
> > AdvanceXLInsertBuffer()) where forcePageWrites is being used, won't
> > happen, no?
> >
> > * Note that forcePageWrites
On 2022-Oct-19, Bharath Rupireddy wrote:
> When the standby is in recovery, calls to XLogInsertRecord() or
> AdvanceXLInsertBuffer()) where forcePageWrites is being used, won't
> happen, no?
>
> * Note that forcePageWrites has no effect during an online backup from
> - * the standby.
>
On Wed, Oct 19, 2022 at 2:30 PM Alvaro Herrera wrote:
>
> Another point before we move on with your 0002 is that forcePageWrites
> is no longer useful and we can remove it, as per the attached.
+1. The following comment enables us to rely on runningBackups and get
rid of forcePageWrites completel
Another point before we move on with your 0002 is that forcePageWrites
is no longer useful and we can remove it, as per the attached.
--
Álvaro HerreraBreisgau, Deutschland — https://www.EnterpriseDB.com/
"No deja de ser humillante para una persona de ingenio saber
que no hay tonto que
On 2022-Oct-14, Bharath Rupireddy wrote:
> On Fri, Oct 14, 2022 at 1:54 PM Alvaro Herrera
> wrote:
> > But alternatively, we could just remove emit_warning as a flag and have
> > the warning be emitted always; then we can use the boolean for the other
> > purpose. I don't think the extra WARNIN
On Fri, Oct 14, 2022 at 1:54 PM Alvaro Herrera wrote:
>
> On 2022-Oct-13, Bharath Rupireddy wrote:
>
> > The pg_backup_start_callback() can just go ahead and reset
> > sessionBackupState. However, it leads us to the complete removal of
> > pg_backup_start_callback() itself and use do_pg_abort_back
On Fri, Oct 14, 2022 at 10:24:41AM +0200, Alvaro Herrera wrote:
> However, what's most problematic about this patch is that it introduces
> a pretty serious bug, yet that bug goes unnoticed if you just run the
> builtin test suites. I only noticed because I added an elog(ERROR,
> "oops") in the ar
On 2022-Oct-13, Bharath Rupireddy wrote:
> The pg_backup_start_callback() can just go ahead and reset
> sessionBackupState. However, it leads us to the complete removal of
> pg_backup_start_callback() itself and use do_pg_abort_backup()
> consistently across, saving 20 LOC attached as v5-0001.
OK
On Thu, Oct 13, 2022 at 4:43 PM Alvaro Herrera wrote:
>
Thanks for reviewing.
> > Hm. Agree. But, that requires us to include xlogbackup.h in
> > xlog_internal.h for SessionBackupState enum in
> > ResetXLogBackupActivity(). Is that okay?
>
> It's not great, but it's not *that* bad, ISTM, mainly
On Thu, Oct 13, 2022 at 7:13 AM Alvaro Herrera wrote:
> On 2022-Oct-13, Bharath Rupireddy wrote:
> > Hm. Agree. But, that requires us to include xlogbackup.h in
> > xlog_internal.h for SessionBackupState enum in
> > ResetXLogBackupActivity(). Is that okay?
>
> It's not great, but it's not *that* b
On 2022-Oct-13, Bharath Rupireddy wrote:
> Hm. Agree. But, that requires us to include xlogbackup.h in
> xlog_internal.h for SessionBackupState enum in
> ResetXLogBackupActivity(). Is that okay?
It's not great, but it's not *that* bad, ISTM, mainly because
xlog_internal.h will affect less stuff t
On Thu, Oct 13, 2022 at 3:12 PM Alvaro Herrera wrote:
>
> As I see it, xlog.h is a header that exports XLOG manipulations to the
> outside world (everything that produces WAL, as well as stuff that
> controls operation); xlog_internal is the header that exports xlog*.c
> internal stuff for other x
On 2022-Oct-13, Bharath Rupireddy wrote:
> On Wed, Oct 12, 2022 at 1:04 PM Alvaro Herrera
> wrote:
> > You added some commentary that these functions are tailor-made for
> > internal operations, and then declared them in the most public header
> > function that xlog has? I think at the bare mi
On Wed, Oct 12, 2022 at 1:04 PM Alvaro Herrera wrote:
>
> > Hm. Here's the v3 patch set without exposing WAL insert lock related
> > functions. Please have a look.
>
> Hmm, I don't like your 0001 very much. This sort of thing:
Thanks for reviewing.
> +ControlFileData *
> +GetControlFile(void)
>
On 2022-Oct-06, Bharath Rupireddy wrote:
> On Thu, Oct 6, 2022 at 4:50 AM Andres Freund wrote:
> >
> > I'm doubtful it's a good idea to expose these to outside of xlog.c - they
> > are
> > very low level, and it's very easy to break stuff by using them wrongly.
>
> Hm. Here's the v3 patch set w
On Thu, Oct 6, 2022 at 4:50 AM Andres Freund wrote:
>
> I'm doubtful it's a good idea to expose these to outside of xlog.c - they are
> very low level, and it's very easy to break stuff by using them wrongly.
Hm. Here's the v3 patch set without exposing WAL insert lock related
functions. Please h
Hi,
On 2022-10-05 15:22:01 +0530, Bharath Rupireddy wrote:
> +extern void WALInsertLockAcquire(void);
> +extern void WALInsertLockAcquireExclusive(void);
> +extern void WALInsertLockRelease(void);
> +extern void WALInsertLockUpdateInsertingAt(XLogRecPtr insertingAt);
>
> Note that I had moved all
On Wed, Oct 05, 2022 at 03:22:01PM +0530, Bharath Rupireddy wrote:
>> On Tue, Oct 04, 2022 at 03:54:20PM -0700, Nathan Bossart wrote:
>> > I would suggest moving this to a separate prerequisite patch that can be
>> > reviewed independently from the patches that simply move code to a
>> > different
On 2022-Oct-05, Michael Paquier wrote:
> And FWIW, the SQL interfaces for pg_backup_start() and
> pg_backup_stop() could stay in xlogfuncs.c. This has the advantage to
> centralize in the same file all the SQL-function-specific checks.
As I recall, that has the disadvantage that the API exposure
On Wed, Oct 5, 2022 at 1:20 PM Michael Paquier wrote:
>
> On Tue, Oct 04, 2022 at 03:54:20PM -0700, Nathan Bossart wrote:
> > I would suggest moving this to a separate prerequisite patch that can be
> > reviewed independently from the patches that simply move code to a
> > different file.
I added
On Tue, Oct 04, 2022 at 03:54:20PM -0700, Nathan Bossart wrote:
> I would suggest moving this to a separate prerequisite patch that can be
> reviewed independently from the patches that simply move code to a
> different file.
And FWIW, the SQL interfaces for pg_backup_start() and
pg_backup_stop()
On Wed, Sep 28, 2022 at 08:16:08PM +0530, Bharath Rupireddy wrote:
> In doing so, I had to add a few Get/Set functions
> for XLogCtl variables so that xlogbackup.c can use them.
I would suggest moving this to a separate prerequisite patch that can be
reviewed independently from the patches that si
28 matches
Mail list logo
