Re: [HACKERS] pg_resetsysid

2015-07-18 Thread Petr Jelinek

On 2015-07-18 02:29, Peter Eisentraut wrote:

On 6/14/15 11:29 AM, Petr Jelinek wrote:

0002 - Adds pg_resetsysid utility which changes the system id to newly
generated one.

0003 - Adds -s option to pg_resetxlog to change the system id to the one
specified - this is separate from the other one as it can be potentially
more dangerous.


Adding a new top-level binary creates a lot of maintenance overhead
(e.g., documentation, man page, translations, packaging), and it seems
silly to create a new tool whose only purpose is to change one number in
one file.  If we're going to have this in pg_resetxlog anyway, why not
create another option letter to assigns a generated ID?  As the
documentation says, resetting the system ID clears the WAL, so it's not
like this is a completely danger-free situation.



Well, last time I submitted this I did it exactly as you propose but 
there was long discussion about this changing the target audience of 
pg_resetxlog and that it would be better as separate binary from 
pg_resetxlog.


It might more future proof to have more generic binary which can do all 
the less dangerous work that pg_resetxlog does (which currently is 
probably only -c and the newly proposed -s). Something like 
pg_setcontroldata but that's too long.


--
 Petr Jelinek  http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training  Services


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


Re: [HACKERS] pg_resetsysid

2015-07-18 Thread Peter Eisentraut
On 7/18/15 9:42 AM, Petr Jelinek wrote:
 Well, last time I submitted this I did it exactly as you propose but
 there was long discussion about this changing the target audience of
 pg_resetxlog and that it would be better as separate binary from
 pg_resetxlog.

In my reading of the thread, I did not get the sense that that was the
consensus.  There were certainly a lot of different opinions, but
specifically some people ended up withdrawing their objections to using
pg_resetxlog.

 It might more future proof to have more generic binary which can do all
 the less dangerous work that pg_resetxlog does (which currently is
 probably only -c and the newly proposed -s).

I don't buy the more or less dangerous argument.  Many tools can be
dangerous.  cp can be dangerous if you overwrite the wrong file.
pg_restore can be dangerous if you give it the wrong options.  Changing
the system ID is also dangerous, as it can break replication and
truncate the WAL.

Right now, changing the system ID is an obscure step in some obscure
workflow related to some obscure feature.  That is not to say it's
invalid, but it's not something that we can present to a normal user as
the official workflow.  Just adding little tools of the nature whack
this around until it's in the right shape for this other thing is just
adding complications on top of complications.  If we want to turn this
into a less dangerous and more user-facing feature, I would like to
see a complete workflow of how this would be used.  Maybe we'll come up
with a better solution.  For example, why couldn't pg_basebackup take
care of this?

 Something like pg_setcontroldata but that's too long. 

Well, there is nothing so far saying that pg_controldata couldn't also
write to pg_control.  It's not called pg_getcontroldata. ;-)




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


Re: [HACKERS] pg_resetsysid

2015-07-17 Thread Peter Eisentraut
On 6/14/15 11:29 AM, Petr Jelinek wrote:
 0002 - Adds pg_resetsysid utility which changes the system id to newly
 generated one.
 
 0003 - Adds -s option to pg_resetxlog to change the system id to the one
 specified - this is separate from the other one as it can be potentially
 more dangerous.

Adding a new top-level binary creates a lot of maintenance overhead
(e.g., documentation, man page, translations, packaging), and it seems
silly to create a new tool whose only purpose is to change one number in
one file.  If we're going to have this in pg_resetxlog anyway, why not
create another option letter to assigns a generated ID?  As the
documentation says, resetting the system ID clears the WAL, so it's not
like this is a completely danger-free situation.



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


[HACKERS] pg_resetsysid

2015-06-14 Thread Petr Jelinek

Hi,

This is an attempt to revive the resetsysid tool submission that didn't 
go anywhere last year [1].


In that thread there were mainly two complains, one that pg_resetxlog 
would preferably be tool for more advanced stuff and similar utilities 
that are not as dangerous should have separate binaries using some of 
the functionality of pg_resetxlog as library and the other was with the 
string to uint64 parsing which is apparently not so easy to do portably. 
About the second one there was also separate discussion between Andres 
and Tom [2].


Here is series of patches that implements both the original feature and 
solves the issues mentioned above.


0001 - This patch splits the code of pg_resetxlog to common.c/h api and 
pg_resetxlog.c itself which does the program logic using the api. The 
only logical change in the XLogSegNoOffsetToRecPtr call is moved into 
main() function, which I think is ok as the main() does all the other 
ControlFile changes. All other changes should be just cosmetic in order 
to not rely on global static variables.


0002 - Adds pg_resetsysid utility which changes the system id to newly 
generated one.


0003 - Adds -s option to pg_resetxlog to change the system id to the one 
specified - this is separate from the other one as it can be potentially 
more dangerous. It also adds some defines that try to portably map 
pg_strtoint64 and pg_strtouint64 to the underlying implementations 
(strtol/strtoll/_strtoi64 and unsigned variants of those). It will leave 
the pg_strtoint64/pg_strtouint64 undefined if the platform does not 
support the functionality. Tom was unsure if there are such platforms on 
our supported list anymore.


Which is why I also added 0004, that adds fall-back implementation of 
the two functions based on very slightly modified code from OpenBSD. 
This is here mostly in case buildfarm gets red after applying 0003 or we 
have reports from users that things got broken.


[1] http://www.postgresql.org/message-id/539b97fc.8040...@2ndquadrant.com
[2] 
http://www.postgresql.org/message-id/20140603144654.gm24...@awork2.anarazel.de


--
 Petr Jelinek  http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training  Services
From f587fd47c9e43f5c08cf840978ff21b64f39d24d Mon Sep 17 00:00:00 2001
From: Petr Jelinek pjmo...@pjmodos.net
Date: Sun, 14 Jun 2015 16:23:39 +0200
Subject: [PATCH 4/4] Add fallback implementations of pg_strto(u)int64.

---
 src/include/c.h  |   6 ++
 src/port/Makefile|   2 +-
 src/port/pgstrtoint64.c  | 169 +++
 src/port/pgstrtouint64.c | 129 
 4 files changed, 305 insertions(+), 1 deletion(-)
 create mode 100644 src/port/pgstrtoint64.c
 create mode 100644 src/port/pgstrtouint64.c

diff --git a/src/include/c.h b/src/include/c.h
index 537b4d0..a01b455 100644
--- a/src/include/c.h
+++ b/src/include/c.h
@@ -1102,6 +1102,9 @@ extern int	fdatasync(int fildes);
 #define pg_strtoint64 strtoll
 #elif defined(WIN32)
 #define pg_strtoint64 _strtoi64
+#else
+extern int64 pg_strtoint64(const char *nptr, char **endptr, int base);
+#define PGSTRTOINT64_IMPL 1
 #endif
 
 /* Define portable pg_strtouint64() */
@@ -,6 +1114,9 @@ extern int	fdatasync(int fildes);
 #define pg_strtouint64 strtoull
 #elif defined(WIN32)
 #define pg_strtouint64 _strtoui64
+#else
+extern uint64 pg_strtouint64(const char *nptr, char **endptr, int base);
+#define PGSTRTOUINT64_IMPL 1
 #endif
 
 /*
diff --git a/src/port/Makefile b/src/port/Makefile
index bc9b63a..91e4a2b 100644
--- a/src/port/Makefile
+++ b/src/port/Makefile
@@ -32,7 +32,7 @@ LIBS += $(PTHREAD_LIBS)
 
 OBJS = $(LIBOBJS) $(PG_CRC32C_OBJS) chklocale.o erand48.o inet_net_ntop.o \
 	noblock.o path.o pgcheckdir.o pgmkdirp.o pgsleep.o \
-	pgstrcasecmp.o pqsignal.o \
+	pgstrcasecmp.o pgstrtoint64.o pgstrtouint64.o pqsignal.o \
 	qsort.o qsort_arg.o quotes.o sprompt.o tar.o thread.o
 
 # foo_srv.o and foo.o are both built from foo.c, but only foo.o has -DFRONTEND
diff --git a/src/port/pgstrtoint64.c b/src/port/pgstrtoint64.c
new file mode 100644
index 000..e968df0
--- /dev/null
+++ b/src/port/pgstrtoint64.c
@@ -0,0 +1,169 @@
+/*-
+ *
+ * pgstrtoint64.c
+ *
+ * Portions Copyright (c) 1996-2014, PostgreSQL Global Development Group
+ *
+ * IDENTIFICATION
+ *	  src/port/pgstrtoint64.c
+ *
+ * Based on OpenBSD strtoll() implementation.
+ * Differences from vanilla implementaion:
+ *  - renamed to pg_strtoint64
+ *  - returns int64 instead of long long
+ *  - internal params defined as int64 instead of long long
+ *  - uses PG_INT64_MIN/PG_INT64_MAX
+ *
+ * The OpenBSD copyright terms follow.
+ */
+
+/* $OpenBSD: strtoll.c,v 1.7 2013/03/28 18:09:38 martynas Exp $ */
+/*-
+ * Copyright (c) 1992 The Regents of the University of California.
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or