Re: [HACKERS] Set new system identifier using pg_resetxlog

2014-08-27 Thread Robert Haas 
On Mon, Aug 25, 2014 at 4:06 PM, Heikki Linnakangas
hlinnakan...@vmware.com wrote:
 I didn't understand this one. But it seems like the obvious solution is to
 not use the consumer's system identifier as the slot name. Or rename it
 afterwards.

You can't use the consumer's system identifier as the slot name,
because you have to create the slot before you create the consumer.
But you could rename it afterwards, or just use some other naming
convention entirely, which is why I'm -0.25 on this whole proposal.
What the 2ndQuadrant folks are proposing is not unreasonable (which is
why I'm only -0.25) but it opens an (admittedly small) can of worms
that I see no real need to open.

-- 
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

Re: [HACKERS] Set new system identifier using pg_resetxlog

2014-08-25 Thread Heikki Linnakangas 

On 06/18/2014 09:17 PM, Josh Berkus wrote:

On 06/18/2014 11:03 AM, Andres Freund wrote:

Well, all those actually do write to the xlog (to write a new
checkpoint, containing the updated control file). Since pg_resetxlog has
done all this pretty much since forever renaming it now seems to be a
big hassle for users for pretty much no benefit? This isn't a tool the
average user should ever touch.


If we're using it to create a unique system ID which can be used to
orchestrate replication and clustering systems, a lot more people are
going to be touching it than ever did before -- and not just for BDR.


I think pg_resetxlog is still appropriate: changing the system ID will 
reset the WAL. In particular, any archived WAL will become useless.


But yeah, this does change the target audience of pg_resetxlog 
significantly. Now that we'll have admins running pg_resetxlog as part 
of normal operations, we have to document it carefully. We also have to 
design the user interface carefully, to make it more clear that while 
resetting the system ID won't eat your data, some of the other settings 
will.


The proposed pg_resetxlog --help output looks like this:


pg_resetxlog resets the PostgreSQL transaction log.

Usage:
  pg_resetxlog [OPTION]... DATADIR

Options:
  -e XIDEPOCH  set next transaction ID epoch
  -f   force update to be done
  -l XLOGFILE  force minimum WAL starting location for new transaction log
  -m MXID,MXID set next and oldest multitransaction ID
  -n   no update, just show what would be done (for testing)
  -o OID   set next OID
  -O OFFSETset next multitransaction offset
  -s [SYSID]   set system identifier (or generate one)
  -V, --versionoutput version information, then exit
  -x XID   set next transaction ID
  -?, --help   show this help, then exit

Report bugs to pgsql-b...@postgresql.org.


I don't think that's good enough. The -s option should be displayed more 
prominently, and the dangerous options like -l and -x should be more 
clearly labeled as such.


I would de-emphasize setting the system ID to a particular value. It 
might be useful for disaster recovery, like -x, but in general you 
should always reset it to a new unique value. If you make it too easy to 
set it to a particular value, someone will try initialize a streaming 
standby server using initdb+pg_dump, and changing the system ID to match 
the master's.


The user manual for pg_resetxlog says:


pg_resetxlog clears the write-ahead log (WAL) and optionally resets
some other control information stored in the pg_control file. This
function is sometimes needed if these files have become corrupted. It
should be used only as a last resort, when the server will not start
due to such corruption.


That's clearly not true for the -s option.

In summary, I think we want this feature in some form, but we'll somehow 
need to be make the distinction to the dangerous pg_resetxlog usage. It 
might be best, after all, to make this a separate utility, 
pg_resetsystemid. It would not need to have the capability to set the 
system ID to a particular value, only a randomly assigned one (setting 
it to a particular value could be added to pg_resetxlog, where other 
dangerous options are).


- Heikki


--
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] Set new system identifier using pg_resetxlog

2014-08-25 Thread Tom Lane 
Heikki Linnakangas hlinnakan...@vmware.com writes:
 In summary, I think we want this feature in some form, but we'll somehow 
 need to be make the distinction to the dangerous pg_resetxlog usage. It 
 might be best, after all, to make this a separate utility, 
 pg_resetsystemid.

That sounds fairly reasonable given your point about not wanting people to
confuse this with the can-eat-your-data aspects of pg_resetxlog.  (OTOH,
won't this result in a lot of code duplication?  We'd still need to erase
and refill the WAL area.)

 It would not need to have the capability to set the 
 system ID to a particular value, only a randomly assigned one (setting 
 it to a particular value could be added to pg_resetxlog, where other 
 dangerous options are).

I'm less convinced about that.  While you can shoot yourself in the foot
by assigning the same system ID to two installations that share WAL
archive or something like that, this feels a bit different than the ways
you can shoot yourself in the foot with pg_resetxlog.  If we do what you
say here then I think we'll be right back to the discussion of how to
separate the assign-a-sysID option from pg_resetxlog's other, more
dangerous options.

regards, tom lane


-- 
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] Set new system identifier using pg_resetxlog

2014-08-25 Thread Andres Freund 
On August 25, 2014 9:45:50 PM CEST, Tom Lane t...@sss.pgh.pa.us wrote:
Heikki Linnakangas hlinnakan...@vmware.com writes:
 In summary, I think we want this feature in some form, but we'll
somehow 
 need to be make the distinction to the dangerous pg_resetxlog usage.
It 
 might be best, after all, to make this a separate utility, 
 pg_resetsystemid.

That sounds fairly reasonable given your point about not wanting people
to
confuse this with the can-eat-your-data aspects of pg_resetxlog. 
(OTOH,
won't this result in a lot of code duplication?  We'd still need to
erase
and refill the WAL area.)

Let's move pg-controldata, resetxlog, resetsysid into a common src/bin 
directory. Then we can unify the relevant code between all three.

-- 
Please excuse brevity and formatting - I am writing this on my mobile phone.

Andres Freund  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] Set new system identifier using pg_resetxlog

2014-08-25 Thread Heikki Linnakangas 

On 08/25/2014 10:45 PM, Tom Lane wrote:

Heikki Linnakangas hlinnakan...@vmware.com writes:

It would not need to have the capability to set the
system ID to a particular value, only a randomly assigned one (setting
it to a particular value could be added to pg_resetxlog, where other
dangerous options are).


I'm less convinced about that.  While you can shoot yourself in the foot
by assigning the same system ID to two installations that share WAL
archive or something like that, this feels a bit different than the ways
you can shoot yourself in the foot with pg_resetxlog.  If we do what you
say here then I think we'll be right back to the discussion of how to
separate the assign-a-sysID option from pg_resetxlog's other, more
dangerous options.


I don't see the use case for setting system id to a particular value. 
Andres listed four use cases upthread:



a) Mark a database as not being the same. Currently if you promote two
   databases, e.g. to shard out, they'll continue to have same system
   identifier. Which really sucks, especially because timeline ids will
   often increase synchronously.


Yes, this is the legitimate use case a DBA would use this feature for. 
Resetting the system ID to a random value suffices.



b) For data recovery it's sometimes useful to create a new database
   (with the same catalog state) and replay all WAL. For that you need to
   reset the system identifier. I've done so hacking up resetxlog
   before.


This falls squarely in the dangerous category, and you'll have to 
reset other things than the system ID to make it work. Having the option 
in pg_resetxlog is fine for this.



c) We already allow to set pretty much all aspects of the control file
   via resetxlog - there seems little point of not having the ability to
   change the system identifier.


Ok, but it's not something a regular admin would ever use.


d) In a logical replication scenario one way to identify individual
   nodes is via the system identifier. If you want to convert a
   basebackup into logical standby one sensible way to do so is to
   create a logical replication slots *before* promoting a physical
   backup to guarantee that slot is able to stream out all changes. If
   the slot names contain the consumer's system identifier you need to
   know the new system identifier beforehand.


I didn't understand this one. But it seems like the obvious solution is 
to not use the consumer's system identifier as the slot name. Or rename 
it afterwards.


- Heikki


--
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] Set new system identifier using pg_resetxlog

2014-07-18 Thread Petr Jelinek 

On 18/07/14 00:41, Andres Freund wrote:

On 2014-06-27 00:51:02 +0200, Petr Jelinek wrote:

{
switch (c)
{
@@ -227,6 +229,33 @@ main(int argc, char *argv[])
XLogFromFileName(optarg, minXlogTli, 
minXlogSegNo);
break;

+   case 's':
+   if (optarg)
+   {
+#ifdef HAVE_STRTOULL
+   set_sysid = strtoull(optarg, endptr, 
10);
+#else
+   set_sysid = strtoul(optarg, endptr, 
10);
+#endif


Wouldn't it be better to use sscanf()? That should work with large
inputs across platforms.



Well, sscanf does not do much validation and silently truncates too big 
input (which is why I replaced it with strtoull, original patch did have 
sscanf), also I think the portability of sscanf might not be as good, 
see 9d7ded0f4277f5c0063eca8e871a34e2355a8371 commit.




+   /*
+* Validate input, we use strspn 
because strtoul(l) accepts
+* negative numbers and silently 
converts them to unsigned
+*/
+   if (set_sysid == 0 || errno != 0 ||
+   endptr == optarg || *endptr != 
'\0' ||
+   strspn(optarg, 0123456789) != 
strlen(optarg))
+   {
+   fprintf(stderr, _(%s: invalid 
argument for option -s\n), progname);
+   fprintf(stderr, _(Try \%s --help\ 
for more information.\n), progname);
+   exit(1);
+   }


Maybe: 'invalid argument \%s\ ...'?



Ok


@@ -1087,6 +1147,7 @@ usage(void)
printf(_(  -o OID   set next OID\n));
printf(_(  -O OFFSETset next multitransaction offset\n));
printf(_(  -V, --versionoutput version information, then exit\n));
+   printf(_(  -s [SYSID]   set system identifier (or generate 
one)\n));
printf(_(  -x XID   set next transaction ID\n));
printf(_(  -?, --help   show this help, then exit\n));
printf(_(\nReport bugs to pgsql-b...@postgresql.org.\n));


I think we usually try to keep the options roughly alphabetically
ordered. Same in the getopt() above.



Ok, attached v4 with those changes.


--
 Petr Jelinek  http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training  Services
diff --git a/doc/src/sgml/ref/pg_resetxlog.sgml b/doc/src/sgml/ref/pg_resetxlog.sgml
index 34b0606..39ae574 100644
--- a/doc/src/sgml/ref/pg_resetxlog.sgml
+++ b/doc/src/sgml/ref/pg_resetxlog.sgml
@@ -30,6 +30,7 @@ PostgreSQL documentation
arg choice=optoption-m/option replaceable class=parametermxid/replaceable,replaceable class=parametermxid/replaceable/arg
arg choice=optoption-O/option replaceable class=parametermxoff/replaceable/arg
arg choice=optoption-l/option replaceable class=parameterxlogfile/replaceable/arg
+   arg choice=optoption-s/option [replaceable class=parametersysid/replaceable]/arg
arg choice=plainreplaceabledatadir/replaceable/arg
   /cmdsynopsis
  /refsynopsisdiv
@@ -184,6 +185,13 @@ PostgreSQL documentation
   /para
 
   para
+   The option-s/ option instructs commandpg_resetxlog/command to 
+   set the system identifier of the cluster to specified replaceablesysid/.
+   If the replaceablesysid/ is not provided new one will be generated using
+   same algorithm commandinitdb/ uses.
+  /para
+
+  para
The option-V/ and option--version/ options print
the applicationpg_resetxlog/application version and exit.  The
options option-?/ and option--help/ show supported arguments,
diff --git a/src/bin/pg_resetxlog/pg_resetxlog.c b/src/bin/pg_resetxlog/pg_resetxlog.c
index 915a1ed..c94ccda 100644
--- a/src/bin/pg_resetxlog/pg_resetxlog.c
+++ b/src/bin/pg_resetxlog/pg_resetxlog.c
@@ -67,7 +67,9 @@ static MultiXactId set_mxid = 0;
 static MultiXactOffset set_mxoff = (MultiXactOffset) -1;
 static uint32 minXlogTli = 0;
 static XLogSegNo minXlogSegNo = 0;
+static uint64 set_sysid = 0;
 
+static uint64 GenerateSystemIdentifier(void);
 static bool ReadControlFile(void);
 static void GuessControlValues(void);
 static void PrintControlValues(bool guessed);
@@ -111,7 +113,7 @@ main(int argc, char *argv[])
 	}
 
 
-	while ((c = getopt(argc, argv, fl:m:no:O:x:e:)) != -1)
+	while ((c = getopt(argc, argv, fl:m:no:O:s::x:e:)) != -1)
 	{
 		switch (c)
 		{
@@ -227,6 +229,33 @@ main(int argc, char *argv[])
 XLogFromFileName(optarg, minXlogTli, minXlogSegNo);
 break;
 
+			case 's':
+if (optarg)
+{
+#ifdef HAVE_STRTOULL
+	set_sysid = strtoull(optarg,

Re: [HACKERS] Set new system identifier using pg_resetxlog

2014-07-18 Thread Andres Freund 
On 2014-07-18 13:08:24 +0200, Petr Jelinek wrote:
 On 18/07/14 00:41, Andres Freund wrote:
 On 2014-06-27 00:51:02 +0200, Petr Jelinek wrote:
 {
 switch (c)
 {
 @@ -227,6 +229,33 @@ main(int argc, char *argv[])
 XLogFromFileName(optarg, minXlogTli, 
  minXlogSegNo);
 break;
 
 +   case 's':
 +   if (optarg)
 +   {
 +#ifdef HAVE_STRTOULL
 +   set_sysid = strtoull(optarg, endptr, 
 10);
 +#else
 +   set_sysid = strtoul(optarg, endptr, 
 10);
 +#endif
 
 Wouldn't it be better to use sscanf()? That should work with large
 inputs across platforms.
 
 
 Well, sscanf does not do much validation and silently truncates too big
 input (which is why I replaced it with strtoull, original patch did have
 sscanf)

Well, the checks on length you've added should catch that when adapted
properly.

, also I think the portability of sscanf might not be as good, see
 9d7ded0f4277f5c0063eca8e871a34e2355a8371 commit.

Fair point. But I don't think the arguments why using strtoul instead of
strtoull is safe hold much water here. In the pg_stat_statement case the
worst that could happen is that the rowcount isn't accurate. Here it's
setting a wrong system identifier. Not really the same.

Maybe it's time to pursue
http://archives.postgresql.org/message-id/20140603144654.GM24145%40awork2.anarazel.de
further :(

Greetings,

Andres Freund


-- 
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] Set new system identifier using pg_resetxlog

2014-07-18 Thread Petr Jelinek 

On 18/07/14 13:18, Andres Freund wrote:

On 2014-07-18 13:08:24 +0200, Petr Jelinek wrote:

On 18/07/14 00:41, Andres Freund wrote:

Wouldn't it be better to use sscanf()? That should work with large
inputs across platforms.



Well, sscanf does not do much validation and silently truncates too big
input (which is why I replaced it with strtoull, original patch did have
sscanf)


Well, the checks on length you've added should catch that when adapted
properly.


I don't see a portable way how to validate too big input values when 
using sscanf.




Maybe it's time to pursue
http://archives.postgresql.org/message-id/20140603144654.GM24145%40awork2.anarazel.de
further :(



Yes, I've seen that patch and was quite sad that it didn't make it in core.


--
 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] Set new system identifier using pg_resetxlog

2014-07-17 Thread Andres Freund 
On 2014-06-27 00:51:02 +0200, Petr Jelinek wrote:
 - while ((c = getopt(argc, argv, fl:m:no:O:x:e:)) != -1)
 + while ((c = getopt(argc, argv, fl:m:no:O:x:e:s::)) != -1)

Why two :?

   {
   switch (c)
   {
 @@ -227,6 +229,33 @@ main(int argc, char *argv[])
   XLogFromFileName(optarg, minXlogTli, 
 minXlogSegNo);
   break;
  
 + case 's':
 + if (optarg)
 + {
 +#ifdef HAVE_STRTOULL
 + set_sysid = strtoull(optarg, endptr, 
 10);
 +#else
 + set_sysid = strtoul(optarg, endptr, 
 10);
 +#endif

Wouldn't it be better to use sscanf()? That should work with large
inputs across platforms.

 + /*
 +  * Validate input, we use strspn 
 because strtoul(l) accepts
 +  * negative numbers and silently 
 converts them to unsigned
 +  */
 + if (set_sysid == 0 || errno != 0 ||
 + endptr == optarg || *endptr != 
 '\0' ||
 + strspn(optarg, 0123456789) != 
 strlen(optarg))
 + {
 + fprintf(stderr, _(%s: invalid 
 argument for option -s\n), progname);
 + fprintf(stderr, _(Try \%s 
 --help\ for more information.\n), progname);
 + exit(1);
 + }

Maybe: 'invalid argument \%s\ ...'?

 @@ -1087,6 +1147,7 @@ usage(void)
   printf(_(  -o OID   set next OID\n));
   printf(_(  -O OFFSETset next multitransaction offset\n));
   printf(_(  -V, --versionoutput version information, then exit\n));
 + printf(_(  -s [SYSID]   set system identifier (or generate 
 one)\n));
   printf(_(  -x XID   set next transaction ID\n));
   printf(_(  -?, --help   show this help, then exit\n));
   printf(_(\nReport bugs to pgsql-b...@postgresql.org.\n));

I think we usually try to keep the options roughly alphabetically
ordered. Same in the getopt() above.

Greetings,

Andres Freund


-- 
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] Set new system identifier using pg_resetxlog

2014-07-17 Thread Andres Freund 
On 2014-07-18 00:41:05 +0200, Andres Freund wrote:
 On 2014-06-27 00:51:02 +0200, Petr Jelinek wrote:
  -   while ((c = getopt(argc, argv, fl:m:no:O:x:e:)) != -1)
  +   while ((c = getopt(argc, argv, fl:m:no:O:x:e:s::)) != -1)
 
 Why two :?

Obviously strike that, wanted to delete the paragraph, but forgot about
it. I hadn't remembered the autogeneration of sysids at the beginning of
the patch.

Greetings,

Andres Freund

-- 
 Andres Freund 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] Set new system identifier using pg_resetxlog

2014-07-17 Thread Josh Berkus 
On 06/30/2014 09:46 AM, Alvaro Herrera wrote:
 If we only had bricks and mortar, I think we would have a tool to
 display and tweak pg_control separately from emptying pg_xlog, rather
 than this odd separation between pg_controldata and pg_resetxlog, each
 of which do a mixture of those things.  But we have a wall two thirds
 done already, so it seems to make more sense to me to continue forward
 rather than tear it down and start afresh.  This patch is a natural
 extension of what we already have.

As I said previously, I disagree.  Being able to set a system identifier
at runtime is useful for a variety of scenarios which do not involve
database surgery or the risk of wiping out your data; in fact, the only
bad thing you can do by changing the system id is break the replication
connection.

As such, tying a change in system id to pg_resetxlog is kind of like
having a combination dental floss dispenser and bazooka.  No, not
*that* trigger!

It pretty much guarantees that the ability to change the systemid, which
could be a generally useful feature with all kinds of application for
HA, clusters and sharding, would remain an internal feature of BDR only,
because everyone else will be afraid to touch it.

If this means that we need to finally create pg_editcontroldata, then so
be it.

-- 
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com


-- 
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] Set new system identifier using pg_resetxlog

2014-07-01 Thread Robert Haas 
On Mon, Jun 30, 2014 at 12:46 PM, Alvaro Herrera
alvhe...@2ndquadrant.com wrote:
 I think it's pretty much a given that pg_resetxlog is a tool that can
 have disastrous effects if used lightly.  If people changes their sysid
 wrongly, they're not any worse than if they change their multixact
 counters and start getting failures because the old values stored in
 data cannot be resolved anymore (it's already been wrapped around).
 Or if they remove all the XLOG they have since the latest crash.  From
 that POV, I don't think the objection that but this can be used to
 corrupt data! has any value.

After thinking about this a little more, I guess I don't really think
it's a bit problem either - so consider my objection withdrawn.

I am, however, kind of frustrated, still, that the pg_computemaxlsn
patch, which I thought was rather a good idea, was scuttled by the
essentially that same objection: let's not extend pg_resetxlog 
friends because people might use the new functionality to do bad
things and then blame us.

-- 
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

Re: [HACKERS] Set new system identifier using pg_resetxlog

2014-07-01 Thread Andres Freund 
On 2014-07-01 11:11:12 -0400, Robert Haas wrote:
 On Mon, Jun 30, 2014 at 12:46 PM, Alvaro Herrera
 alvhe...@2ndquadrant.com wrote:
  I think it's pretty much a given that pg_resetxlog is a tool that can
  have disastrous effects if used lightly.  If people changes their sysid
  wrongly, they're not any worse than if they change their multixact
  counters and start getting failures because the old values stored in
  data cannot be resolved anymore (it's already been wrapped around).
  Or if they remove all the XLOG they have since the latest crash.  From
  that POV, I don't think the objection that but this can be used to
  corrupt data! has any value.
 
 After thinking about this a little more, I guess I don't really think
 it's a bit problem either - so consider my objection withdrawn.

Thanks!

 I am, however, kind of frustrated, still, that the pg_computemaxlsn
 patch, which I thought was rather a good idea, was scuttled by the
 essentially that same objection: let's not extend pg_resetxlog 
 friends because people might use the new functionality to do bad
 things and then blame us.

Well, the reasons were a bit different. Senior community members
repeatedly suggested that it'd be usable for faillback - and it's not a
unreasonable to think it is. Even though it'd fail subtly because of
hint bit and related reasons.
In contrast you have to be pretty desperate to think that you could make
two clusters replicate from each other by just fudging pg_control long
enough, even if the clusters aren't actually related.

Greetings,

Andres Freund

-- 
 Andres Freund 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] Set new system identifier using pg_resetxlog

2014-07-01 Thread Alvaro Herrera 
Robert Haas wrote:
 On Mon, Jun 30, 2014 at 12:46 PM, Alvaro Herrera
 alvhe...@2ndquadrant.com wrote:
  I think it's pretty much a given that pg_resetxlog is a tool that can
  have disastrous effects if used lightly.  If people changes their sysid
  wrongly, they're not any worse than if they change their multixact
  counters and start getting failures because the old values stored in
  data cannot be resolved anymore (it's already been wrapped around).
  Or if they remove all the XLOG they have since the latest crash.  From
  that POV, I don't think the objection that but this can be used to
  corrupt data! has any value.
 
 After thinking about this a little more, I guess I don't really think
 it's a bit problem either - so consider my objection withdrawn.

Great.

 I am, however, kind of frustrated, still, that the pg_computemaxlsn
 patch, which I thought was rather a good idea, was scuttled by the
 essentially that same objection: let's not extend pg_resetxlog 
 friends because people might use the new functionality to do bad
 things and then blame us.

Uh, I thought you killed that patch yourself:
http://www.postgresql.org/message-id/CA+TgmoZqbYHWYbi18FUk-+dGHig=icv+pj-nnav3miy4daj...@mail.gmail.com

-- 
Álvaro Herrerahttp://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] Set new system identifier using pg_resetxlog

2014-07-01 Thread Robert Haas 
On Tue, Jul 1, 2014 at 11:19 AM, Andres Freund and...@2ndquadrant.com wrote:
 I am, however, kind of frustrated, still, that the pg_computemaxlsn
 patch, which I thought was rather a good idea, was scuttled by the
 essentially that same objection: let's not extend pg_resetxlog 
 friends because people might use the new functionality to do bad
 things and then blame us.

 Well, the reasons were a bit different. Senior community members
 repeatedly suggested that it'd be usable for faillback - and it's not a
 unreasonable to think it is. Even though it'd fail subtly because of
 hint bit and related reasons.
 In contrast you have to be pretty desperate to think that you could make
 two clusters replicate from each other by just fudging pg_control long
 enough, even if the clusters aren't actually related.

Well, I still think it's pretty likely someone could make that mistake
here, too.  Maybe not a senior community member, but somebody.
However, I think the right answer in that case and this one is to tell
the person who has made the mistake you screwed up and move on.

-- 
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

Re: [HACKERS] Set new system identifier using pg_resetxlog

2014-06-30 Thread Fujii Masao 
On Sun, Jun 29, 2014 at 11:18 PM, Andres Freund and...@2ndquadrant.com wrote:
 On 2014-06-29 19:44:21 +0530, Abhijit Menon-Sen wrote:
 At 2014-06-27 00:51:02 +0200, p...@2ndquadrant.com wrote:
 
  Also based on Alvaro's comment, I replaced the scanf parsing code with
  strtoul(l) function.

 As far as I can tell (from the thread and a quick look at the patch),
 your latest version of the patch addresses all the review comments.

 Should I mark this ready for committer now?

 Well, we haven't really agreed on a way forward yet. Robert disagrees
 that the feature is independently useful and thinks it might be
 dangerous for some users. I don't agree with either of those points, but
 that doesn't absolve the patch from the objection. I think tentatively
 have been more people in favor, but it's not clear cut imo.

So what's the usecase of this feature? If it's for normal operation,
using pg_resetxlog for that is a bit dangerous because it can corrupt
the database easily.

Regards,

-- 
Fujii Masao


-- 
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] Set new system identifier using pg_resetxlog

2014-06-30 Thread Andres Freund 
On 2014-06-30 19:57:58 +0900, Fujii Masao wrote:
 On Sun, Jun 29, 2014 at 11:18 PM, Andres Freund and...@2ndquadrant.com 
 wrote:
  On 2014-06-29 19:44:21 +0530, Abhijit Menon-Sen wrote:
  At 2014-06-27 00:51:02 +0200, p...@2ndquadrant.com wrote:
  
   Also based on Alvaro's comment, I replaced the scanf parsing code with
   strtoul(l) function.
 
  As far as I can tell (from the thread and a quick look at the patch),
  your latest version of the patch addresses all the review comments.
 
  Should I mark this ready for committer now?
 
  Well, we haven't really agreed on a way forward yet. Robert disagrees
  that the feature is independently useful and thinks it might be
  dangerous for some users. I don't agree with either of those points, but
  that doesn't absolve the patch from the objection. I think tentatively
  have been more people in favor, but it's not clear cut imo.
 
 So what's the usecase of this feature? If it's for normal operation,
 using pg_resetxlog for that is a bit dangerous because it can corrupt
 the database easily.

a) Mark a database as not being the same. Currently if you promote two
   databases, e.g. to shard out, they'll continue to have same system
   identifier. Which really sucks, especially because timeline ids will
   often increase synchronously.

b) For data recovery it's sometimes useful to create a new database
   (with the same catalog state) and replay all WAL. For that you need to
   reset the system identifier. I've done so hacking up resetxlog
   before.

c) We already allow to set pretty much all aspects of the control file
   via resetxlog - there seems little point of not having the ability to
   change the system identifier.

d) In a logical replication scenario one way to identify individual
   nodes is via the system identifier. If you want to convert a
   basebackup into logical standby one sensible way to do so is to
   create a logical replication slots *before* promoting a physical
   backup to guarantee that slot is able to stream out all changes. If
   the slot names contain the consumer's system identifier you need to
   know the new system identifier beforehand.

Greetings,

Andres Freund

-- 
 Andres Freund 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] Set new system identifier using pg_resetxlog

2014-06-30 Thread Alvaro Herrera 
Andres Freund wrote:

 c) We already allow to set pretty much all aspects of the control file
via resetxlog - there seems little point of not having the ability to
change the system identifier.

I think it's pretty much a given that pg_resetxlog is a tool that can
have disastrous effects if used lightly.  If people changes their sysid
wrongly, they're not any worse than if they change their multixact
counters and start getting failures because the old values stored in
data cannot be resolved anymore (it's already been wrapped around).
Or if they remove all the XLOG they have since the latest crash.  From
that POV, I don't think the objection that but this can be used to
corrupt data! has any value.

Also on the other hand pg_resetxlog is already a tool for PG hackers to
fool around and test things.  In a way, being able to change values in
pg_control is useful to many of us; this is only an extension of that.

If we only had bricks and mortar, I think we would have a tool to
display and tweak pg_control separately from emptying pg_xlog, rather
than this odd separation between pg_controldata and pg_resetxlog, each
of which do a mixture of those things.  But we have a wall two thirds
done already, so it seems to make more sense to me to continue forward
rather than tear it down and start afresh.  This patch is a natural
extension of what we already have.

-- 
Álvaro Herrerahttp://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] Set new system identifier using pg_resetxlog

2014-06-29 Thread Abhijit Menon-Sen 
At 2014-06-27 00:51:02 +0200, p...@2ndquadrant.com wrote:

 Also based on Alvaro's comment, I replaced the scanf parsing code with
 strtoul(l) function.

As far as I can tell (from the thread and a quick look at the patch),
your latest version of the patch addresses all the review comments.

Should I mark this ready for committer now?

-- Abhijit


-- 
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] Set new system identifier using pg_resetxlog

2014-06-29 Thread Andres Freund 
On 2014-06-29 19:44:21 +0530, Abhijit Menon-Sen wrote:
 At 2014-06-27 00:51:02 +0200, p...@2ndquadrant.com wrote:
 
  Also based on Alvaro's comment, I replaced the scanf parsing code with
  strtoul(l) function.
 
 As far as I can tell (from the thread and a quick look at the patch),
 your latest version of the patch addresses all the review comments.
 
 Should I mark this ready for committer now?

Well, we haven't really agreed on a way forward yet. Robert disagrees
that the feature is independently useful and thinks it might be
dangerous for some users. I don't agree with either of those points, but
that doesn't absolve the patch from the objection. I think tentatively
have been more people in favor, but it's not clear cut imo.

Greetings,

Andres Freund

-- 
 Andres Freund 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] Set new system identifier using pg_resetxlog

2014-06-26 Thread Petr Jelinek 

On 25/06/14 19:43, Sawada Masahiko wrote:

Hi,

I send you review comment about thie patch.


Hello, thanks.


--
$ pg_resetxlog -s -n  data | grep Database system identifier
Database system identifier:   6028907917695471865

The -s option does not worksfine with -n option.


Fixed.


--
$ pg_resetxlog
-s602890791769547186511
data
Transaction log reset
$ pg_controldata data | grep Database system identifier
Database system identifier:   18446744073709551615

pg_resetxlog is finished successfully, but system identifier was not
changed.
Also I think that checking data about number of digits is needed.



It actually did change the identifier, just to ULONG_MAX, since that's 
the maximum value that can be set (scanf does that conversion), I added 
check that limits the maximum value of system identifier input to 
ULONG_MAX-1 and reports error if it's bigger. I also added some 
additional input validation.


--
 Petr Jelinek  http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training  Services
diff --git a/doc/src/sgml/ref/pg_resetxlog.sgml b/doc/src/sgml/ref/pg_resetxlog.sgml
index 34b0606..39ae574 100644
--- a/doc/src/sgml/ref/pg_resetxlog.sgml
+++ b/doc/src/sgml/ref/pg_resetxlog.sgml
@@ -30,6 +30,7 @@ PostgreSQL documentation
arg choice=optoption-m/option replaceable class=parametermxid/replaceable,replaceable class=parametermxid/replaceable/arg
arg choice=optoption-O/option replaceable class=parametermxoff/replaceable/arg
arg choice=optoption-l/option replaceable class=parameterxlogfile/replaceable/arg
+   arg choice=optoption-s/option [replaceable class=parametersysid/replaceable]/arg
arg choice=plainreplaceabledatadir/replaceable/arg
   /cmdsynopsis
  /refsynopsisdiv
@@ -184,6 +185,13 @@ PostgreSQL documentation
   /para
 
   para
+   The option-s/ option instructs commandpg_resetxlog/command to 
+   set the system identifier of the cluster to specified replaceablesysid/.
+   If the replaceablesysid/ is not provided new one will be generated using
+   same algorithm commandinitdb/ uses.
+  /para
+
+  para
The option-V/ and option--version/ options print
the applicationpg_resetxlog/application version and exit.  The
options option-?/ and option--help/ show supported arguments,
diff --git a/src/bin/pg_resetxlog/pg_resetxlog.c b/src/bin/pg_resetxlog/pg_resetxlog.c
index 915a1ed..e8bbbc1 100644
--- a/src/bin/pg_resetxlog/pg_resetxlog.c
+++ b/src/bin/pg_resetxlog/pg_resetxlog.c
@@ -67,7 +67,9 @@ static MultiXactId set_mxid = 0;
 static MultiXactOffset set_mxoff = (MultiXactOffset) -1;
 static uint32 minXlogTli = 0;
 static XLogSegNo minXlogSegNo = 0;
+static uint64 set_sysid = 0;
 
+static uint64 GenerateSystemIdentifier(void);
 static bool ReadControlFile(void);
 static void GuessControlValues(void);
 static void PrintControlValues(bool guessed);
@@ -111,7 +113,7 @@ main(int argc, char *argv[])
 	}
 
 
-	while ((c = getopt(argc, argv, fl:m:no:O:x:e:)) != -1)
+	while ((c = getopt(argc, argv, fl:m:no:O:x:e:s::)) != -1)
 	{
 		switch (c)
 		{
@@ -227,6 +229,29 @@ main(int argc, char *argv[])
 XLogFromFileName(optarg, minXlogTli, minXlogSegNo);
 break;
 
+			case 's':
+if (optarg)
+{
+	if (strspn(optarg, 0123456789) != strlen(optarg) ||
+		sscanf(optarg, UINT64_FORMAT, set_sysid) != 1)
+	{
+		fprintf(stderr, _(%s: invalid argument for option -s\n), progname);
+		fprintf(stderr, _(Try \%s --help\ for more information.\n), progname);
+		exit(1);
+	}
+
+	if (set_sysid == ULONG_MAX)
+	{
+		fprintf(stderr, _(%s: system identifier (-s) is too large\n), progname);
+		exit(1);
+	}
+}
+else
+{
+	set_sysid = GenerateSystemIdentifier();
+}
+break;
+
 			default:
 fprintf(stderr, _(Try \%s --help\ for more information.\n), progname);
 exit(1);
@@ -354,6 +379,9 @@ main(int argc, char *argv[])
 	if (minXlogSegNo  newXlogSegNo)
 		newXlogSegNo = minXlogSegNo;
 
+	if (set_sysid != 0)
+		ControlFile.system_identifier = set_sysid;
+
 	/*
 	 * If we had to guess anything, and -f was not given, just print the
 	 * guessed values and exit.  Also print if -n is given.
@@ -395,6 +423,26 @@ main(int argc, char *argv[])
 
 
 /*
+ * Create a new unique installation identifier.
+ *
+ * See notes in xlog.c about the algorithm.
+ */
+static uint64
+GenerateSystemIdentifier(void)
+{
+	uint64			sysidentifier;
+	struct timeval	tv;
+
+	gettimeofday(tv, NULL);
+	sysidentifier = ((uint64) tv.tv_sec)  32;
+	sysidentifier |= ((uint64) tv.tv_usec)  12;
+	sysidentifier |= getpid()  0xFFF;
+
+	return sysidentifier;
+}
+
+
+/*
  * Try to read the existing pg_control file.
  *
  * This routine is also responsible for updating old pg_control versions
@@ -475,9 +523,6 @@ ReadControlFile(void)
 static void
 GuessControlValues(void)
 {
-	uint64		sysidentifier;
-	struct timeval tv;
-
 	/*
 	 * Set up a completely default set of

Re: [HACKERS] Set new system identifier using pg_resetxlog

2014-06-26 Thread Sawada Masahiko 
Thank you for updating the patch.
I think that the following behaviour of pg_resetxlog is bug.

$ pg_controldata data | grep Database system identifier
Database system identifier:   6029284919152642525

--
$ pg_resetxlog -s0 -n data
Current pg_control values:

pg_control version number:942
Catalog version number:   201406181
Database system identifier:   6029284919152642525
Latest checkpoint's TimeLineID:   1
Latest checkpoint's full_page_writes: on
Latest checkpoint's NextXID:  0/1810
Latest checkpoint's NextOID:  13004
Latest checkpoint's NextMultiXactId:  1
Latest checkpoint's NextMultiOffset:  0
Latest checkpoint's oldestXID:1800
Latest checkpoint's oldestXID's DB:   1
Latest checkpoint's oldestActiveXID:  0
Latest checkpoint's oldestMultiXid:   1
Latest checkpoint's oldestMulti's DB: 1
Maximum data alignment:   8
Database block size:  8192
Blocks per segment of large relation: 131072
WAL block size:   8192
Bytes per WAL segment:16777216
Maximum length of identifiers:64
Maximum columns in an index:  32
Maximum size of a TOAST chunk:1996
Size of a large-object chunk: 2048
Date/time type storage:   64-bit integers
Float4 argument passing:  by value
Float8 argument passing:  by value
Data page checksum version:   0


Values to be changed:

First log segment after reset:00010002

--
$ pg_resetxlog  -s0 data
Transaction log reset
$ pg_controldata data | grep Database system identifier
Database system identifier:   6029284919152642525

this patch dose not works fine with -s0.

Regards,
--
Sawada Masahiko

On Thursday, June 26, 2014, Petr Jelinek p...@2ndquadrant.com wrote:

 On 25/06/14 19:43, Sawada Masahiko wrote:

 Hi,

 I send you review comment about thie patch.


 Hello, thanks.

  --
 $ pg_resetxlog -s -n  data | grep Database system identifier
 Database system identifier:   6028907917695471865

 The -s option does not worksfine with -n option.


 Fixed.

  --
 $ pg_resetxlog
 -s6028907917695471865
 11
 data
 Transaction log reset
 $ pg_controldata data | grep Database system identifier
 Database system identifier:   18446744073709551615

 pg_resetxlog is finished successfully, but system identifier was not
 changed.
 Also I think that checking data about number of digits is needed.


 It actually did change the identifier, just to ULONG_MAX, since that's the
 maximum value that can be set (scanf does that conversion), I added check
 that limits the maximum value of system identifier input to ULONG_MAX-1 and
 reports error if it's bigger. I also added some additional input validation.

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



-- 
Regards,

---
Sawada Masahiko

Re: [HACKERS] Set new system identifier using pg_resetxlog

2014-06-26 Thread Petr Jelinek 

On 26/06/14 19:57, Sawada Masahiko wrote:

$ pg_resetxlog  -s0 data
Transaction log reset
$ pg_controldata data | grep Database system identifier
Database system identifier:   6029284919152642525

this patch dose not works fine with -s0.



Yes, this is a bug, 0 input should throw error, which it does now.

Also based on Alvaro's comment, I replaced the scanf parsing code with 
strtoul(l) function.



--
 Petr Jelinek  http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training  Services
diff --git a/doc/src/sgml/ref/pg_resetxlog.sgml b/doc/src/sgml/ref/pg_resetxlog.sgml
index 34b0606..39ae574 100644
--- a/doc/src/sgml/ref/pg_resetxlog.sgml
+++ b/doc/src/sgml/ref/pg_resetxlog.sgml
@@ -30,6 +30,7 @@ PostgreSQL documentation
arg choice=optoption-m/option replaceable class=parametermxid/replaceable,replaceable class=parametermxid/replaceable/arg
arg choice=optoption-O/option replaceable class=parametermxoff/replaceable/arg
arg choice=optoption-l/option replaceable class=parameterxlogfile/replaceable/arg
+   arg choice=optoption-s/option [replaceable class=parametersysid/replaceable]/arg
arg choice=plainreplaceabledatadir/replaceable/arg
   /cmdsynopsis
  /refsynopsisdiv
@@ -184,6 +185,13 @@ PostgreSQL documentation
   /para
 
   para
+   The option-s/ option instructs commandpg_resetxlog/command to 
+   set the system identifier of the cluster to specified replaceablesysid/.
+   If the replaceablesysid/ is not provided new one will be generated using
+   same algorithm commandinitdb/ uses.
+  /para
+
+  para
The option-V/ and option--version/ options print
the applicationpg_resetxlog/application version and exit.  The
options option-?/ and option--help/ show supported arguments,
diff --git a/src/bin/pg_resetxlog/pg_resetxlog.c b/src/bin/pg_resetxlog/pg_resetxlog.c
index 915a1ed..0b28bc0 100644
--- a/src/bin/pg_resetxlog/pg_resetxlog.c
+++ b/src/bin/pg_resetxlog/pg_resetxlog.c
@@ -67,7 +67,9 @@ static MultiXactId set_mxid = 0;
 static MultiXactOffset set_mxoff = (MultiXactOffset) -1;
 static uint32 minXlogTli = 0;
 static XLogSegNo minXlogSegNo = 0;
+static uint64 set_sysid = 0;
 
+static uint64 GenerateSystemIdentifier(void);
 static bool ReadControlFile(void);
 static void GuessControlValues(void);
 static void PrintControlValues(bool guessed);
@@ -111,7 +113,7 @@ main(int argc, char *argv[])
 	}
 
 
-	while ((c = getopt(argc, argv, fl:m:no:O:x:e:)) != -1)
+	while ((c = getopt(argc, argv, fl:m:no:O:x:e:s::)) != -1)
 	{
 		switch (c)
 		{
@@ -227,6 +229,33 @@ main(int argc, char *argv[])
 XLogFromFileName(optarg, minXlogTli, minXlogSegNo);
 break;
 
+			case 's':
+if (optarg)
+{
+#ifdef HAVE_STRTOULL
+	set_sysid = strtoull(optarg, endptr, 10);
+#else
+	set_sysid = strtoul(optarg, endptr, 10);
+#endif
+	/*
+	 * Validate input, we use strspn because strtoul(l) accepts
+	 * negative numbers and silently converts them to unsigned
+	 */
+	if (set_sysid == 0 || errno != 0 ||
+		endptr == optarg || *endptr != '\0' ||
+		strspn(optarg, 0123456789) != strlen(optarg))
+	{
+		fprintf(stderr, _(%s: invalid argument for option -s\n), progname);
+		fprintf(stderr, _(Try \%s --help\ for more information.\n), progname);
+		exit(1);
+	}
+}
+else
+{
+	set_sysid = GenerateSystemIdentifier();
+}
+break;
+
 			default:
 fprintf(stderr, _(Try \%s --help\ for more information.\n), progname);
 exit(1);
@@ -354,6 +383,9 @@ main(int argc, char *argv[])
 	if (minXlogSegNo  newXlogSegNo)
 		newXlogSegNo = minXlogSegNo;
 
+	if (set_sysid != 0)
+		ControlFile.system_identifier = set_sysid;
+
 	/*
 	 * If we had to guess anything, and -f was not given, just print the
 	 * guessed values and exit.  Also print if -n is given.
@@ -395,6 +427,26 @@ main(int argc, char *argv[])
 
 
 /*
+ * Create a new unique installation identifier.
+ *
+ * See notes in xlog.c about the algorithm.
+ */
+static uint64
+GenerateSystemIdentifier(void)
+{
+	uint64			sysidentifier;
+	struct timeval	tv;
+
+	gettimeofday(tv, NULL);
+	sysidentifier = ((uint64) tv.tv_sec)  32;
+	sysidentifier |= ((uint64) tv.tv_usec)  12;
+	sysidentifier |= getpid()  0xFFF;
+
+	return sysidentifier;
+}
+
+
+/*
  * Try to read the existing pg_control file.
  *
  * This routine is also responsible for updating old pg_control versions
@@ -475,9 +527,6 @@ ReadControlFile(void)
 static void
 GuessControlValues(void)
 {
-	uint64		sysidentifier;
-	struct timeval tv;
-
 	/*
 	 * Set up a completely default set of pg_control values.
 	 */
@@ -489,14 +538,10 @@ GuessControlValues(void)
 
 	/*
 	 * Create a new unique installation identifier, since we can no longer use
-	 * any old XLOG records.  See notes in xlog.c about the algorithm.
+	 * any old XLOG records.
 	 */
-	gettimeofday(tv, NULL);
-	sysidentifier = ((uint64) tv.tv_sec)  32;
-	sysidentifier |= ((uint64) tv.tv_usec)  12;
-	sysidentifier |= getpid()  0xFFF;
 
-

Re: [HACKERS] Set new system identifier using pg_resetxlog

2014-06-25 Thread Sawada Masahiko 
Hi,

I send you review comment about thie patch.

I found no error/warning with compling and installation.
I have executed pg_resetxlog with some input pattern.

$ initdb -D data -E UTF8 --no-locale
$ pg_controldata data | grep Database system identifier
Database system identifier:   6028907917695471865

--
$ pg_resetxlog -s -n  data | grep Database system identifier
Database system identifier:   6028907917695471865

The -s option does not works fine with -n option.

--
$ pg_resetxlog
-s602890791769547186511
data
Transaction log reset
$ pg_controldata data | grep Database system identifier
Database system identifier:   18446744073709551615

pg_resetxlog is finished successfully, but system identifier was not
changed.
Also I think that checking data about number of digits is needed.

regards
--
Sawada Masahiko

On Thursday, June 19, 2014, Petr Jelinek p...@2ndquadrant.com wrote:

 On 18/06/14 19:26, Robert Haas wrote:

 On Wed, Jun 18, 2014 at 12:54 PM, Andres Freund and...@2ndquadrant.com
 wrote:

 I don't see how the proposed ability makes it more dangerous. It
 *already* has the ability to reset the timelineid. That's the case where
 users are much more likely to think about resetting it because that's
 much more plausible than taking a unrelated cluster and resetting its
 sysid, timeline and LSN.


 All right, well, I've said my piece.  I don't have anything to add to
 that that isn't sheer repetition.  My vote is to hold off on this
 until we've talked about replication identifiers and other related
 topics in more depth.  But if that position doesn't garner majority
 support ... so be it!


 I am not sure I get what does this have to do with replication
 identifiers. The patch has several use-cases, one of them has to do that
 you can know the future system id before you set it, which is useful for
 automating some things...

 --
  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



-- 
Regards,

---
Sawada Masahiko

Re: [HACKERS] Set new system identifier using pg_resetxlog

2014-06-25 Thread Michael Paquier 
On Thu, Jun 26, 2014 at 2:43 AM, Sawada Masahiko sawada.m...@gmail.com
wrote:

 Hi,

 I send you review comment about thie patch.

 I found no error/warning with compling and installation.
 I have executed pg_resetxlog with some input pattern.

 $ initdb -D data -E UTF8 --no-locale
 $ pg_controldata data | grep Database system identifier
 Database system identifier:   6028907917695471865

 --
 $ pg_resetxlog -s -n  data | grep Database system identifier
 Database system identifier:   6028907917695471865

 The -s option does not works fine with -n option.

 --
 $ pg_resetxlog
 -s602890791769547186511
 data
 Transaction log reset
 $ pg_controldata data | grep Database system identifier
 Database system identifier:   18446744073709551615

 pg_resetxlog is finished successfully, but system identifier was not
 changed.
 Also I think that checking data about number of digits is needed.


Yep, system_identifier is a uint64, and the input you are giving here is
incompatible with that.
-- 
Michael

Re: [HACKERS] Set new system identifier using pg_resetxlog

2014-06-18 Thread Robert Haas 
On Tue, Jun 17, 2014 at 12:50 PM, Andres Freund and...@2ndquadrant.com wrote:
  I can clearly understand the utility of being able to reset the system
  ID to a new, randomly-generated system ID - but giving the user the
  ability to set a particular value of their own choosing seems like a
  pretty sharp tool.  What is the use case for that?

 I've previously hacked this up adhoc during data recovery when I needed
 to make another cluster similar enough that I could replay WAL.

 Another usecase is to mark a database as independent from its
 origin. Imagine a database that gets sharded across several
 servers. It's not uncommon to do that by initially basebackup'ing the
 database to several nodes and then use them separately from
 thereon. It's quite useful to actually mark them as being
 distinct. Especially as several of them right now would end up with the
 same timeline id...

Sure, but that only requires being able to reset the ID randomly, not
to a particular value.

 But it seems to me that we might need to have a process discussion
 here, because, while I'm all in favor of incremental feature proposals
 that build towards a larger goal, it currently appears that the larger
 goal toward which you are building is not something that's been
 publicly discussed and debated on this list.  And I really think we
 need to have that conversation.  Obviously, individual patches will
 still need to be debated, but I feel like 2ndQuadrant is trying to
 construct a castle without showing the community the floor plan.  I
 believe that there is relatively broad agreement that we would all
 like a castle, but different people may have legitimately different
 ideas about how it should be constructed.  If the work arrives as a
 series of disconnected pieces (user-specified system ID, event
 triggers for CREATE, etc.), then everyone outside of 2ndQuadrant has
 to take it on faith that those pieces are going to eventually fit
 together in a way that we'll all be happy with.  In some cases, that's
 fine, because the feature is useful on its own merits whether it ends
 up being part of the castle or not.


 Uh. Right now this patch has been written because it's needed for a out
 of core replication solution. That's what BDR is at this point. The
 patch is unobtrusive, has other usecases than just our internal one and
 doesn't make pg_resetxlog even more dangerous than it already is. I
 don't see much problem with considering it on it's own cost/benefit?

Well, I think it *does* make pg_resetxlog more dangerous; see previous
discussion of pg_computemaxlsn.

 So this seems to be a concern that's relatively independent of this
 patch. Am I seing that right?

Partially; not completely.

 I actually don't think any of the discussions I was involved in had the
 externally visible version of replication identifiers limited to 16bits?
 If you are referring to my patch, 16bits was just the width of the
 *internal* name that should basically never be looked at. User visible
 replication identifiers are always identified by an arbitrary string -
 whose format is determined by the user of the replication identifier
 facility. *BDR* currently stores the system identifer, the database id
 and a name in there - but that's nothing core needs to concern itself
 with.

I don't think you're going to be able to avoid users needing to know
about those IDs.  The configuration table is going to have to be the
same on all nodes, and how are you going to get that set up without
those IDs being user-visible?

-- 
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

Re: [HACKERS] Set new system identifier using pg_resetxlog

2014-06-18 Thread Andres Freund 
On 2014-06-18 12:36:13 -0400, Robert Haas wrote:
 On Tue, Jun 17, 2014 at 12:50 PM, Andres Freund and...@2ndquadrant.com 
 wrote:
   I can clearly understand the utility of being able to reset the system
   ID to a new, randomly-generated system ID - but giving the user the
   ability to set a particular value of their own choosing seems like a
   pretty sharp tool.  What is the use case for that?
 
  I've previously hacked this up adhoc during data recovery when I needed
  to make another cluster similar enough that I could replay WAL.
 
  Another usecase is to mark a database as independent from its
  origin. Imagine a database that gets sharded across several
  servers. It's not uncommon to do that by initially basebackup'ing the
  database to several nodes and then use them separately from
  thereon. It's quite useful to actually mark them as being
  distinct. Especially as several of them right now would end up with the
  same timeline id...
 
 Sure, but that only requires being able to reset the ID randomly, not
 to a particular value.

I can definitely see a point in a version of the option that generates
the id randomly. But the use case one up actually does require setting
it to a s specific value...

  Uh. Right now this patch has been written because it's needed for a out
  of core replication solution. That's what BDR is at this point. The
  patch is unobtrusive, has other usecases than just our internal one and
  doesn't make pg_resetxlog even more dangerous than it already is. I
  don't see much problem with considering it on it's own cost/benefit?
 
 Well, I think it *does* make pg_resetxlog more dangerous; see previous
 discussion of pg_computemaxlsn.

Wasn't the thing around pg_computemaxlsn that there's actually no case
where it could be used safely? And that experienced people suggested to
use it an unsafe fashion?
I don't see how the proposed ability makes it more dangerous. It
*already* has the ability to reset the timelineid. That's the case where
users are much more likely to think about resetting it because that's
much more plausible than taking a unrelated cluster and resetting its
sysid, timeline and LSN.

Greetings,

Andres Freund

-- 
 Andres Freund 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] Set new system identifier using pg_resetxlog

2014-06-18 Thread Andres Freund 
On 2014-06-18 18:54:05 +0200, Andres Freund wrote:
 On 2014-06-18 12:36:13 -0400, Robert Haas wrote:
  Sure, but that only requires being able to reset the ID randomly, not
  to a particular value.
 
 I can definitely see a point in a version of the option that generates
 the id randomly.

That's actually included in the patch btw (thanks Abhijit)...

Greetings,

Andres Freund

-- 
 Andres Freund 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] Set new system identifier using pg_resetxlog

2014-06-18 Thread Robert Haas 
On Wed, Jun 18, 2014 at 12:54 PM, Andres Freund and...@2ndquadrant.com wrote:
 Well, I think it *does* make pg_resetxlog more dangerous; see previous
 discussion of pg_computemaxlsn.

 Wasn't the thing around pg_computemaxlsn that there's actually no case
 where it could be used safely? And that experienced people suggested to
 use it an unsafe fashion?

There is a use case - to determine whether WAL has been replayed from
the future relative to the WAL stream and control file you have on
disk.  But the rest is true enough.

 I don't see how the proposed ability makes it more dangerous. It
 *already* has the ability to reset the timelineid. That's the case where
 users are much more likely to think about resetting it because that's
 much more plausible than taking a unrelated cluster and resetting its
 sysid, timeline and LSN.

All right, well, I've said my piece.  I don't have anything to add to
that that isn't sheer repetition.  My vote is to hold off on this
until we've talked about replication identifiers and other related
topics in more depth.  But if that position doesn't garner majority
support ... so be it!

-- 
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

Re: [HACKERS] Set new system identifier using pg_resetxlog

2014-06-18 Thread Petr Jelinek 

On 18/06/14 19:26, Robert Haas wrote:

On Wed, Jun 18, 2014 at 12:54 PM, Andres Freund and...@2ndquadrant.com wrote:

I don't see how the proposed ability makes it more dangerous. It
*already* has the ability to reset the timelineid. That's the case where
users are much more likely to think about resetting it because that's
much more plausible than taking a unrelated cluster and resetting its
sysid, timeline and LSN.


All right, well, I've said my piece.  I don't have anything to add to
that that isn't sheer repetition.  My vote is to hold off on this
until we've talked about replication identifiers and other related
topics in more depth.  But if that position doesn't garner majority
support ... so be it!



I am not sure I get what does this have to do with replication 
identifiers. The patch has several use-cases, one of them has to do that 
you can know the future system id before you set it, which is useful for 
automating some things...


--
 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] Set new system identifier using pg_resetxlog

2014-06-18 Thread Andres Freund 
On 2014-06-18 13:26:37 -0400, Robert Haas wrote:
 My vote is to hold off on this until we've talked about replication
 identifiers and other related topics in more depth.

I really don't understand this. We're *NOT* proposing this patch as an
underhanded way of preempting the discussion of whether/how replication
identifiers are going to be used. We're proposing it because we
currently have a need for the facility and this will reduce the number
of patches we have to keep around after 9.5. And more importantly
because there's several other use cases than our internal one for it.

To settle one more point: I am not planning to propose BDR's generation
of replication identifier names for integration. It works well enough
for BDR but I think we can come up with something better for core. If I
had my current knowledge two years back I'd not have chosen the current
scheme.

Greetings,

Andres Freund

-- 
 Andres Freund 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] Set new system identifier using pg_resetxlog

2014-06-18 Thread Josh Berkus 
On 06/13/2014 05:31 PM, Petr Jelinek wrote:
 Hello,
 
 attached is a simple patch which makes it possible to change the system
 identifier of the cluster in pg_control. This is useful for
 individualization of the instance that is started on top of data
 directory produced by pg_basebackup - something that's helpful for
 logical replication setup where you need to easily identify each node
 (it's used by Bidirectional Replication for example).

I'm unclear on why we would overload pg_resetxlog for this.  Wouldn't it
be better design to have an independant function,
pg_set_system_identifier?

-- 
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com


-- 
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] Set new system identifier using pg_resetxlog

2014-06-18 Thread Alvaro Herrera 
Josh Berkus wrote:
 On 06/13/2014 05:31 PM, Petr Jelinek wrote:
  Hello,
  
  attached is a simple patch which makes it possible to change the system
  identifier of the cluster in pg_control. This is useful for
  individualization of the instance that is started on top of data
  directory produced by pg_basebackup - something that's helpful for
  logical replication setup where you need to easily identify each node
  (it's used by Bidirectional Replication for example).
 
 I'm unclear on why we would overload pg_resetxlog for this.  Wouldn't it
 be better design to have an independant function,
 pg_set_system_identifier?

We have overloaded pg_resetxlog for all pg_control-tweaking needs.  I
don't see any reason to do differently here.

-- 
Álvaro Herrerahttp://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] Set new system identifier using pg_resetxlog

2014-06-18 Thread Andres Freund 
On 2014-06-18 10:44:56 -0700, Josh Berkus wrote:
 On 06/13/2014 05:31 PM, Petr Jelinek wrote:
  Hello,
  
  attached is a simple patch which makes it possible to change the system
  identifier of the cluster in pg_control. This is useful for
  individualization of the instance that is started on top of data
  directory produced by pg_basebackup - something that's helpful for
  logical replication setup where you need to easily identify each node
  (it's used by Bidirectional Replication for example).
 
 I'm unclear on why we would overload pg_resetxlog for this.  Wouldn't it
 be better design to have an independant function,
 pg_set_system_identifier?

You mean an independent binary? Because it's not possible to change this
at runtime.

If so, it's because pg_resetxlog already has the option to change many
related things (e.g. the timeline id). And it'd require another copy of
several hundred lines of code. It's all stored in the control file/checkpoints.

Greetings,

Andres Freund

-- 
 Andres Freund 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] Set new system identifier using pg_resetxlog

2014-06-18 Thread Abhijit Menon-Sen 
At 2014-06-18 10:44:56 -0700, j...@agliodbs.com wrote:

 I'm unclear on why we would overload pg_resetxlog for this. 

Because pg_resetxlog already does something very similar, so the patch
is small. If it were independent, it would have to copy quite some code
from pg_resetxlog.

 Wouldn't it be better design to have an independant function,
 pg_set_system_identifier?

A *function*? Why?

-- Abhijit


-- 
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] Set new system identifier using pg_resetxlog

