agora inbox for [email protected]
help / color / mirror / Atom feedpg_waldump command line arguments
7+ messages / 6 participants
[nested] [flat]
* pg_waldump command line arguments
@ 2017-06-15 21:08 Robert Haas <[email protected]>
2017-06-16 08:55 ` Re: pg_waldump command line arguments Ashutosh Bapat <[email protected]>
2017-06-16 17:58 ` Re: pg_waldump command line arguments Andres Freund <[email protected]>
0 siblings, 2 replies; 7+ messages in thread
From: Robert Haas @ 2017-06-15 21:08 UTC (permalink / raw)
To: pgsql-hackers
pg_waldump --help claims that you run it like this:
Usage:
pg_waldump [OPTION]... [STARTSEG [ENDSEG]]
And https://www.postgresql.org/docs/10/static/pgwaldump.html agrees.
Since square brackets indicate optional arguments, this sort of makes
it sound like running pg_waldump with no arguments ought to work. But
it doesn't:
$ pg_waldump
pg_waldump: no arguments specified
Try "pg_waldump --help" for more information.
If we removed the error check that displays "pg_waldump: no arguments
specified", then it would still fail, but with a more useful error
message:
$ pg_waldump --
pg_waldump: no start WAL location given
Try "pg_waldump --help" for more information.
That message ought to perhaps be changed to say that you specified
neither the start WAL location nor the start WAL file, but even as it
stands it's certainly better than "no arguments specified".
Another problem is that if the file name you pass to pg_waldump
doesn't happen to have a name that looks like a WAL file, it fails in
a completely ridiculous fashion:
$ pg_waldump /etc/passwd
pg_waldump: FATAL: could not find file "000000017C55C16F000000FF": No
such file or directory
The problem appears to be that fuzzy_open_file() successfully opens
the file and then invokes XLogFromFileName() on the filename.
XLogFromFileName() calls sscanf() on the file name without any error
checking, which I think results in leaving private.timeline
uninitialized and setting segno to whatever preexisting garbage was in
the log and segno variables declared inside XLogFromFileName(),
resulting in an attempt to find a more or less completely random file.
A slightly broader concern is whether we need to require the start
position at all. It seems like one could locate the WAL directory
using the existing logic, then search for the earliest file. It might
be a little unclear what "earliest" means when multiple timelines are
present, but I bet we could come up with some behavior that would be
convenient for most users. It would be quite handy to be able to run
this without arguments (or just with -z) and have it process all the
WAL files that you've got on hand.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: pg_waldump command line arguments
2017-06-15 21:08 pg_waldump command line arguments Robert Haas <[email protected]>
@ 2017-06-16 08:55 ` Ashutosh Bapat <[email protected]>
1 sibling, 0 replies; 7+ messages in thread
From: Ashutosh Bapat @ 2017-06-16 08:55 UTC (permalink / raw)
To: Robert Haas <[email protected]>; +Cc: pgsql-hackers
On Fri, Jun 16, 2017 at 2:38 AM, Robert Haas <[email protected]> wrote:
>
> A slightly broader concern is whether we need to require the start
> position at all. It seems like one could locate the WAL directory
> using the existing logic, then search for the earliest file. It might
> be a little unclear what "earliest" means when multiple timelines are
> present, but I bet we could come up with some behavior that would be
> convenient for most users.
We already have some default behaviour defined
--
-t timeline
--timeline=timeline
Timeline from which to read log records. The default is to use the
value in startseg, if that is specified; otherwise, the default is 1.
--
So, if startseg is not provided, choose the earliest file in the
default timeline (given by -t 1 when specified).
> It would be quite handy to be able to run
> this without arguments (or just with -z) and have it process all the
> WAL files that you've got on hand.
>
+1.
--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: pg_waldump command line arguments
2017-06-15 21:08 pg_waldump command line arguments Robert Haas <[email protected]>
@ 2017-06-16 17:58 ` Andres Freund <[email protected]>
1 sibling, 0 replies; 7+ messages in thread
From: Andres Freund @ 2017-06-16 17:58 UTC (permalink / raw)
To: Robert Haas <[email protected]>; +Cc: pgsql-hackers
On 2017-06-15 17:08:23 -0400, Robert Haas wrote:
> pg_waldump --help claims that you run it like this:
>
> Usage:
> pg_waldump [OPTION]... [STARTSEG [ENDSEG]]
>
> And https://www.postgresql.org/docs/10/static/pgwaldump.html agrees.
> Since square brackets indicate optional arguments, this sort of makes
> it sound like running pg_waldump with no arguments ought to work. But
> it doesn't:
Well, not really, it indicates that positional arguments are allowed,
but not required. You can get by with with -s / -e, which are sometimes
important, if you want to look at multiple timelines etc.
> A slightly broader concern is whether we need to require the start
> position at all. It seems like one could locate the WAL directory
> using the existing logic, then search for the earliest file.
"earliest file" isn't actually that trivial to determine if there's
timelines etc. But leaving that aside, it'll be frequently so much data
that'll be output, that it'd make the output pretty much useless, no? I
think if we were to add a bit more magic, it'd make more sense to parse
pg_control and start at the last flushed point nof WAL forward,
especially with -f.
> It might be a little unclear what "earliest" means when multiple
> timelines are present, but I bet we could come up with some behavior
> that would be convenient for most users. It would be quite handy to
> be able to run this without arguments (or just with -z) and have it
> process all the WAL files that you've got on hand.
With -z I agree, probably best by parsing pg_control and parsing
[checkpoint - 1, minRecoveryPoint) or such.
I'm willing to review some patches here, but I don't plan to personally
work on patches around this...
Greetings,
Andres Freund
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 7+ messages in thread
* [PATCH] Let complete segment archived immediately on standy
@ 2020-06-29 07:12 Kyotaro Horiguchi <[email protected]>
0 siblings, 0 replies; 7+ messages in thread
From: Kyotaro Horiguchi @ 2020-06-29 07:12 UTC (permalink / raw)
walreceiver marks a completed segment as ".ready" after any data for
the next segment comes. So standby can archive a WAL segment later
than the primary archives the same segment. Let walreceiver archive a
segment as soon as it is completed.
---
src/backend/replication/walreceiver.c | 77 +++++++++++++++------------
1 file changed, 42 insertions(+), 35 deletions(-)
diff --git a/src/backend/replication/walreceiver.c b/src/backend/replication/walreceiver.c
index d1ad75da87..06c1e3cbe4 100644
--- a/src/backend/replication/walreceiver.c
+++ b/src/backend/replication/walreceiver.c
@@ -902,44 +902,10 @@ XLogWalRcvWrite(char *buf, Size nbytes, XLogRecPtr recptr)
{
int segbytes;
- if (recvFile < 0 || !XLByteInSeg(recptr, recvSegNo, wal_segment_size))
+ if (recvFile < 0)
{
bool use_existent;
- /*
- * fsync() and close current file before we switch to next one. We
- * would otherwise have to reopen this file to fsync it later
- */
- if (recvFile >= 0)
- {
- char xlogfname[MAXFNAMELEN];
-
- XLogWalRcvFlush(false);
-
- XLogFileName(xlogfname, recvFileTLI, recvSegNo, wal_segment_size);
-
- /*
- * XLOG segment files will be re-read by recovery in startup
- * process soon, so we don't advise the OS to release cache
- * pages associated with the file like XLogFileClose() does.
- */
- if (close(recvFile) != 0)
- ereport(PANIC,
- (errcode_for_file_access(),
- errmsg("could not close log segment %s: %m",
- xlogfname)));
-
- /*
- * Create .done file forcibly to prevent the streamed segment
- * from being archived later.
- */
- if (XLogArchiveMode != ARCHIVE_MODE_ALWAYS)
- XLogArchiveForceDone(xlogfname);
- else
- XLogArchiveNotify(xlogfname);
- }
- recvFile = -1;
-
/* Create/use new log file */
XLByteToSeg(recptr, recvSegNo, wal_segment_size);
use_existent = true;
@@ -985,6 +951,47 @@ XLogWalRcvWrite(char *buf, Size nbytes, XLogRecPtr recptr)
buf += byteswritten;
LogstreamResult.Write = recptr;
+
+ /*
+ * Close the current WAL segment if it is completed then let the file
+ * be archived if needed.
+ */
+ if (!XLByteInSeg(recptr, recvSegNo, wal_segment_size))
+ {
+ char xlogfname[MAXFNAMELEN];
+
+ Assert (recvFile >= 0);
+
+ /*
+ * fsync() and close current file before we switch to next one. We
+ * would otherwise have to reopen this file to fsync it later
+ */
+ XLogWalRcvFlush(false);
+
+ XLogFileName(xlogfname, recvFileTLI, recvSegNo, wal_segment_size);
+
+ /*
+ * XLOG segment files will be re-read by recovery in startup
+ * process soon, so we don't advise the OS to release cache
+ * pages associated with the file like XLogFileClose() does.
+ */
+ if (close(recvFile) != 0)
+ ereport(PANIC,
+ (errcode_for_file_access(),
+ errmsg("could not close log segment %s: %m",
+ xlogfname)));
+
+ /*
+ * Create .done file forcibly to prevent the streamed segment
+ * from being archived later.
+ */
+ if (XLogArchiveMode != ARCHIVE_MODE_ALWAYS)
+ XLogArchiveForceDone(xlogfname);
+ else
+ XLogArchiveNotify(xlogfname);
+
+ recvFile = -1;
+ }
}
/* Update shared-memory status */
--
2.18.4
----Next_Part(Mon_Jun_29_16_41_11_2020_855)----
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Adding REPACK [concurrently]
@ 2026-02-15 16:03 Mihail Nikalayeu <[email protected]>
2026-02-16 07:47 ` Re: Adding REPACK [concurrently] Antonin Houska <[email protected]>
0 siblings, 1 reply; 7+ messages in thread
From: Mihail Nikalayeu @ 2026-02-15 16:03 UTC (permalink / raw)
To: Alvaro Herrera <[email protected]>; +Cc: Antonin Houska <[email protected]>; Pg Hackers <[email protected]>; Robert Treat <[email protected]>
Hello!
Some feedback for v33.
> else if (pg_strcasecmp(cmd, "REPACK") == 0)
> cmdtype = PROGRESS_COMMAND_REPACK;
src/backend/utils/adt/pgstatfuncs.c:290
I think we need to add "CLUSTER" here too to avoid regression.
-----------
> ConditionVariablePrepareToSleep(&shared->cv);
> for (;;)
> {
> bool initialized;
>
> SpinLockAcquire(&shared->mutex);
> initialized = shared->initialized;
> SpinLockRelease(&shared->mutex);
src/backend/commands/cluster.c:3663
I think we should check GetBackgroundWorkerPid for worker status, to
not wait forever in case of some issue with the worker.
-----------
> /* Error queue. */
> shm_mq *error_mq;
src/backend/commands/cluster.c:210.
Not used anywhere.
-----------
> finish_heap_swap(old_table_oid, new_table_oid,
> is_system_catalog,
> false, /* swap_toast_by_content */
> false, true, false,
> frozenXid, cutoffMulti,
> relpersistence);
src/backend/commands/cluster.c
I think we should add comments for other boolean parameters.
-----------
> elog(ERROR, "Incomplete insert info.");
> elog(ERROR, "Incomplete update info.");
src/backend/replication/pgoutput_repack/pgoutput_repack.c:118,132
Should be non-capitalized?
-----------
> # Copyright (c) 2022-2024, PostgreSQL Global Development Group
src/backend/replication/pgoutput_repack/meson.build
2022-2026
-----------
> int newxcnt = 0;
src/backend/replication/logical/snapbuild.c:53
uint32 is better here.
Best regards,
Mikhail.
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Adding REPACK [concurrently]
2026-02-15 16:03 Re: Adding REPACK [concurrently] Mihail Nikalayeu <[email protected]>
@ 2026-02-16 07:47 ` Antonin Houska <[email protected]>
2026-02-16 11:16 ` Re: Adding REPACK [concurrently] Mihail Nikalayeu <[email protected]>
0 siblings, 1 reply; 7+ messages in thread
From: Antonin Houska @ 2026-02-16 07:47 UTC (permalink / raw)
To: Mihail Nikalayeu <[email protected]>; +Cc: Alvaro Herrera <[email protected]>; Pg Hackers <[email protected]>; Robert Treat <[email protected]>
Mihail Nikalayeu <[email protected]> wrote:
> Some feedback for v33.
Thanks again for your review!
> > else if (pg_strcasecmp(cmd, "REPACK") == 0)
> > cmdtype = PROGRESS_COMMAND_REPACK;
> src/backend/utils/adt/pgstatfuncs.c:290
>
> I think we need to add "CLUSTER" here too to avoid regression.
What kind of regression? There is no pg_stat_get_progress_info('CLUSTER') call
in system_views.sql.
> -----------
>
> > ConditionVariablePrepareToSleep(&shared->cv);
> > for (;;)
> > {
> > bool initialized;
> >
> > SpinLockAcquire(&shared->mutex);
> > initialized = shared->initialized;
> > SpinLockRelease(&shared->mutex);
> src/backend/commands/cluster.c:3663
>
> I think we should check GetBackgroundWorkerPid for worker status, to
> not wait forever in case of some issue with the worker.
ConditionVariableSleep() calls CHECK_FOR_INTERRUPTS(). That should process
error messages from the worker.
> -----------
>
> > /* Error queue. */
> > shm_mq *error_mq;
> src/backend/commands/cluster.c:210.
>
> Not used anywhere.
ok
> -----------
>
> > finish_heap_swap(old_table_oid, new_table_oid,
> > is_system_catalog,
> > false, /* swap_toast_by_content */
> > false, true, false,
> > frozenXid, cutoffMulti,
> > relpersistence);
> src/backend/commands/cluster.c
>
> I think we should add comments for other boolean parameters.
Perhaps, it wouldn't hurt.
> -----------
>
> > elog(ERROR, "Incomplete insert info.");
> > elog(ERROR, "Incomplete update info.");
> src/backend/replication/pgoutput_repack/pgoutput_repack.c:118,132
>
> Should be non-capitalized?
ok
> -----------
>
> > # Copyright (c) 2022-2024, PostgreSQL Global Development Group
> src/backend/replication/pgoutput_repack/meson.build
>
> 2022-2026
ok
> -----------
>
> > int newxcnt = 0;
> src/backend/replication/logical/snapbuild.c:53
>
> uint32 is better here.
This was already discussed:
https://www.postgresql.org/message-id/137668.1768235610%40localhost
--
Antonin Houska
Web: https://www.cybertec-postgresql.com
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Adding REPACK [concurrently]
2026-02-15 16:03 Re: Adding REPACK [concurrently] Mihail Nikalayeu <[email protected]>
2026-02-16 07:47 ` Re: Adding REPACK [concurrently] Antonin Houska <[email protected]>
@ 2026-02-16 11:16 ` Mihail Nikalayeu <[email protected]>
0 siblings, 0 replies; 7+ messages in thread
From: Mihail Nikalayeu @ 2026-02-16 11:16 UTC (permalink / raw)
To: Antonin Houska <[email protected]>; +Cc: Alvaro Herrera <[email protected]>; Pg Hackers <[email protected]>; Robert Treat <[email protected]>
Hello!
> What kind of regression? There is no pg_stat_get_progress_info('CLUSTER') call
> in system_views.sql.
I am about a direct call to that function (some monitoring tools may use it).
^ permalink raw reply [nested|flat] 7+ messages in thread
end of thread, other threads:[~2026-02-16 11:16 UTC | newest]
Thread overview: 7+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2017-06-15 21:08 pg_waldump command line arguments Robert Haas <[email protected]>
2017-06-16 08:55 ` Ashutosh Bapat <[email protected]>
2017-06-16 17:58 ` Andres Freund <[email protected]>
2020-06-29 07:12 [PATCH] Let complete segment archived immediately on standy Kyotaro Horiguchi <[email protected]>
2026-02-15 16:03 Re: Adding REPACK [concurrently] Mihail Nikalayeu <[email protected]>
2026-02-16 07:47 ` Re: Adding REPACK [concurrently] Antonin Houska <[email protected]>
2026-02-16 11:16 ` Re: Adding REPACK [concurrently] Mihail Nikalayeu <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox