Rename RECOVERYXLOG to RECOVERYWAL?

Started by David Steeleover 8 years ago5 messages

david@pgmasters.net

over 8 years ago

1 attachment(s)

I searched the various threads on the xlog -> wal rename and I couldn't
find any specific mention of why this was not renamed.

I have attached a patch in case it was an oversight rather than left
as-is on purpose.

Thanks,
--
-David
david@pgmasters.net

Attachments:

recoveryxlog-v1.patchtext/plain; charset=UTF-8; name=recoveryxlog-v1.patch; x-mac-creator=0; x-mac-type=0Download

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index df4843f409..fd7bfa11d1 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -3562,7 +3562,7 @@ XLogFileRead(XLogSegNo segno, int emode, TimeLineID tli,
 			set_ps_display(activitymsg, false);
 
 			restoredFromArchive = RestoreArchivedFile(path, xlogfname,
-													  "RECOVERYXLOG",
+													  "RECOVERYWAL",
 													  XLogSegSize,
 													  InRedo);
 			if (!restoredFromArchive)
@@ -5550,10 +5550,10 @@ exitArchiveRecovery(TimeLineID endTLI, XLogRecPtr endOfLog)
 	XLogArchiveCleanup(xlogfname);
 
 	/*
-	 * Since there might be a partial WAL segment named RECOVERYXLOG, get rid
+	 * Since there might be a partial WAL segment named RECOVERYWAL, get rid
 	 * of it.
 	 */
-	snprintf(recoveryPath, MAXPGPATH, XLOGDIR "/RECOVERYXLOG");
+	snprintf(recoveryPath, MAXPGPATH, XLOGDIR "/RECOVERYWAL");
 	unlink(recoveryPath);		/* ignore any error */
 
 	/* Get rid of any remaining recovered timeline-history file, too */

robertmhaas@gmail.com

over 8 years ago

In reply to: David Steele (#1)

Re: Rename RECOVERYXLOG to RECOVERYWAL?

On Fri, Sep 1, 2017 at 12:57 PM, David Steele <david@pgmasters.net> wrote:

I searched the various threads on the xlog -> wal rename and I couldn't
find any specific mention of why this was not renamed.

I have attached a patch in case it was an oversight rather than left
as-is on purpose.

I don't think this really buys us anything. If we'd applied it to v10
maybe, but what do we get out of whacking it around now?

"Consistency", I hear you cry! Fair point. But we never had a goal
of eliminating all internal references to "xlog", just the user-facing
ones. And since RECOVERYXLOG is not documented, I think there's a
good argument that it's not user-facing. You could argue that since
it shows up in the file system it's implicitly user-facing, and maybe
you're right; if some other committer really wants to make this
change, I won't grouse much. But personally I'd favor leaving it
alone to avoid having the behavior change a little bit in every new
release.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

david@pgmasters.net

over 8 years ago

In reply to: Robert Haas (#2)

Re: Rename RECOVERYXLOG to RECOVERYWAL?

On 9/1/17 2:06 PM, Robert Haas wrote:

On Fri, Sep 1, 2017 at 12:57 PM, David Steele <david@pgmasters.net> wrote:

I searched the various threads on the xlog -> wal rename and I couldn't
find any specific mention of why this was not renamed.

I have attached a patch in case it was an oversight rather than left
as-is on purpose.

I don't think this really buys us anything. If we'd applied it to v10
maybe, but what do we get out of whacking it around now?

I was thinking it would be applied to v10.

"Consistency", I hear you cry! Fair point. But we never had a goal
of eliminating all internal references to "xlog", just the user-facing
ones. And since RECOVERYXLOG is not documented, I think there's a
good argument that it's not user-facing. You could argue that since
it shows up in the file system it's implicitly user-facing, and maybe
you're right;

That's exactly my argument, in fact!

if some other committer really wants to make this
change, I won't grouse much. But personally I'd favor leaving it
alone to avoid having the behavior change a little bit in every new
release.

Seems like since v10 is still beta and this is not really documented it
wouldn't be that big a deal to make the change. If nothing else it
might keep the question from coming up in the future.

I'm not going to make a big fuss about it, though. I noticed it while
testing the v10 support in pgbackRest and thought it was worth bringing up.

Thanks,
--
-David
david@pgmasters.net

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Michael Paquier

michael.paquier@gmail.com

over 8 years ago

In reply to: Robert Haas (#2)

Re: Rename RECOVERYXLOG to RECOVERYWAL?

On Sat, Sep 2, 2017 at 3:06 AM, Robert Haas <robertmhaas@gmail.com> wrote:

I don't think this really buys us anything. If we'd applied it to v10
maybe, but what do we get out of whacking it around now?

"Consistency", I hear you cry! Fair point. But we never had a goal
of eliminating all internal references to "xlog", just the user-facing
ones. And since RECOVERYXLOG is not documented, I think there's a
good argument that it's not user-facing. You could argue that since
it shows up in the file system it's implicitly user-facing, and maybe
you're right; if some other committer really wants to make this
change, I won't grouse much. But personally I'd favor leaving it
alone to avoid having the behavior change a little bit in every new
release.

I may be wrong, but I would suspect that some backup tools doing
FS-level backup are checking on the existence of this file and skip
it. From the point of view of operations, it does not matter much as
any existing RECOVERYXLOG is unlinked before being replaced by a new
one, but that would not be nice to add silently 16MB in each backup.
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

david@pgmasters.net

over 8 years ago

In reply to: Michael Paquier (#4)

Re: Rename RECOVERYXLOG to RECOVERYWAL?

On 9/1/17 7:53 PM, Michael Paquier wrote:

On Sat, Sep 2, 2017 at 3:06 AM, Robert Haas <robertmhaas@gmail.com> wrote:

I don't think this really buys us anything. If we'd applied it to v10
maybe, but what do we get out of whacking it around now?

"Consistency", I hear you cry! Fair point. But we never had a goal
of eliminating all internal references to "xlog", just the user-facing
ones. And since RECOVERYXLOG is not documented, I think there's a
good argument that it's not user-facing. You could argue that since
it shows up in the file system it's implicitly user-facing, and maybe
you're right; if some other committer really wants to make this
change, I won't grouse much. But personally I'd favor leaving it
alone to avoid having the behavior change a little bit in every new
release.

I may be wrong, but I would suspect that some backup tools doing
FS-level backup are checking on the existence of this file and skip
it. From the point of view of operations, it does not matter much as
any existing RECOVERYXLOG is unlinked before being replaced by a new
one, but that would not be nice to add silently 16MB in each backup.

Yes, pgBackRest does have an "offline" mode that can be used (when the
database is shutdown) to do an FS-level backup.

It never occurred to me to exclude RECOVERYXLOG but with 1GB WAL
segments coming in v11 it might be a good idea.

--
-David
david@pgmasters.net

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers