On Fri, Oct 14, 2022 at 12:10:18PM +0530, Bharath Rupireddy wrote:
> + ereport(ERROR,
> + (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
> + errmsg("both archive_command and archive_library
> specified"),
> + errdetail("Only one of archive_command,
> archive_library may be set.")));
>
> The above errmsg looks informational. Can we just say something like
> below? It doesn't require errdetail as the errmsg says it all. See
> the other instances elsewhere [2].
>
> ereport(ERROR,
> (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
> errmsg("cannot specify both \"archive_command\" and
> \"archive_library\"")));
I modeled this after the ERROR that error_multiple_recovery_targets()
emits. I don't think there's really any material difference between your
proposal and mine, but I don't have a strong opinion.
> 2) I think we have a problem - set archive_mode and archive_library
> and start the server, then set archive_command, reload the conf, see
> [3] - the archiver needs to error out right? The archiver gets
> restarted whenever archive_library changes but not when
> archive_command changes. I think the right place for the error is
> after or at the end of HandlePgArchInterrupts().
Good catch. You are right, this is broken. I believe that we need to
check for the misconfiguration in HandlePgArchInterrupts() in addition to
LoadArchiveLibrary(). I will work on fixing this.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