2014-06-18 Thread Josh Berkus 
On 06/18/2014 10:48 AM, Abhijit Menon-Sen wrote:
 At 2014-06-18 10:44:56 -0700, j...@agliodbs.com wrote:

 I'm unclear on why we would overload pg_resetxlog for this. 
 
 Because pg_resetxlog already does something very similar, so the patch
 is small. If it were independent, it would have to copy quite some code
 from pg_resetxlog.

Aha.  In that case, it seems like it's time to rename pg_resetxlog, if
it does a bunch of things that aren't resetting the xlog.

-- 
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com


-- 
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] Set new system identifier using pg_resetxlog

2014-06-18 Thread Andres Freund 
On 2014-06-18 10:59:59 -0700, Josh Berkus wrote:
 On 06/18/2014 10:48 AM, Abhijit Menon-Sen wrote:
  At 2014-06-18 10:44:56 -0700, j...@agliodbs.com wrote:
 
  I'm unclear on why we would overload pg_resetxlog for this. 
  
  Because pg_resetxlog already does something very similar, so the patch
  is small. If it were independent, it would have to copy quite some code
  from pg_resetxlog.
 
 Aha.  In that case, it seems like it's time to rename pg_resetxlog, if
 it does a bunch of things that aren't resetting the xlog.

Well, all those actually do write to the xlog (to write a new
checkpoint, containing the updated control file). Since pg_resetxlog has
done all this pretty much since forever renaming it now seems to be a
big hassle for users for pretty much no benefit? This isn't a tool the
average user should ever touch.

