Re: Move backup-related code to xlogbackup.c/.h

On Thu, Oct 13, 2022 at 3:12 PM Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> 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 xlog*.c files and specialized frontends to use.
> These new functions are internal to xlogbackup.c and xlog.c, so IMO they
> belong in xlog_internal.h.
>
> Stuff that is used from xlog.c only by xlogrecovery.c should also appear
> in xlog_internal.h only, not xlog.h, so I suggest not to take that as
> precedent. Also, that file (xlogrecovery.c) is pretty new so we haven't
> had time to nail down the .h layout yet.

Hm. Agree. But, that requires us to include xlogbackup.h in
xlog_internal.h for SessionBackupState enum in
ResetXLogBackupActivity(). Is that okay?

SessionBackupState and it needs to be set before we release WAL insert
locks, see the comment [1]. Should we just remove the
SessionBackupState enum and convert SESSION_BACKUP_NONE and
SESSION_BACKUP_RUNNING to just macros in xlogbackup.h and use integer
type to pass the state across? I don't know what's better here. Do you
have any thoughts on this?

[1]
* You might think that WALInsertLockRelease() can be called before
* cleaning up session-level lock because session-level lock doesn't need
* to be protected with WAL insertion lock. But since
* CHECK_FOR_INTERRUPTS() can occur in it, session-level lock must be
* cleaned up before it.
*/

--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com