From: | Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> |
---|---|
To: | Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> |
Cc: | Andres Freund <andres(at)anarazel(dot)de>, Michael Paquier <michael(at)paquier(dot)xyz>, Nathan Bossart <nathandbossart(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: Move backup-related code to xlogbackup.c/.h |
Date: | 2022-10-19 13:00:15 |
Message-ID: | 20221019130015.ilmzn4fonudfsurm@alvherre.pgsql |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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()
ResetXLogBackupActivity -> XLogBackupNotRunning()
// or maybe SetNotRunning, or ResetRunning? I prefer the one above
SetlastBackupStart -> XLogBackupSetLastStart()
GetlastFpwDisableRecPtr -> XLogGetLastFPWDisableRecptr()
GetminRecoveryPoint -> XLogGetMinRecoveryPoint()
I wouldn't say in the xlog_internal.h comment that these new functions
are for xlogbackup.c to use. The API definition doesn't have to concern
itself with that. Maybe one day xlogrecovery.c or some other xlog*.c
would like to call those functions, and then the comment becomes a lie;
and what for?
0002 is where the interesting stuff happens. I have not reviewed that
part with any care, but it appears that set_backup_state is pretty much
useless. Let's get rid of it instead of moving it. Which also means
that we shouldn't introduce reset_backup_status in 0001, I suppose.
I think xlogfuncs.c is content with having just get_backup_status().
Speaking of which -- I'm not sure we really want to do 0003.
xlogfuncs.c is not a big file, the functions are not complex, and there
are no interesting interactions in those functions with the internals
(other than get_backup_status). I see that Michael advised the same.
I propose we keep those functions where they are.
--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
"I suspect most samba developers are already technically insane...
Of course, since many of them are Australians, you can't tell." (L. Torvalds)
From | Date | Subject | |
---|---|---|---|
Next Message | Robert Haas | 2022-10-19 13:57:03 | Re: allowing for control over SET ROLE |
Previous Message | kuroda.hayato@fujitsu.com | 2022-10-19 12:49:52 | RE: Perform streaming logical transactions by background workers and parallel apply |