Greetings,

Andres Freund

-- 
 Andres Freund 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] Set new system identifier using pg_resetxlog

2014-06-18 Thread Josh Berkus 
On 06/18/2014 11:03 AM, Andres Freund wrote:
 Well, all those actually do write to the xlog (to write a new
 checkpoint, containing the updated control file). Since pg_resetxlog has
 done all this pretty much since forever renaming it now seems to be a
 big hassle for users for pretty much no benefit? This isn't a tool the
 average user should ever touch.

If we're using it to create a unique system ID which can be used to
orchestrate replication and clustering systems, a lot more people are
going to be touching it than ever did before -- and not just for BDR.

Or are you saying that we have to destroy the data by resetting the xlog
before we can change the system identifier?  If so, this feature is less
than completely useful ...

-- 
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com


-- 
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] Set new system identifier using pg_resetxlog

2014-06-17 Thread Robert Haas 
On Fri, Jun 13, 2014 at 8:31 PM, Petr Jelinek p...@2ndquadrant.com wrote:
 attached is a simple patch which makes it possible to change the system
 identifier of the cluster in pg_control. This is useful for
 individualization of the instance that is started on top of data directory
 produced by pg_basebackup - something that's helpful for logical replication
 setup where you need to easily identify each node (it's used by
 Bidirectional Replication for example).

