Re: [HACKERS] Set new system identifier using pg_resetxlog
On Mon, Aug 25, 2014 at 4:06 PM, Heikki Linnakangas 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
On 08/25/2014 10:45 PM, Tom Lane wrote: Heikki Linnakangas 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
On August 25, 2014 9:45:50 PM CEST, Tom Lane wrote: >Heikki Linnakangas 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
Heikki Linnakangas 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
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 . 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
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
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
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 .\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 -m mxid,mxid -O mxoff -l xlogfile + -s [sysid] datadir @@ -184,6 +185,13 @@ PostgreSQL documentation + The -s option instructs pg_resetxlog to + set the system identifier of the cluster to specified sysid. + If the sysid is not provided new one will be generated using + same algorithm initdb uses. + + + The -V and --version options print the pg_resetxlog version and exit. The options -? and --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, &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 \"%s\" for option -s\n"), progname, optarg); + fprintf(stderr, _("Try \"
Re: [HACKERS] Set new system identifier using pg_resetxlog
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
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
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 .\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
On Tue, Jul 1, 2014 at 11:19 AM, Andres Freund 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
Robert Haas wrote: > On Mon, Jun 30, 2014 at 12:46 PM, Alvaro Herrera > 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
On 2014-07-01 11:11:12 -0400, Robert Haas wrote: > On Mon, Jun 30, 2014 at 12:46 PM, Alvaro Herrera > 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
On Mon, Jun 30, 2014 at 12:46 PM, Alvaro Herrera 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
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
On 2014-06-30 19:57:58 +0900, Fujii Masao wrote: > On Sun, Jun 29, 2014 at 11:18 PM, Andres Freund > 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
On Sun, Jun 29, 2014 at 11:18 PM, Andres Freund 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
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
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
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 -m mxid,mxid -O mxoff -l xlogfile + -s [sysid] datadir @@ -184,6 +185,13 @@ PostgreSQL documentation + The -s option instructs pg_resetxlog to + set the system identifier of the cluster to specified sysid. + If the sysid is not provided new one will be generated using + same algorithm initdb uses. + + + The -V and --version options print the pg_resetxlog version and exit. The options -? and --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; - ControlFile.system_identifier = sysidentifier; + ControlFile.system_identifier = GenerateSystemIdentifier(); ControlFile.checkPointCopy.redo = SizeOfXLogLongPHD; ControlFile.checkPointCopy.ThisTimeLineID = 1; @@ -649,6 +694,21 @@ PrintNewControlValues() XLogFileName(fname, ControlFile.checkPointCopy.ThisTimeLineID, newXlogSegNo); printf(_("First log segment after reset:%s\n"), fname); + if (set_sysid != 0) + { + char sysident_str[32]; + + /* + * Forma
Re: [HACKERS] Set new system identifier using pg_resetxlog
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 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
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 -m mxid,mxid -O mxoff -l xlogfile + -s [sysid] datadir @@ -184,6 +185,13 @@ PostgreSQL documentation + The -s option instructs pg_resetxlog to + set the system identifier of the cluster to specified sysid. + If the sysid is not provided new one will be generated using + same algorithm initdb uses. + + + The -V and --version options print the pg_resetxlog version and exit. The options -? and --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 pg_control values. */ @@ -489,14 +534,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; - ControlFile.system_identifier = sysidentifier; + ControlFile.system
Re: [HACKERS] Set new system identifier using pg_resetxlog
On Thu, Jun 26, 2014 at 2:43 AM, Sawada Masahiko 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
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 wrote: > On 18/06/14 19:26, Robert Haas wrote: > >> On Wed, Jun 18, 2014 at 12:54 PM, Andres Freund >> 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
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
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
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
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
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
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
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
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
On 18/06/14 19:26, Robert Haas wrote: On Wed, Jun 18, 2014 at 12:54 PM, Andres Freund 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
On Wed, Jun 18, 2014 at 12:54 PM, Andres Freund 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
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
On 2014-06-18 12:36:13 -0400, Robert Haas wrote: > On Tue, Jun 17, 2014 at 12:50 PM, Andres Freund > 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
On Tue, Jun 17, 2014 at 12:50 PM, Andres Freund 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
On 2014-06-17 12:07:04 -0400, Robert Haas wrote: > On Tue, Jun 17, 2014 at 10:33 AM, Petr Jelinek wrote: > > On 17/06/14 16:18, Robert Haas wrote: > >> On Fri, Jun 13, 2014 at 8:31 PM, Petr Jelinek > >> 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
Re: [HACKERS] Set new system identifier using pg_resetxlog
On Tue, Jun 17, 2014 at 10:33 AM, Petr Jelinek wrote: > On 17/06/14 16:18, Robert Haas wrote: >> On Fri, Jun 13, 2014 at 8:31 PM, Petr Jelinek >> 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
On 17/06/14 16:18, Robert Haas wrote: On Fri, Jun 13, 2014 at 8:31 PM, Petr Jelinek 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
On Fri, Jun 13, 2014 at 8:31 PM, Petr Jelinek 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