I can clearly understand the utility of being able to reset the system
ID to a new, randomly-generated system ID - but giving the user the
ability to set a particular value of their own choosing seems like a
pretty sharp tool.  What is the use case for that?

-- 
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

Re: [HACKERS] Set new system identifier using pg_resetxlog

2014-06-17 Thread Petr Jelinek 

On 17/06/14 16:18, Robert Haas wrote:

On Fri, Jun 13, 2014 at 8:31 PM, Petr Jelinek p...@2ndquadrant.com wrote:

attached is a simple patch which makes it possible to change the system
identifier of the cluster in pg_control. This is useful for
individualization of the instance that is started on top of data directory
produced by pg_basebackup - something that's helpful for logical replication
setup where you need to easily identify each node (it's used by
Bidirectional Replication for example).


I can clearly understand the utility of being able to reset the system
ID to a new, randomly-generated system ID - but giving the user the
ability to set a particular value of their own choosing seems like a
pretty sharp tool.  What is the use case for that?



Let's say you want to initialize new logical replication node via 
pg_basebackup and you want your replication slots to be easily 
identifiable so you use your local system id as part of the slot name.


In that case you need to know the future system id of the node because 
you need to register the slot before consistent point to which you 
replay via streaming replication (and you can't replay anymore once you 
changed the system id). Which means you need to generate your system id 
in advance and be able to change it in pg_control later.


--
 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] Set new system identifier using pg_resetxlog

2014-06-17 Thread Robert Haas 
On Tue, Jun 17, 2014 at 10:33 AM, Petr Jelinek p...@2ndquadrant.com wrote:
 On 17/06/14 16:18, Robert Haas wrote:
 On Fri, Jun 13, 2014 at 8:31 PM, Petr Jelinek p...@2ndquadrant.com
 wrote:
 attached is a simple patch which makes it possible to change the system
 identifier of the cluster in pg_control. This is useful for
 individualization of the instance that is started on top of data
 directory
 produced by pg_basebackup - something that's helpful for logical
 replication
 setup where you need to easily identify each node (it's used by
 Bidirectional Replication for example).


 I can clearly understand the utility of being able to reset the system
 ID to a new, randomly-generated system ID - but giving the user the
 ability to set a particular value of their own choosing seems like a
 pretty sharp tool.  What is the use case for that?

 Let's say you want to initialize new logical replication node via
 pg_basebackup and you want your replication slots to be easily identifiable
 so you use your local system id as part of the slot name.

 In that case you need to know the future system id of the node because you
 need to register the slot before consistent point to which you replay via
 streaming replication (and you can't replay anymore once you changed the
 system id). Which means you need to generate your system id in advance and
 be able to change it in pg_control later.

Hmm.  I guess that makes sense.

But it seems to me that we might need to have a process discussion
here, because, while I'm all in favor of incremental feature proposals
that build towards a larger goal, it currently appears that the larger
goal toward which you are building is not something that's been
publicly discussed and debated on this list.  And I really think we
need to have that conversation.  Obviously, individual patches will
still need to be debated, but I feel like 2ndQuadrant is trying to
construct a castle without showing the community the floor plan.  I
believe that there is relatively broad agreement that we would all
like a castle, but different people may have legitimately different
ideas about how it should be constructed.  If the work arrives as a
series of disconnected pieces (user-specified system ID, event
triggers for CREATE, etc.), then everyone outside of 2ndQuadrant has
to take it on faith that those pieces are going to eventually fit
together in a way that we'll all be happy with.  In some cases, that's
fine, because the feature is useful on its own merits whether it ends
up being part of the castle or not.

But in other cases, like this one, if the premise that the slot name
should match the system identifier isn't something the community wants
to accept, then taking a patch that lets people do that is probably a
bad idea, because at least one person will use it to set the system
identifier of a system to a value that enables physical replication to
take place when that is actually totally unsafe, and we don't want to
enable that for no reason.  Maybe the slot name should match the
replication identifier rather than the standby system ID, for example.
There are conflicting proposals for how replication identifiers should
work, but one of those proposals limits it to 16 bits.  If we're going
to have multiple identifiers floating around anyway, I'd rather have a
slot called 7 than one called 6024402925054484590.  On the other hand,
maybe there's going to be a new proposal to use the database system
identifier as a replication identifier, which might be a fine idea and
which would demolish that argument.

-- 
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

Re: [HACKERS] Set new system identifier using pg_resetxlog

2014-06-17 Thread Andres Freund 
On 2014-06-17 12:07:04 -0400, Robert Haas wrote:
 On Tue, Jun 17, 2014 at 10:33 AM, Petr Jelinek p...@2ndquadrant.com wrote:
  On 17/06/14 16:18, Robert Haas wrote:
  On Fri, Jun 13, 2014 at 8:31 PM, Petr Jelinek p...@2ndquadrant.com
  wrote:
  attached is a simple patch which makes it possible to change the system
  identifier of the cluster in pg_control. This is useful for
  individualization of the instance that is started on top of data
  directory
  produced by pg_basebackup - something that's helpful for logical
  replication
  setup where you need to easily identify each node (it's used by
  Bidirectional Replication for example).
 
 
  I can clearly understand the utility of being able to reset the system
  ID to a new, randomly-generated system ID - but giving the user the
  ability to set a particular value of their own choosing seems like a
  pretty sharp tool.  What is the use case for that?

I've previously hacked this up adhoc during data recovery when I needed
to make another cluster similar enough that I could replay WAL.

Another usecase is to mark a database as independent from its
origin. Imagine a database that gets sharded across several
servers. It's not uncommon to do that by initially basebackup'ing the
database to several nodes and then use them separately from
thereon. It's quite useful to actually mark them as being
distinct. Especially as several of them right now would end up with the
same timeline id...

 But it seems to me that we might need to have a process discussion
 here, because, while I'm all in favor of incremental feature proposals
 that build towards a larger goal, it currently appears that the larger
 goal toward which you are building is not something that's been
 publicly discussed and debated on this list.  And I really think we
 need to have that conversation.  Obviously, individual patches will
 still need to be debated, but I feel like 2ndQuadrant is trying to
 construct a castle without showing the community the floor plan.  I
 believe that there is relatively broad agreement that we would all
 like a castle, but different people may have legitimately different
 ideas about how it should be constructed.  If the work arrives as a
 series of disconnected pieces (user-specified system ID, event
 triggers for CREATE, etc.), then everyone outside of 2ndQuadrant has
 to take it on faith that those pieces are going to eventually fit
 together in a way that we'll all be happy with.  In some cases, that's
 fine, because the feature is useful on its own merits whether it ends
 up being part of the castle or not.


Uh. Right now this patch has been written because it's needed for a out
of core replication solution. That's what BDR is at this point. The
patch is unobtrusive, has other usecases than just our internal one and
doesn't make pg_resetxlog even more dangerous than it already is. I
don't see much problem with considering it on it's own cost/benefit?

So this seems to be a concern that's relatively independent of this
patch. Am I seing that right?

I think one very important point here is that BDR is *not* the proposed
in core solution. I think a reasonable community perspective - besides
also being useful on it's own - is to view it as a *prototype* for a in
core solution. And e.g. logical decoding would have looked much worse -
and likely not have been integrated - without externally already being
used for BDR.

I'm not sure how we can ease or even resolve your conerns when talking
about pretty independent and general pieces of functionality like the
DDL even trigger stuff. We needed to actually *write* those to see how
BDR will look like. And the communities feedback heavily influenced how
BDR looks like by accepting some pieces, demanding others, and outright
rejecting the remainder.

I think there's some pieces that need to consider them on their own
merit. Logical decoding is useful on it's own. The ability for out of
core systems to do DDL replication is another piece (that you referred
to above).
I think the likelihood of success if we were to try to design a in-core
system from ground up first and then follow through prety exactly along
those lines is minimal.

So, what I think we can do is to continue trying to build independent,
generally useful bits. Which imo all the stuff that's been integrated
is. Then, somewhat soon I think, we'll have to come up with a proposal
how the parts that are *not* necessarily useful outside of in-core
logical rep. might look like. Which will likely trigger some long long
discussions that turn that design around a couple of times. Which is
fine. I *don't* think that's going to be a trimmed down version of
todays BDR.

 But in other cases, like this one, if the premise that the slot name
 should match the system identifier isn't something the community wants
 to accept, then taking a patch that lets people do that is probably a
 bad idea, because at least one person will use it to set the system
 identifier of a system