Re: [HACKERS] xlog location arithmetic
On Sat, Apr 14, 2012 at 5:30 AM, Robert Haas robertmh...@gmail.com wrote: On Mon, Mar 12, 2012 at 10:34 PM, Fujii Masao masao.fu...@gmail.com wrote: I can see a usecase for having a pg_size_pretty(numeric) as an option. Not necessarily a very big one, but a 0 one. +1. +1, too. I did some beautification of this patch. I think the attached version is cleaner and easier to read. Thoughts? Looks good to me. Thanks for polishing the patch! 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] xlog location arithmetic
On Sat, Apr 14, 2012 at 7:25 AM, Fujii Masao masao.fu...@gmail.com wrote: On Sat, Apr 14, 2012 at 5:30 AM, Robert Haas robertmh...@gmail.com wrote: On Mon, Mar 12, 2012 at 10:34 PM, Fujii Masao masao.fu...@gmail.com wrote: I can see a usecase for having a pg_size_pretty(numeric) as an option. Not necessarily a very big one, but a 0 one. +1. +1, too. I did some beautification of this patch. I think the attached version is cleaner and easier to read. Thoughts? Looks good to me. Thanks for polishing the patch! You're welcome. Committed. -- 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] xlog location arithmetic
On Mon, Mar 12, 2012 at 10:34 PM, Fujii Masao masao.fu...@gmail.com wrote: I can see a usecase for having a pg_size_pretty(numeric) as an option. Not necessarily a very big one, but a 0 one. +1. +1, too. I did some beautification of this patch. I think the attached version is cleaner and easier to read. Thoughts? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company sizepretty_v4.patch Description: Binary data -- 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] xlog location arithmetic
On Fri, Mar 09, 2012 at 03:04:23PM -0500, Tom Lane wrote: The main actual simplification would be in getting rid of the hole at the end of each 4GB worth of WAL, cf this bit in xlog_internal.h: /* * We break each logical log file (xlogid value) into segment files of the * size indicated by XLOG_SEG_SIZE. One possible segment at the end of each * log file is wasted, to ensure that we don't have problems representing * last-byte-position-plus-1. */ #define XLogSegSize ((uint32) XLOG_SEG_SIZE) #define XLogSegsPerFile (((uint32) 0x) / XLogSegSize) #define XLogFileSize (XLogSegsPerFile * XLogSegSize) If we can't get rid of that and have a continuous 64-bit WAL address space then it's unlikely we can actually simplify any logic. Now, doing that doesn't break the naming convention exactly; what it changes is that there will be WAL files numbered xxx (for some number of trailing-1-bits I'm too lazy to work out at the moment) where before there were not. So the question really is how much external code there is that is aware of that specific noncontiguous numbering behavior and would be broken if things stopped being that way. Our current WAL naming is hopelessly arcane, and we would certainly be benfitting users to simplify it. Is this a TODO? -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- 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] xlog location arithmetic
On Sat, Mar 10, 2012 at 3:23 AM, Robert Haas robertmh...@gmail.com wrote: On Fri, Mar 9, 2012 at 12:38 PM, Magnus Hagander mag...@hagander.net wrote: On Fri, Mar 9, 2012 at 18:18, Tom Lane t...@sss.pgh.pa.us wrote: Magnus Hagander mag...@hagander.net writes: On Fri, Mar 9, 2012 at 15:37, Tom Lane t...@sss.pgh.pa.us wrote: Why would it be useful to use pg_size_pretty on xlog locations? -1 because of the large expense of bigint-numeric-whatever conversion that would be added to existing uses. Given the expense, perhaps we need to different (overloaded) functions instead? Agreed. Attached patch introduces the overloaded funtion pg_size_pretty(numeric). That would be a workable solution, but I continue to not believe that this is useful enough to be worth the trouble. There's certainly some use to being able to prettify it. Wouldn't a pg_size_pretty(numeric) also be useful if you want to pg_size_() a sum() of something? Used on files it doesn't make too much sense, given how big those files have to be, but it can be used on other things as well... I can see a usecase for having a pg_size_pretty(numeric) as an option. Not necessarily a very big one, but a 0 one. +1. +1, too. Regards, -- Fujii Masao NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center *** a/doc/src/sgml/func.sgml --- b/doc/src/sgml/func.sgml *** *** 14989,14995 postgres=# SELECT * FROM pg_xlogfile_name_offset(pg_stop_backup()); /row row entry ! literalfunctionpg_size_pretty(typebigint/type)/function/literal /entry entrytypetext/type/entry entryConverts a size in bytes into a human-readable format with size units/entry --- 14989,14995 /row row entry ! literalfunctionpg_size_pretty(typebigint/type or typenumeric/type)/function/literal /entry entrytypetext/type/entry entryConverts a size in bytes into a human-readable format with size units/entry *** a/src/backend/utils/adt/dbsize.c --- b/src/backend/utils/adt/dbsize.c *** *** 24,29 --- 24,30 #include storage/fd.h #include utils/acl.h #include utils/builtins.h + #include utils/numeric.h #include utils/rel.h #include utils/relmapper.h #include utils/syscache.h *** *** 550,555 pg_size_pretty(PG_FUNCTION_ARGS) --- 551,652 PG_RETURN_TEXT_P(cstring_to_text(buf)); } + Datum + pg_size_pretty_numeric(PG_FUNCTION_ARGS) + { + Numeric size = PG_GETARG_NUMERIC(0); + Numeric limit, limit2; + char *buf, *result; + + limit = DatumGetNumeric(DirectFunctionCall1(int8_numeric, Int64GetDatum((int64) (10 * 1024; + limit2 = DatumGetNumeric(DirectFunctionCall1(int8_numeric, Int64GetDatum((int64) (10 * 1024 * 2 - 1; + + if (DatumGetBool(DirectFunctionCall2(numeric_lt, NumericGetDatum(size), NumericGetDatum(limit + { + buf = DatumGetCString(DirectFunctionCall1(numeric_out, NumericGetDatum(size))); + result = palloc(strlen(buf) + 7); + strcpy(result, buf); + strcat(result, bytes); + } + else + { + Numeric arg2; + + /* keep one extra bit for rounding */ + /* size = 9 */ + arg2 = DatumGetNumeric(DirectFunctionCall1(int8_numeric, Int64GetDatum((int64) pow(2, 9; + size = DatumGetNumeric(DirectFunctionCall2(numeric_div_trunc, NumericGetDatum(size), NumericGetDatum(arg2))); + + if (DatumGetBool(DirectFunctionCall2(numeric_lt, NumericGetDatum(size), NumericGetDatum(limit2 + { + /* size = (size + 1) / 2 */ + size = DatumGetNumeric(DirectFunctionCall2(numeric_add, NumericGetDatum(size), + DirectFunctionCall1(int8_numeric, Int64GetDatum(1; + size = DatumGetNumeric(DirectFunctionCall2(numeric_div_trunc, NumericGetDatum(size), + DirectFunctionCall1(int8_numeric, Int64GetDatum(2; + buf = DatumGetCString(DirectFunctionCall1(numeric_out, NumericGetDatum(size))); + result = palloc(strlen(buf) + 4); + strcpy(result, buf); + strcat(result, kB); + } + else + { + Numeric arg3; + + /* size = 10 */ + arg3 = DatumGetNumeric(DirectFunctionCall1(int8_numeric, Int64GetDatum((int64) pow(2, 10; + size = DatumGetNumeric(DirectFunctionCall2(numeric_div_trunc, NumericGetDatum(size), NumericGetDatum(arg3))); + + if (DatumGetBool(DirectFunctionCall2(numeric_lt, NumericGetDatum(size), NumericGetDatum(limit2 + { + /* size = (size + 1) / 2 */ + size = DatumGetNumeric(DirectFunctionCall2(numeric_add, NumericGetDatum(size), + DirectFunctionCall1(int8_numeric, Int64GetDatum(1; + size = DatumGetNumeric(DirectFunctionCall2(numeric_div_trunc, NumericGetDatum(size), + DirectFunctionCall1(int8_numeric, Int64GetDatum(2; + buf = DatumGetCString(DirectFunctionCall1(numeric_out, NumericGetDatum(size))); + result = palloc(strlen(buf) + 4); + strcpy(result, buf); + strcat(result, MB); + }
Re: [HACKERS] xlog location arithmetic
On Sat, Mar 10, 2012 at 5:04 AM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Fri, Mar 9, 2012 at 2:34 PM, Robert Haas robertmh...@gmail.com wrote: On Fri, Mar 9, 2012 at 2:26 PM, Tom Lane t...@sss.pgh.pa.us wrote: Hm. I think thousands is an overestimate, but yeah the logic could be greatly simplified. However, I'm not sure we could avoid breaking the existing naming convention for WAL files. How much do we care about that? Probably not very much, since WAL files aren't portable across major versions anyway. But I don't see why you couldn't keep the naming convention - there's nothing to prevent you from converting a 64-bit integer back into two 32-bit integers if and where needed. On further reflection, this seems likely to break quite a few third-party tools. Maybe it'd be worth it anyway, but it definitely seems like it would be worth going to at least some minor trouble to avoid it. The main actual simplification would be in getting rid of the hole at the end of each 4GB worth of WAL, cf this bit in xlog_internal.h: /* * We break each logical log file (xlogid value) into segment files of the * size indicated by XLOG_SEG_SIZE. One possible segment at the end of each * log file is wasted, to ensure that we don't have problems representing * last-byte-position-plus-1. */ #define XLogSegSize ((uint32) XLOG_SEG_SIZE) #define XLogSegsPerFile (((uint32) 0x) / XLogSegSize) #define XLogFileSize (XLogSegsPerFile * XLogSegSize) If we can't get rid of that and have a continuous 64-bit WAL address space then it's unlikely we can actually simplify any logic. Now, doing that doesn't break the naming convention exactly; what it changes is that there will be WAL files numbered xxx (for some number of trailing-1-bits I'm too lazy to work out at the moment) where before there were not. So the question really is how much external code there is that is aware of that specific noncontiguous numbering behavior and would be broken if things stopped being that way. A page header contains WAL location, so getting rid of hole seems to break pg_upgrade. No? Unless pg_upgrade converts noncontinuous location to continuous one, we still need to handle noncontinuous one after upgrade. Regards, -- Fujii Masao NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center -- 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] xlog location arithmetic
Fujii Masao masao.fu...@gmail.com writes: On Sat, Mar 10, 2012 at 5:04 AM, Tom Lane t...@sss.pgh.pa.us wrote: The main actual simplification would be in getting rid of the hole at the end of each 4GB worth of WAL, cf this bit in xlog_internal.h: If we can't get rid of that and have a continuous 64-bit WAL address space then it's unlikely we can actually simplify any logic. ... Now, doing that doesn't break the naming convention exactly; what it changes is that there will be WAL files numbered xxx (for some number of trailing-1-bits I'm too lazy to work out at the moment) where before there were not. So the question really is how much external code there is that is aware of that specific noncontiguous numbering behavior and would be broken if things stopped being that way. A page header contains WAL location, so getting rid of hole seems to break pg_upgrade. No? No, why would it do that? The meaning and ordering of WAL addresses is the same as before. The only difference is that after the upgrade, the system will stop skipping over 16MB of potentially usable WAL addresses at the end of each subsequently-used 4GB of space. The holes before the switchover point are still holes, but that doesn't matter. 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] xlog location arithmetic
On Tue, Mar 13, 2012 at 12:03 PM, Tom Lane t...@sss.pgh.pa.us wrote: Fujii Masao masao.fu...@gmail.com writes: On Sat, Mar 10, 2012 at 5:04 AM, Tom Lane t...@sss.pgh.pa.us wrote: The main actual simplification would be in getting rid of the hole at the end of each 4GB worth of WAL, cf this bit in xlog_internal.h: If we can't get rid of that and have a continuous 64-bit WAL address space then it's unlikely we can actually simplify any logic. ... Now, doing that doesn't break the naming convention exactly; what it changes is that there will be WAL files numbered xxx (for some number of trailing-1-bits I'm too lazy to work out at the moment) where before there were not. So the question really is how much external code there is that is aware of that specific noncontiguous numbering behavior and would be broken if things stopped being that way. A page header contains WAL location, so getting rid of hole seems to break pg_upgrade. No? No, why would it do that? The meaning and ordering of WAL addresses is the same as before. The only difference is that after the upgrade, the system will stop skipping over 16MB of potentially usable WAL addresses at the end of each subsequently-used 4GB of space. The holes before the switchover point are still holes, but that doesn't matter. Oh, I see. You're right. Regards, -- Fujii Masao NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center -- 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] xlog location arithmetic
On Sun, Mar 4, 2012 at 8:26 PM, Magnus Hagander mag...@hagander.net wrote: On Sun, Feb 26, 2012 at 00:53, Euler Taveira de Oliveira eu...@timbira.com wrote: On 25-02-2012 09:23, Magnus Hagander wrote: Do we even *need* the validate_xlog_location() function? If we just remove those calls, won't we still catch all the incorrectly formatted ones in the errors of the sscanf() calls? Or am I too deep into weekend-mode and missing something obvious? sscanf() is too fragile for input sanity check. Try pg_xlog_location_diff('12/3', '-10/0'), for example. I won't object removing that function if you protect xlog location input from silly users. Ah, good point. No, that's the reason I was missing :-) Patch applied, thanks! Thanks for committing the patch! Euler proposed one more patch upthread, which replaces pg_size_pretty(bigint) with pg_size_pretty(numeric) so that pg_size_pretty(pg_xlog_location_diff()) succeeds. It's also worth committing this patch? http://archives.postgresql.org/message-id/4f315f6c.8030...@timbira.com Regards, -- Fujii Masao NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center -- 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] xlog location arithmetic
Fujii Masao masao.fu...@gmail.com writes: Euler proposed one more patch upthread, which replaces pg_size_pretty(bigint) with pg_size_pretty(numeric) so that pg_size_pretty(pg_xlog_location_diff()) succeeds. It's also worth committing this patch? Why would it be useful to use pg_size_pretty on xlog locations? -1 because of the large expense of bigint-numeric-whatever conversion that would be added to existing uses. 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] xlog location arithmetic
On Fri, Mar 9, 2012 at 9:37 AM, Tom Lane t...@sss.pgh.pa.us wrote: Fujii Masao masao.fu...@gmail.com writes: Euler proposed one more patch upthread, which replaces pg_size_pretty(bigint) with pg_size_pretty(numeric) so that pg_size_pretty(pg_xlog_location_diff()) succeeds. It's also worth committing this patch? Why would it be useful to use pg_size_pretty on xlog locations? -1 because of the large expense of bigint-numeric-whatever conversion that would be added to existing uses. The point is that it would be useful to use it on the difference between two xlog locations, but that is a numeric value, not int8, because of signedness issues. -- 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] xlog location arithmetic
I wrote: Fujii Masao masao.fu...@gmail.com writes: Euler proposed one more patch upthread, which replaces pg_size_pretty(bigint) with pg_size_pretty(numeric) so that pg_size_pretty(pg_xlog_location_diff()) succeeds. It's also worth committing this patch? Why would it be useful to use pg_size_pretty on xlog locations? -1 because of the large expense of bigint-numeric-whatever conversion that would be added to existing uses. Actually ... now that I look at it, isn't it completely bogus to be using numeric for the result of pg_xlog_location_diff? There's no way for the difference of two xlog locations to be anywhere near as wide as 64 bits. That'd only be possible if XLogFileSize exceeded 1GB, which we don't let it get anywhere near. 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] xlog location arithmetic
Robert Haas robertmh...@gmail.com writes: On Fri, Mar 9, 2012 at 9:37 AM, Tom Lane t...@sss.pgh.pa.us wrote: Why would it be useful to use pg_size_pretty on xlog locations? The point is that it would be useful to use it on the difference between two xlog locations, Um, that is exactly the claim I was questioning. Why is that useful? but that is a numeric value, not int8, because of signedness issues. See my followup --- this statement appears factually incorrect, whatever you may feel about the usefulness issue. 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] xlog location arithmetic
On Fri, Mar 9, 2012 at 9:55 AM, Tom Lane t...@sss.pgh.pa.us wrote: I wrote: Fujii Masao masao.fu...@gmail.com writes: Euler proposed one more patch upthread, which replaces pg_size_pretty(bigint) with pg_size_pretty(numeric) so that pg_size_pretty(pg_xlog_location_diff()) succeeds. It's also worth committing this patch? Why would it be useful to use pg_size_pretty on xlog locations? -1 because of the large expense of bigint-numeric-whatever conversion that would be added to existing uses. Actually ... now that I look at it, isn't it completely bogus to be using numeric for the result of pg_xlog_location_diff? There's no way for the difference of two xlog locations to be anywhere near as wide as 64 bits. That'd only be possible if XLogFileSize exceeded 1GB, which we don't let it get anywhere near. rhaas=# select pg_xlog_location_diff('/0', '0/0'); pg_xlog_location_diff --- 18374686475393433600 (1 row) rhaas=# select pg_xlog_location_diff('/0', '0/0')::int8; ERROR: bigint out of range STATEMENT: select pg_xlog_location_diff('/0', '0/0')::int8; -- 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] xlog location arithmetic
Robert Haas robertmh...@gmail.com writes: On Fri, Mar 9, 2012 at 9:55 AM, Tom Lane t...@sss.pgh.pa.us wrote: Actually ... now that I look at it, isn't it completely bogus to be using numeric for the result of pg_xlog_location_diff? rhaas=# select pg_xlog_location_diff('/0', '0/0')::int8; ERROR: bigint out of range Oh ... I see my mistake. I was looking at this: /* * result = XLogFileSize * (xlogid1 - xlogid2) + xrecoff1 - xrecoff2 */ and confusing XLogFileSize with XLogSegSize. Not the best choice of names. 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] xlog location arithmetic
On Fri, Mar 9, 2012 at 16:37, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Fri, Mar 9, 2012 at 9:55 AM, Tom Lane t...@sss.pgh.pa.us wrote: Actually ... now that I look at it, isn't it completely bogus to be using numeric for the result of pg_xlog_location_diff? rhaas=# select pg_xlog_location_diff('/0', '0/0')::int8; ERROR: bigint out of range Oh ... I see my mistake. I was looking at this: /* * result = XLogFileSize * (xlogid1 - xlogid2) + xrecoff1 - xrecoff2 */ and confusing XLogFileSize with XLogSegSize. Not the best choice of names. Yeah, the use of XLogFile to mean something other than, well a file in the xlog, is greatly annoying.. I guess we could change it, but it goes pretty deep in the system so it's not a small change... -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.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] xlog location arithmetic
On Fri, Mar 9, 2012 at 15:37, Tom Lane t...@sss.pgh.pa.us wrote: Fujii Masao masao.fu...@gmail.com writes: Euler proposed one more patch upthread, which replaces pg_size_pretty(bigint) with pg_size_pretty(numeric) so that pg_size_pretty(pg_xlog_location_diff()) succeeds. It's also worth committing this patch? Why would it be useful to use pg_size_pretty on xlog locations? -1 because of the large expense of bigint-numeric-whatever conversion that would be added to existing uses. Given the expense, perhaps we need to different (overloaded) functions instead? -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.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] xlog location arithmetic
Magnus Hagander mag...@hagander.net writes: On Fri, Mar 9, 2012 at 15:37, Tom Lane t...@sss.pgh.pa.us wrote: Why would it be useful to use pg_size_pretty on xlog locations? -1 because of the large expense of bigint-numeric-whatever conversion that would be added to existing uses. Given the expense, perhaps we need to different (overloaded) functions instead? That would be a workable solution, but I continue to not believe that this is useful enough to be worth the trouble. 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] xlog location arithmetic
On Fri, Mar 9, 2012 at 18:18, Tom Lane t...@sss.pgh.pa.us wrote: Magnus Hagander mag...@hagander.net writes: On Fri, Mar 9, 2012 at 15:37, Tom Lane t...@sss.pgh.pa.us wrote: Why would it be useful to use pg_size_pretty on xlog locations? -1 because of the large expense of bigint-numeric-whatever conversion that would be added to existing uses. Given the expense, perhaps we need to different (overloaded) functions instead? That would be a workable solution, but I continue to not believe that this is useful enough to be worth the trouble. There's certainly some use to being able to prettify it. Wouldn't a pg_size_pretty(numeric) also be useful if you want to pg_size_() a sum() of something? Used on files it doesn't make too much sense, given how big those files have to be, but it can be used on other things as well... I can see a usecase for having a pg_size_pretty(numeric) as an option. Not necessarily a very big one, but a 0 one. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.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] xlog location arithmetic
On Fri, Mar 9, 2012 at 12:38 PM, Magnus Hagander mag...@hagander.net wrote: On Fri, Mar 9, 2012 at 18:18, Tom Lane t...@sss.pgh.pa.us wrote: Magnus Hagander mag...@hagander.net writes: On Fri, Mar 9, 2012 at 15:37, Tom Lane t...@sss.pgh.pa.us wrote: Why would it be useful to use pg_size_pretty on xlog locations? -1 because of the large expense of bigint-numeric-whatever conversion that would be added to existing uses. Given the expense, perhaps we need to different (overloaded) functions instead? That would be a workable solution, but I continue to not believe that this is useful enough to be worth the trouble. There's certainly some use to being able to prettify it. Wouldn't a pg_size_pretty(numeric) also be useful if you want to pg_size_() a sum() of something? Used on files it doesn't make too much sense, given how big those files have to be, but it can be used on other things as well... I can see a usecase for having a pg_size_pretty(numeric) as an option. Not necessarily a very big one, but a 0 one. +1. -- 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] xlog location arithmetic
On fre, 2012-03-09 at 18:13 +0100, Magnus Hagander wrote: and confusing XLogFileSize with XLogSegSize. Not the best choice of names. Yeah, the use of XLogFile to mean something other than, well a file in the xlog, is greatly annoying.. I guess we could change it, but it goes pretty deep in the system so it's not a small change... The whole thing was built around the lack of 64 bit integers. If we bit the bullet and changed the whole thing to be just a single 64-bit counter, we could probably delete thousands of lines of code. -- 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] xlog location arithmetic
Peter Eisentraut pete...@gmx.net writes: Yeah, the use of XLogFile to mean something other than, well a file in the xlog, is greatly annoying.. I guess we could change it, but it goes pretty deep in the system so it's not a small change... The whole thing was built around the lack of 64 bit integers. If we bit the bullet and changed the whole thing to be just a single 64-bit counter, we could probably delete thousands of lines of code. Hm. I think thousands is an overestimate, but yeah the logic could be greatly simplified. However, I'm not sure we could avoid breaking the existing naming convention for WAL files. How much do we care about that? 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] xlog location arithmetic
On Fri, Mar 9, 2012 at 2:26 PM, Tom Lane t...@sss.pgh.pa.us wrote: Peter Eisentraut pete...@gmx.net writes: Yeah, the use of XLogFile to mean something other than, well a file in the xlog, is greatly annoying.. I guess we could change it, but it goes pretty deep in the system so it's not a small change... The whole thing was built around the lack of 64 bit integers. If we bit the bullet and changed the whole thing to be just a single 64-bit counter, we could probably delete thousands of lines of code. Hm. I think thousands is an overestimate, but yeah the logic could be greatly simplified. However, I'm not sure we could avoid breaking the existing naming convention for WAL files. How much do we care about that? Probably not very much, since WAL files aren't portable across major versions anyway. But I don't see why you couldn't keep the naming convention - there's nothing to prevent you from converting a 64-bit integer back into two 32-bit integers if and where needed. -- 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] xlog location arithmetic
Tom Lane t...@sss.pgh.pa.us wrote: Peter Eisentraut pete...@gmx.net writes: The whole thing was built around the lack of 64 bit integers. If we bit the bullet and changed the whole thing to be just a single 64-bit counter, we could probably delete thousands of lines of code. Hm. I think thousands is an overestimate, but yeah the logic could be greatly simplified. However, I'm not sure we could avoid breaking the existing naming convention for WAL files. How much do we care about that? We have a few scripts in our backup area that are based around the current WAL file naming convention, so there would be some impact; but I have to believe it would be pretty minor. Most of the pain would be related to the need to support both naming conventions for some transition period. If it simplifies the WAL-related logic, it seems well worth it to me. We just have to know it's coming and be clear on what the new naming rules are. -Kevin -- 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] xlog location arithmetic
On Fri, Mar 9, 2012 at 2:34 PM, Robert Haas robertmh...@gmail.com wrote: On Fri, Mar 9, 2012 at 2:26 PM, Tom Lane t...@sss.pgh.pa.us wrote: Peter Eisentraut pete...@gmx.net writes: Yeah, the use of XLogFile to mean something other than, well a file in the xlog, is greatly annoying.. I guess we could change it, but it goes pretty deep in the system so it's not a small change... The whole thing was built around the lack of 64 bit integers. If we bit the bullet and changed the whole thing to be just a single 64-bit counter, we could probably delete thousands of lines of code. Hm. I think thousands is an overestimate, but yeah the logic could be greatly simplified. However, I'm not sure we could avoid breaking the existing naming convention for WAL files. How much do we care about that? Probably not very much, since WAL files aren't portable across major versions anyway. But I don't see why you couldn't keep the naming convention - there's nothing to prevent you from converting a 64-bit integer back into two 32-bit integers if and where needed. On further reflection, this seems likely to break quite a few third-party tools. Maybe it'd be worth it anyway, but it definitely seems like it would be worth going to at least some minor trouble to avoid 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] xlog location arithmetic
Robert Haas robertmh...@gmail.com writes: On Fri, Mar 9, 2012 at 2:34 PM, Robert Haas robertmh...@gmail.com wrote: On Fri, Mar 9, 2012 at 2:26 PM, Tom Lane t...@sss.pgh.pa.us wrote: Hm. I think thousands is an overestimate, but yeah the logic could be greatly simplified. However, I'm not sure we could avoid breaking the existing naming convention for WAL files. How much do we care about that? Probably not very much, since WAL files aren't portable across major versions anyway. But I don't see why you couldn't keep the naming convention - there's nothing to prevent you from converting a 64-bit integer back into two 32-bit integers if and where needed. On further reflection, this seems likely to break quite a few third-party tools. Maybe it'd be worth it anyway, but it definitely seems like it would be worth going to at least some minor trouble to avoid it. The main actual simplification would be in getting rid of the hole at the end of each 4GB worth of WAL, cf this bit in xlog_internal.h: /* * We break each logical log file (xlogid value) into segment files of the * size indicated by XLOG_SEG_SIZE. One possible segment at the end of each * log file is wasted, to ensure that we don't have problems representing * last-byte-position-plus-1. */ #define XLogSegSize ((uint32) XLOG_SEG_SIZE) #define XLogSegsPerFile (((uint32) 0x) / XLogSegSize) #define XLogFileSize(XLogSegsPerFile * XLogSegSize) If we can't get rid of that and have a continuous 64-bit WAL address space then it's unlikely we can actually simplify any logic. Now, doing that doesn't break the naming convention exactly; what it changes is that there will be WAL files numbered xxx (for some number of trailing-1-bits I'm too lazy to work out at the moment) where before there were not. So the question really is how much external code there is that is aware of that specific noncontiguous numbering behavior and would be broken if things stopped being that way. 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] xlog location arithmetic
On Fri, Mar 9, 2012 at 3:04 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Fri, Mar 9, 2012 at 2:34 PM, Robert Haas robertmh...@gmail.com wrote: On Fri, Mar 9, 2012 at 2:26 PM, Tom Lane t...@sss.pgh.pa.us wrote: Hm. I think thousands is an overestimate, but yeah the logic could be greatly simplified. However, I'm not sure we could avoid breaking the existing naming convention for WAL files. How much do we care about that? Probably not very much, since WAL files aren't portable across major versions anyway. But I don't see why you couldn't keep the naming convention - there's nothing to prevent you from converting a 64-bit integer back into two 32-bit integers if and where needed. On further reflection, this seems likely to break quite a few third-party tools. Maybe it'd be worth it anyway, but it definitely seems like it would be worth going to at least some minor trouble to avoid it. The main actual simplification would be in getting rid of the hole at the end of each 4GB worth of WAL, cf this bit in xlog_internal.h: /* * We break each logical log file (xlogid value) into segment files of the * size indicated by XLOG_SEG_SIZE. One possible segment at the end of each * log file is wasted, to ensure that we don't have problems representing * last-byte-position-plus-1. */ #define XLogSegSize ((uint32) XLOG_SEG_SIZE) #define XLogSegsPerFile (((uint32) 0x) / XLogSegSize) #define XLogFileSize (XLogSegsPerFile * XLogSegSize) If we can't get rid of that and have a continuous 64-bit WAL address space then it's unlikely we can actually simplify any logic. Now, doing that doesn't break the naming convention exactly; what it changes is that there will be WAL files numbered xxx (for some number of trailing-1-bits I'm too lazy to work out at the moment) where before there were not. So the question really is how much external code there is that is aware of that specific noncontiguous numbering behavior and would be broken if things stopped being that way. I would expect that most things would NOT know about that particular foible, and just be matching pathnames on an RE, which should be fine. -- 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] xlog location arithmetic
On Sun, Feb 26, 2012 at 00:53, Euler Taveira de Oliveira eu...@timbira.com wrote: On 25-02-2012 09:23, Magnus Hagander wrote: Do we even *need* the validate_xlog_location() function? If we just remove those calls, won't we still catch all the incorrectly formatted ones in the errors of the sscanf() calls? Or am I too deep into weekend-mode and missing something obvious? sscanf() is too fragile for input sanity check. Try pg_xlog_location_diff('12/3', '-10/0'), for example. I won't object removing that function if you protect xlog location input from silly users. Ah, good point. No, that's the reason I was missing :-) Patch applied, thanks! -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.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] xlog location arithmetic
On Tue, Feb 28, 2012 at 07:21, Fujii Masao masao.fu...@gmail.com wrote: On Sun, Feb 26, 2012 at 8:53 AM, Euler Taveira de Oliveira eu...@timbira.com wrote: On 25-02-2012 09:23, Magnus Hagander wrote: Do we even *need* the validate_xlog_location() function? If we just remove those calls, won't we still catch all the incorrectly formatted ones in the errors of the sscanf() calls? Or am I too deep into weekend-mode and missing something obvious? sscanf() is too fragile for input sanity check. Try pg_xlog_location_diff('12/3', '-10/0'), for example. I won't object removing that function if you protect xlog location input from silly users. After this patch will have been committed, it would be better to change pg_xlogfile_name() and pg_xlogfile_name_offset() so that they use the validate_xlog_location() function to validate the input. And I've done this part as well. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.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] xlog location arithmetic
On Sun, Feb 26, 2012 at 8:53 AM, Euler Taveira de Oliveira eu...@timbira.com wrote: On 25-02-2012 09:23, Magnus Hagander wrote: Do we even *need* the validate_xlog_location() function? If we just remove those calls, won't we still catch all the incorrectly formatted ones in the errors of the sscanf() calls? Or am I too deep into weekend-mode and missing something obvious? sscanf() is too fragile for input sanity check. Try pg_xlog_location_diff('12/3', '-10/0'), for example. I won't object removing that function if you protect xlog location input from silly users. After this patch will have been committed, it would be better to change pg_xlogfile_name() and pg_xlogfile_name_offset() so that they use the validate_xlog_location() function to validate the input. Regards, -- Fujii Masao NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center -- 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] xlog location arithmetic
On Fri, Feb 10, 2012 at 09:32, Fujii Masao masao.fu...@gmail.com wrote: On Fri, Feb 10, 2012 at 7:00 AM, Euler Taveira de Oliveira eu...@timbira.com wrote: On 08-02-2012 09:35, Fujii Masao wrote: Fujii, new patch attached. Thanks for your tests. Thanks for the new patch! But another problem happened. When I changed pg_proc.h so that the unused OID was assigned to pg_xlog_location_diff(), and executed the above again, I encountered the segmentation fault: I reproduced the problems in my old 32-bit laptop. I fixed it casting to int64. I also updated the duplicated OID. Yep, in the updated patch, I could confirm that the function works fine without any error in my machine. The patch looks fine to me except the following minor comments: I started working on this one to commit it, and came up with a few things more. Do we even *need* the validate_xlog_location() function? If we just remove those calls, won't we still catch all the incorrectly formatted ones in the errors of the sscanf() calls? Or am I too deep into weekend-mode and missing something obvious? I've also removed tabs in the documentation, fixed the merge confllict in pg_proc.h that happened during the wait, and fixed some indentation (updated patch with these changes attached). But I'm going to hold off committing it until someone confirms I'm not caught too deeply in weekend-mode and am missing something obvious in the comment above about validate_xlog_location. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml index e8e637b..4ae76e2 100644 --- a/doc/src/sgml/func.sgml +++ b/doc/src/sgml/func.sgml @@ -14454,11 +14454,15 @@ SELECT set_config('log_statement_stats', 'off', false); indexterm primarypg_xlogfile_name_offset/primary /indexterm + indexterm +primarypg_xlog_location_diff/primary + /indexterm para The functions shown in xref linkend=functions-admin-backup-table assist in making on-line backups. -These functions cannot be executed during recovery. +These functions cannot be executed during recovery (except +functionpg_xlog_location_diff/function). /para table id=functions-admin-backup-table @@ -14526,6 +14530,13 @@ SELECT set_config('log_statement_stats', 'off', false); entrytypetext/, typeinteger//entry entryConvert transaction log location string to file name and decimal byte offset within file/entry /row + row + entry +literalfunctionpg_xlog_location_diff(parameterlocation/ typetext/, parameterlocation/ typetext/)/function/literal + /entry + entrytypenumeric//entry + entryCalculate the difference between two transaction log locations/entry + /row /tbody /tgroup /table @@ -14619,6 +14630,13 @@ postgres=# SELECT * FROM pg_xlogfile_name_offset(pg_stop_backup()); /para para +functionpg_xlog_location_diff/ calculates the difference in bytes +between two transaction log locations. It can be used with +structnamepg_stat_replication/structname or some functions shown in +xref linkend=functions-admin-backup-table to get the replication lag. + /para + + para For details about proper usage of these functions, see xref linkend=continuous-archiving. /para diff --git a/src/backend/access/transam/xlogfuncs.c b/src/backend/access/transam/xlogfuncs.c index 2e10d4d..b8f8152 100644 --- a/src/backend/access/transam/xlogfuncs.c +++ b/src/backend/access/transam/xlogfuncs.c @@ -26,6 +26,7 @@ #include replication/walreceiver.h #include storage/smgr.h #include utils/builtins.h +#include utils/numeric.h #include utils/guc.h #include utils/timestamp.h @@ -465,3 +466,87 @@ pg_is_in_recovery(PG_FUNCTION_ARGS) { PG_RETURN_BOOL(RecoveryInProgress()); } + +static void +validate_xlog_location(char *str) +{ +#define MAXLSNCOMPONENT 8 + + int len1, +len2; + + len1 = strspn(str, 0123456789abcdefABCDEF); + if (len1 1 || len1 MAXLSNCOMPONENT || str[len1] != '/') + ereport(ERROR, +(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION), + errmsg(invalid input syntax for transaction log location: \%s\, str))); + + len2 = strspn(str + len1 + 1, 0123456789abcdefABCDEF); + if (len2 1 || len2 MAXLSNCOMPONENT || str[len1 + 1 + len2] != '\0') + ereport(ERROR, +(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION), + errmsg(invalid input syntax for transaction log location: \%s\, str))); +} + +/* + * Compute the difference in bytes between two WAL locations. + */ +Datum +pg_xlog_location_diff(PG_FUNCTION_ARGS) +{ + text *location1 = PG_GETARG_TEXT_P(0); + text *location2 = PG_GETARG_TEXT_P(1); + char *str1, + *str2; + XLogRecPtr loc1, +loc2; + Numeric result; + + /* + * Read and parse input + */ + str1 = text_to_cstring(location1); + str2 = text_to_cstring(location2); + + validate_xlog_location(str1); + validate_xlog_location(str2); + + if
Re: [HACKERS] xlog location arithmetic
On 25-02-2012 09:23, Magnus Hagander wrote: Do we even *need* the validate_xlog_location() function? If we just remove those calls, won't we still catch all the incorrectly formatted ones in the errors of the sscanf() calls? Or am I too deep into weekend-mode and missing something obvious? sscanf() is too fragile for input sanity check. Try pg_xlog_location_diff('12/3', '-10/0'), for example. I won't object removing that function if you protect xlog location input from silly users. -- Euler Taveira de Oliveira - Timbira http://www.timbira.com.br/ PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento -- 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] xlog location arithmetic
On Fri, Feb 10, 2012 at 7:00 AM, Euler Taveira de Oliveira eu...@timbira.com wrote: On 08-02-2012 09:35, Fujii Masao wrote: Fujii, new patch attached. Thanks for your tests. Thanks for the new patch! But another problem happened. When I changed pg_proc.h so that the unused OID was assigned to pg_xlog_location_diff(), and executed the above again, I encountered the segmentation fault: I reproduced the problems in my old 32-bit laptop. I fixed it casting to int64. I also updated the duplicated OID. Yep, in the updated patch, I could confirm that the function works fine without any error in my machine. The patch looks fine to me except the following minor comments: In the document, it's better to explain clearly that the function subtracts the second argument from the first. -These functions cannot be executed during recovery. + These functions cannot be executed during recovery (except + functionpg_xlog_location_diff/function). + functionpg_xlog_location_diff/ calculates the difference in bytes + between two transaction log locations. It can be used with + structnamepg_stat_replication/structname or some functions shown in + xref linkend=functions-admin-backup-table to get the replication lag. Very minor comment: you should use spaces rather than a tab to indent each line. Why OID needs to be reassigned? There isn't a compelling reason. It is just a way to say: hey, it is another function with the same old name. I'll not attach another version for pg_size_pretty because it is a matter of updating a duplicated OID. Okay, I reviewed the previous patch again. That looks fine to me. Regards, -- Fujii Masao NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center -- 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] xlog location arithmetic
On 08-02-2012 09:35, Fujii Masao wrote: Fujii, new patch attached. Thanks for your tests. But another problem happened. When I changed pg_proc.h so that the unused OID was assigned to pg_xlog_location_diff(), and executed the above again, I encountered the segmentation fault: I reproduced the problems in my old 32-bit laptop. I fixed it casting to int64. I also updated the duplicated OID. Why OID needs to be reassigned? There isn't a compelling reason. It is just a way to say: hey, it is another function with the same old name. I'll not attach another version for pg_size_pretty because it is a matter of updating a duplicated OID. -- Euler Taveira de Oliveira - Timbira http://www.timbira.com.br/ PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml index 236a60a..826f002 100644 --- a/doc/src/sgml/func.sgml +++ b/doc/src/sgml/func.sgml @@ -14446,11 +14446,15 @@ SELECT set_config('log_statement_stats', 'off', false); indexterm primarypg_xlogfile_name_offset/primary /indexterm + indexterm +primarypg_xlog_location_diff/primary + /indexterm para The functions shown in xref linkend=functions-admin-backup-table assist in making on-line backups. -These functions cannot be executed during recovery. + These functions cannot be executed during recovery (except + functionpg_xlog_location_diff/function). /para table id=functions-admin-backup-table @@ -14518,6 +14522,13 @@ SELECT set_config('log_statement_stats', 'off', false); entrytypetext/, typeinteger//entry entryConvert transaction log location string to file name and decimal byte offset within file/entry /row + row + entry +literalfunctionpg_xlog_location_diff(parameterlocation/ typetext/, parameterlocation/ typetext/)/function/literal +/entry + entrytypenumeric//entry + entryCalculate the difference between two transaction log locations/entry + /row /tbody /tgroup /table @@ -14611,6 +14622,13 @@ postgres=# SELECT * FROM pg_xlogfile_name_offset(pg_stop_backup()); /para para + functionpg_xlog_location_diff/ calculates the difference in bytes + between two transaction log locations. It can be used with + structnamepg_stat_replication/structname or some functions shown in + xref linkend=functions-admin-backup-table to get the replication lag. + /para + + para For details about proper usage of these functions, see xref linkend=continuous-archiving. /para diff --git a/src/backend/access/transam/xlogfuncs.c b/src/backend/access/transam/xlogfuncs.c index 2e10d4d..be7d388 100644 --- a/src/backend/access/transam/xlogfuncs.c +++ b/src/backend/access/transam/xlogfuncs.c @@ -26,6 +26,7 @@ #include replication/walreceiver.h #include storage/smgr.h #include utils/builtins.h +#include utils/numeric.h #include utils/guc.h #include utils/timestamp.h @@ -465,3 +466,83 @@ pg_is_in_recovery(PG_FUNCTION_ARGS) { PG_RETURN_BOOL(RecoveryInProgress()); } + +static void +validate_xlog_location(char *str) +{ +#define MAXLSNCOMPONENT 8 + + int len1, len2; + + len1 = strspn(str, 0123456789abcdefABCDEF); + if (len1 1 || len1 MAXLSNCOMPONENT || str[len1] != '/') + ereport(ERROR, +(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION), + errmsg(invalid input syntax for transaction log location: \%s\, str))); + len2 = strspn(str + len1 + 1, 0123456789abcdefABCDEF); + if (len2 1 || len2 MAXLSNCOMPONENT || str[len1 + 1 + len2] != '\0') + ereport(ERROR, +(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION), + errmsg(invalid input syntax for transaction log location: \%s\, str))); +} + +/* + * Compute the difference in bytes between two WAL locations. + */ +Datum +pg_xlog_location_diff(PG_FUNCTION_ARGS) +{ + text *location1 = PG_GETARG_TEXT_P(0); + text *location2 = PG_GETARG_TEXT_P(1); + char *str1, *str2; + XLogRecPtr loc1, loc2; + Numeric result; + + /* + * Read and parse input + */ + str1 = text_to_cstring(location1); + str2 = text_to_cstring(location2); + + validate_xlog_location(str1); + validate_xlog_location(str2); + + if (sscanf(str1, %X/%X, loc1.xlogid, loc1.xrecoff) != 2) + ereport(ERROR, +(errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg(could not parse transaction log location \%s\, str1))); + if (sscanf(str2, %X/%X, loc2.xlogid, loc2.xrecoff) != 2) + ereport(ERROR, +(errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg(could not parse transaction log location \%s\, str2))); + + /* + * Sanity check + */ + if (loc1.xrecoff XLogFileSize) + ereport(ERROR, +(errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg(xrecoff \%X\ is out of valid range, 0..%X, loc1.xrecoff, XLogFileSize))); + if (loc2.xrecoff XLogFileSize) + ereport(ERROR, +(errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg(xrecoff \%X\ is out of valid range, 0..%X, loc2.xrecoff, XLogFileSize))); + + /* + * result =
Re: [HACKERS] xlog location arithmetic
On Wed, Feb 8, 2012 at 2:29 AM, Euler Taveira de Oliveira eu...@timbira.com wrote: On 26-01-2012 06:19, Fujii Masao wrote: Thanks for your review. Comments below. When I compiled the source with xlogdiff.patch, I got the following warnings. xlogfuncs.c:511:2: warning: format '%lX' expects argument of type 'long unsigned int *', but argument 3 has type 'uint64 *' [-Wformat] What is your compiler? I'm using gcc 4.6.2. I refactored the patch so I'm now using XLogRecPtr and %X. gcc version 4.6.1 (Ubuntu/Linaro 4.6.1-9ubuntu3) $ uname -a Linux hermes 3.0.0-15-generic #26-Ubuntu SMP Fri Jan 20 15:59:53 UTC 2012 i686 i686 i386 GNU/Linux In the updated version of the patch, I got no warnings at the compile time. But initdb failed because the OID which you assigned to pg_xlog_location_diff() has already been used for other function. So you need to update pg_proc.h. postgres=# SELECT pg_xlog_location_diff('0/274', '0/274'); ERROR: xrecoff 274 is out of valid range, 0..A4A534C Ugh? I can't reproduce that. It seems to be related to long int used by the prior version. Maybe. But another problem happened. When I changed pg_proc.h so that the unused OID was assigned to pg_xlog_location_diff(), and executed the above again, I encountered the segmentation fault: LOG: server process (PID 14384) was terminated by signal 11: Segmentation fault DETAIL: Failed process was running: SELECT pg_xlog_location_diff('0/274', '0/274'); LOG: terminating any other active server processes ISTM that the cause is that int8_numeric() is executed for uint32 value. We should use int4_numeric(), instead? While the output was int8 I could use pg_size_pretty but now I couldn't. I attached another patch that implements pg_size_pretty(numeric). I realized that it collides with the pg_size_pretty(int8) if we don't specify a type. Hence, I decided to drop the pg_size_pretty(int8) in favor of pg_size_pretty(numeric). It is slower than the former but it is not a performance critical function. I'm OK with this. -DATA(insert OID = 2288 ( pg_size_prettyPGNSP PGUID 12 1 0 0 0 f f f t f v 1 0 25 20 _null_ _null_ _null_ _null_ pg_size_pretty _null_ _null_ _null_ )); -DESCR(convert a long int to a human readable text using size units); +DATA(insert OID = 3158 ( pg_size_prettyPGNSP PGUID 12 1 0 0 0 f f f t f v 1 0 25 1700 _null_ _null_ _null_ _null_ pg_size_pretty _null_ _null_ _null_ )); +DESCR(convert a numeric to a human readable text using size units); Why OID needs to be reassigned? Regards, -- Fujii Masao NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center -- 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] xlog location arithmetic
On 26-01-2012 06:19, Fujii Masao wrote: Thanks for your review. Comments below. When I compiled the source with xlogdiff.patch, I got the following warnings. xlogfuncs.c:511:2: warning: format '%lX' expects argument of type 'long unsigned int *', but argument 3 has type 'uint64 *' [-Wformat] What is your compiler? I'm using gcc 4.6.2. I refactored the patch so I'm now using XLogRecPtr and %X. postgres=# SELECT pg_xlog_location_diff('0/274', '0/274'); ERROR: xrecoff 274 is out of valid range, 0..A4A534C Ugh? I can't reproduce that. It seems to be related to long int used by the prior version. Since pg_xlog_location_diff() can be executed during recovery, the above needs to be updated. Fixed. While the output was int8 I could use pg_size_pretty but now I couldn't. I attached another patch that implements pg_size_pretty(numeric). I realized that it collides with the pg_size_pretty(int8) if we don't specify a type. Hence, I decided to drop the pg_size_pretty(int8) in favor of pg_size_pretty(numeric). It is slower than the former but it is not a performance critical function. According to the above source code comment in pg_proc.h, ISTM pg_size_pretty() for numeric also needs to have its own DESCR(). Fixed. According to man strcat, the dest string must have enough space for the result. buf has enough space? Ops. Fixed. -- Euler Taveira de Oliveira - Timbira http://www.timbira.com.br/ PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml index 236a60a..511a918 100644 --- a/doc/src/sgml/func.sgml +++ b/doc/src/sgml/func.sgml @@ -14942,7 +14942,7 @@ postgres=# SELECT * FROM pg_xlogfile_name_offset(pg_stop_backup()); /row row entry -literalfunctionpg_size_pretty(typebigint/type)/function/literal +literalfunctionpg_size_pretty(typenumeric/type)/function/literal /entry entrytypetext/type/entry entryConverts a size in bytes into a human-readable format with size units/entry diff --git a/src/backend/utils/adt/dbsize.c b/src/backend/utils/adt/dbsize.c index 26a8c01..d4a142b 100644 --- a/src/backend/utils/adt/dbsize.c +++ b/src/backend/utils/adt/dbsize.c @@ -24,6 +24,7 @@ #include storage/fd.h #include utils/acl.h #include utils/builtins.h +#include utils/numeric.h #include utils/rel.h #include utils/relmapper.h #include utils/syscache.h @@ -506,48 +507,101 @@ pg_total_relation_size(PG_FUNCTION_ARGS) PG_RETURN_INT64(size); } -/* - * formatting with size units - */ Datum pg_size_pretty(PG_FUNCTION_ARGS) { - int64 size = PG_GETARG_INT64(0); - char buf[64]; - int64 limit = 10 * 1024; - int64 limit2 = limit * 2 - 1; + Numeric size = PG_GETARG_NUMERIC(0); + Numeric limit, limit2; + + char *buf, *result; - if (size limit) - snprintf(buf, sizeof(buf), INT64_FORMAT bytes, size); + limit = DatumGetNumeric(DirectFunctionCall1(int8_numeric, Int64GetDatum((int64) (10 * 1024; + limit2 = DatumGetNumeric(DirectFunctionCall1(int8_numeric, Int64GetDatum((int64) (10 * 1024 * 2 - 1; + + if (DatumGetBool(DirectFunctionCall2(numeric_lt, NumericGetDatum(size), NumericGetDatum(limit + { + buf = DatumGetCString(DirectFunctionCall1(numeric_out, NumericGetDatum(size))); + result = palloc(strlen(buf) + 7); + strcpy(result, buf); + strcat(result, bytes); + } else { - size = 9;/* keep one extra bit for rounding */ - if (size limit2) - snprintf(buf, sizeof(buf), INT64_FORMAT kB, - (size + 1) / 2); + Numeric arg2; + + /* keep one extra bit for rounding */ + /* size = 9 */ + arg2 = DatumGetNumeric(DirectFunctionCall1(int8_numeric, Int64GetDatum((int64) pow(2, 9; + size = DatumGetNumeric(DirectFunctionCall2(numeric_div_trunc, NumericGetDatum(size), NumericGetDatum(arg2))); + + if (DatumGetBool(DirectFunctionCall2(numeric_lt, NumericGetDatum(size), NumericGetDatum(limit2 + { + /* size = (size + 1) / 2 */ + size = DatumGetNumeric(DirectFunctionCall2(numeric_add, NumericGetDatum(size), + DirectFunctionCall1(int8_numeric, Int64GetDatum(1; + size = DatumGetNumeric(DirectFunctionCall2(numeric_div_trunc, NumericGetDatum(size), + DirectFunctionCall1(int8_numeric, Int64GetDatum(2; + buf = DatumGetCString(DirectFunctionCall1(numeric_out, NumericGetDatum(size))); + result = palloc(strlen(buf) + 4); + strcpy(result, buf); + strcat(result, kB); + } else { - size = 10; - if (size limit2) -snprintf(buf, sizeof(buf), INT64_FORMAT MB, - (size + 1) / 2); + Numeric arg3; + + /* size = 10 */ + arg3 = DatumGetNumeric(DirectFunctionCall1(int8_numeric, Int64GetDatum((int64) pow(2, 10; + size = DatumGetNumeric(DirectFunctionCall2(numeric_div_trunc, NumericGetDatum(size), NumericGetDatum(arg3))); + + if (DatumGetBool(DirectFunctionCall2(numeric_lt, NumericGetDatum(size), NumericGetDatum(limit2 + { +/* size
Re: [HACKERS] xlog location arithmetic
On Sun, Jan 22, 2012 at 1:13 AM, Euler Taveira de Oliveira eu...@timbira.com wrote: On 23-12-2011 12:05, Tom Lane wrote: I too think a datatype is overkill, if we're only planning on providing one function. Just emit the values as numeric and have done. Here it is. Output changed to numeric. Thanks! When I compiled the source with xlogdiff.patch, I got the following warnings. xlogfuncs.c:511:2: warning: format '%lX' expects argument of type 'long unsigned int *', but argument 3 has type 'uint64 *' [-Wformat] xlogfuncs.c:511:2: warning: format '%lX' expects argument of type 'long unsigned int *', but argument 4 has type 'uint64 *' [-Wformat] xlogfuncs.c:515:2: warning: format '%lX' expects argument of type 'long unsigned int *', but argument 3 has type 'uint64 *' [-Wformat] xlogfuncs.c:515:2: warning: format '%lX' expects argument of type 'long unsigned int *', but argument 4 has type 'uint64 *' [-Wformat] xlogfuncs.c:524:3: warning: format '%lX' expects argument of type 'long unsigned int', but argument 2 has type 'uint64' [-Wformat] xlogfuncs.c:528:3: warning: format '%lX' expects argument of type 'long unsigned int', but argument 2 has type 'uint64' [-Wformat] When I tested the patch, I got the following error: postgres=# SELECT pg_current_xlog_location(); pg_current_xlog_location -- 0/274 (1 row) postgres=# SELECT pg_xlog_location_diff('0/274', '0/274'); ERROR: xrecoff 274 is out of valid range, 0..A4A534C In func.sgml para The functions shown in xref linkend=functions-admin-backup-table assist in making on-line backups. These functions cannot be executed during recovery. /para Since pg_xlog_location_diff() can be executed during recovery, the above needs to be updated. While the output was int8 I could use pg_size_pretty but now I couldn't. I attached another patch that implements pg_size_pretty(numeric). I agree it's necessary. * Note: every entry in pg_proc.h is expected to have a DESCR() comment, * except for functions that implement pg_operator.h operators and don't * have a good reason to be called directly rather than via the operator. According to the above source code comment in pg_proc.h, ISTM pg_size_pretty() for numeric also needs to have its own DESCR(). + buf = DatumGetCString(DirectFunctionCall1(numeric_out, NumericGetDatum(size))); + result = strcat(buf, kB); According to man strcat, the dest string must have enough space for the result. buf has enough space? Regards, -- Fujii Masao NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center -- 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] xlog location arithmetic
On 23-12-2011 12:05, Tom Lane wrote: I too think a datatype is overkill, if we're only planning on providing one function. Just emit the values as numeric and have done. Here it is. Output changed to numeric. While the output was int8 I could use pg_size_pretty but now I couldn't. I attached another patch that implements pg_size_pretty(numeric). -- Euler Taveira de Oliveira - Timbira http://www.timbira.com.br/ PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml index 48631cc..04bc24d 100644 --- a/doc/src/sgml/func.sgml +++ b/doc/src/sgml/func.sgml @@ -14378,6 +14378,9 @@ SELECT set_config('log_statement_stats', 'off', false); indexterm primarypg_xlogfile_name_offset/primary /indexterm + indexterm +primarypg_xlog_location_diff/primary + /indexterm para The functions shown in xref @@ -14450,6 +14453,13 @@ SELECT set_config('log_statement_stats', 'off', false); entrytypetext/, typeinteger//entry entryConvert transaction log location string to file name and decimal byte offset within file/entry /row + row + entry +literalfunctionpg_xlog_location_diff(parameterlocation/ typetext/, parameterlocation/ typetext/)/function/literal +/entry + entrytypenumeric//entry + entryCalculate the difference between two transaction log locations/entry + /row /tbody /tgroup /table @@ -14543,6 +14553,13 @@ postgres=# SELECT * FROM pg_xlogfile_name_offset(pg_stop_backup()); /para para + functionpg_xlog_location_diff/ calculates the difference in bytes + between two transaction log locations. It can be used with + structnamepg_stat_replication/structname or some functions shown in + xref linkend=functions-admin-backup-table to get the replication lag. + /para + + para For details about proper usage of these functions, see xref linkend=continuous-archiving. /para diff --git a/src/backend/access/transam/xlogfuncs.c b/src/backend/access/transam/xlogfuncs.c index 2e10d4d..e03c5e8 100644 --- a/src/backend/access/transam/xlogfuncs.c +++ b/src/backend/access/transam/xlogfuncs.c @@ -26,6 +26,7 @@ #include replication/walreceiver.h #include storage/smgr.h #include utils/builtins.h +#include utils/numeric.h #include utils/guc.h #include utils/timestamp.h @@ -465,3 +466,84 @@ pg_is_in_recovery(PG_FUNCTION_ARGS) { PG_RETURN_BOOL(RecoveryInProgress()); } + +static void +validate_xlog_location(char *str) +{ +#define MAXLSNCOMPONENT 8 + + int len1, len2; + + len1 = strspn(str, 0123456789abcdefABCDEF); + if (len1 1 || len1 MAXLSNCOMPONENT || str[len1] != '/') + ereport(ERROR, +(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION), + errmsg(invalid input syntax for transaction log location: \%s\, str))); + len2 = strspn(str + len1 + 1, 0123456789abcdefABCDEF); + if (len2 1 || len2 MAXLSNCOMPONENT || str[len1 + 1 + len2] != '\0') + ereport(ERROR, +(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION), + errmsg(invalid input syntax for transaction log location: \%s\, str))); +} + +/* + * Compute the difference in bytes between two WAL locations. + */ +Datum +pg_xlog_location_diff(PG_FUNCTION_ARGS) +{ + text *location1 = PG_GETARG_TEXT_P(0); + text *location2 = PG_GETARG_TEXT_P(1); + char *str1, *str2; + uint64 xlogid1, xrecoff1; + uint64 xlogid2, xrecoff2; + Numeric result; + + /* + * Read and parse input + */ + str1 = text_to_cstring(location1); + str2 = text_to_cstring(location2); + + validate_xlog_location(str1); + validate_xlog_location(str2); + + if (sscanf(str1, %8lX/%8lX, xlogid1, xrecoff1) != 2) + ereport(ERROR, +(errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg(could not parse transaction log location \%s\, str1))); + if (sscanf(str2, %8lX/%8lX, xlogid2, xrecoff2) != 2) + ereport(ERROR, +(errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg(could not parse transaction log location \%s\, str2))); + + /* + * Sanity check + */ + if (xrecoff1 XLogFileSize) + ereport(ERROR, +(errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg(xrecoff \%lX\ is out of valid range, 0..%X, xrecoff1, XLogFileSize))); + if (xrecoff2 XLogFileSize) + ereport(ERROR, +(errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg(xrecoff \%lX\ is out of valid range, 0..%X, xrecoff2, XLogFileSize))); + + /* + * result = XLogFileSize * (xlogid1 - xlogid2) + xrecoff1 - xrecoff2 + */ + result = DatumGetNumeric(DirectFunctionCall2(numeric_sub, + DirectFunctionCall1(int8_numeric, Int64GetDatum(xlogid1)), + DirectFunctionCall1(int8_numeric, Int64GetDatum(xlogid2; + result = DatumGetNumeric(DirectFunctionCall2(numeric_mul, + DirectFunctionCall1(int8_numeric, Int64GetDatum(XLogFileSize)), + NumericGetDatum(result))); + result = DatumGetNumeric(DirectFunctionCall2(numeric_add, + NumericGetDatum(result), + DirectFunctionCall1(int8_numeric, Int64GetDatum(xrecoff1;
Re: [HACKERS] xlog location arithmetic
On 20 December 2011 10:27, Magnus Hagander mag...@hagander.net wrote: Doing it in numeric should be perfectly fine. The only real reason to pick int8 over in this context would be performance, but it's not like this is something that's going to be called in really performance critical paths... FYI, my group commit patch has a little macro, in the spirit of XLByteAdvance, to get the delta between two LSNs in bytes as an uint64. -- Peter Geoghegan http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training and 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] xlog location arithmetic
On Tue, Dec 6, 2011 at 1:19 PM, Euler Taveira de Oliveira eu...@timbira.com wrote: Hi, A while ago when blogging about WAL [1], I noticed a function to deal with xlog location arithmetic is wanted. I remembered Depez [2] mentioning it and after some questions during trainings and conferences I decided to translate my shell script function in C. The attached patch implements the function pg_xlog_location_diff (bikeshed colors are welcome). It calculates the difference between two given transaction log locations. Now that we have pg_stat_replication view, it will be easy to get the lag just passing columns as parameters. Also, the monitoring tools could take advantage of it instead of relying on a fragile routine to get the lag. I noticed that pg_xlogfile_name* functions does not sanity check the xrecoff boundaries but that is material for another patch. [1] http://eulerto.blogspot.com/2011/11/understanding-wal-nomenclature.html [2] http://www.depesz.com/index.php/2011/01/24/waiting-for-9-1-pg_stat_replication/ I think that this function is very useful. Can you add the patch into CommitFest 2012-1 ? Regards, -- Fujii Masao NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center -- 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] xlog location arithmetic
On 14-01-2012 11:06, Fujii Masao wrote: I think that this function is very useful. Can you add the patch into CommitFest 2012-1 ? Sure. But I must adjust the patch based on the thread comments (basically, numeric output). I have a new patch but need to test it before submitting it. I'll post this weekend. -- Euler Taveira de Oliveira - Timbira http://www.timbira.com.br/ PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento -- 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] xlog location arithmetic
On 01/14/2012 09:12 AM, Euler Taveira de Oliveira wrote: But I must adjust the patch based on the thread comments (basically, numeric output). I have a new patch but need to test it before submitting it. I'll post this weekend. It's now at https://commitfest.postgresql.org/action/patch_view?id=776 listed as waiting on you right now. It's good to put patches into the CF application early. Helps planning, and gives a safety net against all sorts of things. We wouldn't want something this obviously useful to get kicked out if, for example, you lost your Internet connection over the weekend and then didn't technically qualify as having submitted it there before the deadline. As someone who sweated today for two hours when my power at home was turned off to install a new circuit breaker, I'm feeling particularly paranoid right now about that sort of thing here. The fact that you got some review feedback before the official CF start doesn't mean you can't be listed there right now. In fact, those are things I like to see tracked. Having links to all of the e-mail messages that were important turning points for a feature that changed during review is very helpful to reviewers and committers. And the easiest way to keep up with that is to start as early as possible: add it to the app right after the first patch submission. -- Greg Smith 2ndQuadrant USg...@2ndquadrant.com Baltimore, MD PostgreSQL Training, Services, and 24x7 Support www.2ndQuadrant.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] xlog location arithmetic
On Sat, Jan 14, 2012 at 8:18 PM, Greg Smith g...@2ndquadrant.com wrote: On 01/14/2012 09:12 AM, Euler Taveira de Oliveira wrote: But I must adjust the patch based on the thread comments (basically, numeric output). I have a new patch but need to test it before submitting it. I'll post this weekend. It's now at https://commitfest.postgresql.**org/action/patch_view?id=776https://commitfest.postgresql.org/action/patch_view?id=776listed as waiting on you right now. It's good to put patches into the CF application early. Helps planning, and gives a safety net against all sorts of things. We wouldn't want something this obviously useful to get kicked out if, for example, you lost your Internet connection over the weekend and then didn't technically qualify as having submitted it there before the deadline. As someone who sweated today for two hours when my power at home was turned off to install a new circuit breaker, I'm feeling particularly paranoid right now about that sort of thing here. he patch The fact that you got some review feedback before the official CF start doesn't mean you can't be listed there right now. In fact, those are things I like to see tracked. Having links to all of the e-mail messages that were important turning points for a feature that changed during review is very helpful to reviewers and committers. And the easiest way to keep up with that is to start as early as possible: add it to the app right after the first patch submission. I agree. So lets make it easy for the patch submitter to start the process. I propose that we have a page in the CF application where people can upload/attach the patch, and the app posts the patch to -hackers and uses the post URL to create the CF entry. Regards, -- Gurjeet Singh EnterpriseDB Corporation The Enterprise PostgreSQL Company
Re: [HACKERS] xlog location arithmetic
On Tue, Dec 20, 2011 at 14:08, Euler Taveira de Oliveira eu...@timbira.com wrote: On 20-12-2011 07:27, Magnus Hagander wrote: On Tue, Dec 6, 2011 at 19:06, Robert Haas robertmh...@gmail.com wrote: On Tue, Dec 6, 2011 at 1:00 PM, Euler Taveira de Oliveira eu...@timbira.com wrote: On 06-12-2011 13:11, Robert Haas wrote: On Tue, Dec 6, 2011 at 5:14 AM, Magnus Hagander mag...@hagander.net wrote: I've been considering similar things, as you can find in the archives, but what I was thinking of was converting the number to just a plain bigint, then letting the user apply whatever arithmetic wanted at the SQL level. I never got around to acutally coding it, though. It could easily be extracted from your patch of course - and I think that's a more flexible approach. Is there some advantage to your method that I'm missing? I went so far as to put together an lsn data type. I didn't actually get all that far with it, which is why I haven't posted it sooner, but here's what I came up with. It's missing indexing support and stuff, but that could be added if people like the approach. It solves this problem by implementing -(lsn,lsn) = numeric (not int8, that can overflow since it is not unsigned), which allows an lsn = numeric conversion by just subtracting '0/0'::lsn. Interesting approach. I don't want to go that far. If so, you want to change all of those functions that deal with LSNs and add some implicit conversion between text and lsn data types (for backward compatibility). As of int8, I'm As long as you have the conversion, you don't really need to change them, do you? It might be nice in some ways, but this is still a pretty internal operation, so I don't see it as critical. For correctness, yes. At this point, my question is: do we want to support the lsn data type idea or a basic function that implements the difference between LSNs? Personally I think a function is enough - it solves the only case that I've actually seen. But a datatype would be a more complete solution, of course - but it seems a bit of an overkill to me. Not really sure which way we should go - I was hoping somebody else would comment as well.. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.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] xlog location arithmetic
Magnus Hagander mag...@hagander.net writes: At this point, my question is: do we want to support the lsn data type idea or a basic function that implements the difference between LSNs? Personally I think a function is enough - it solves the only case that I've actually seen. But a datatype would be a more complete solution, of course - but it seems a bit of an overkill to me. Not really sure which way we should go - I was hoping somebody else would comment as well.. I too think a datatype is overkill, if we're only planning on providing one function. Just emit the values as numeric and have done. 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] xlog location arithmetic
On Fri, Dec 23, 2011 at 10:05 AM, Tom Lane t...@sss.pgh.pa.us wrote: I too think a datatype is overkill, if we're only planning on providing one function. Are there any other functions we ought to provide? -- 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] xlog location arithmetic
On 12/23/2011 10:05 AM, Tom Lane wrote: Magnus Hagandermag...@hagander.net writes: At this point, my question is: do we want to support the lsn data type idea or a basic function that implements the difference between LSNs? Personally I think a function is enough - it solves the only case that I've actually seen. But a datatype would be a more complete solution, of course - but it seems a bit of an overkill to me. Not really sure which way we should go - I was hoping somebody else would comment as well.. I too think a datatype is overkill, if we're only planning on providing one function. Just emit the values as numeric and have done. +1. cheers andrew -- 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] xlog location arithmetic
Robert Haas robertmh...@gmail.com writes: On Fri, Dec 23, 2011 at 10:05 AM, Tom Lane t...@sss.pgh.pa.us wrote: I too think a datatype is overkill, if we're only planning on providing one function. Are there any other functions we ought to provide? Even if there are several, what exact advantage does a datatype offer over representing LSN values as numerics? It seems to me to be adding complication and extra code (I/O converters at least) for very little gain. 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] xlog location arithmetic
On Fri, Dec 23, 2011 at 10:18 AM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Fri, Dec 23, 2011 at 10:05 AM, Tom Lane t...@sss.pgh.pa.us wrote: I too think a datatype is overkill, if we're only planning on providing one function. Are there any other functions we ought to provide? Even if there are several, what exact advantage does a datatype offer over representing LSN values as numerics? It seems to me to be adding complication and extra code (I/O converters at least) for very little gain. I guess I'm just constitutionally averse to labeling things as text when they really aren't. I do it all the time in Perl, of course, but in PostgreSQL we have strong data typing, and it seems like we might as well use it. Also, we've occasionally talked (in the light of Pavan's single-pass vacuum patch, for example) about needing to store LSNs in system catalogs; and we're certainly not going to want to do that as text. I'll admit that it's not 100% clear that anything like this will ever happen, though, so maybe it's premature to worry about it. I can see I'm in the minority on this one, though... -- 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] xlog location arithmetic
Robert Haas robertmh...@gmail.com writes: On Fri, Dec 23, 2011 at 10:18 AM, Tom Lane t...@sss.pgh.pa.us wrote: Even if there are several, what exact advantage does a datatype offer over representing LSN values as numerics? It seems to me to be adding complication and extra code (I/O converters at least) for very little gain. I guess I'm just constitutionally averse to labeling things as text when they really aren't. Er ... text? I thought the proposal was to use numeric. 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] xlog location arithmetic
On Fri, Dec 23, 2011 at 10:59 AM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Fri, Dec 23, 2011 at 10:18 AM, Tom Lane t...@sss.pgh.pa.us wrote: Even if there are several, what exact advantage does a datatype offer over representing LSN values as numerics? It seems to me to be adding complication and extra code (I/O converters at least) for very little gain. I guess I'm just constitutionally averse to labeling things as text when they really aren't. Er ... text? I thought the proposal was to use numeric. The proposal is to make a function that takes a text argument (which is really an LSN, but we choose to represent it as text) and returns numeric. -- 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] xlog location arithmetic
On Tue, Dec 6, 2011 at 19:06, Robert Haas robertmh...@gmail.com wrote: On Tue, Dec 6, 2011 at 1:00 PM, Euler Taveira de Oliveira eu...@timbira.com wrote: On 06-12-2011 13:11, Robert Haas wrote: On Tue, Dec 6, 2011 at 5:14 AM, Magnus Hagander mag...@hagander.net wrote: I've been considering similar things, as you can find in the archives, but what I was thinking of was converting the number to just a plain bigint, then letting the user apply whatever arithmetic wanted at the SQL level. I never got around to acutally coding it, though. It could easily be extracted from your patch of course - and I think that's a more flexible approach. Is there some advantage to your method that I'm missing? I went so far as to put together an lsn data type. I didn't actually get all that far with it, which is why I haven't posted it sooner, but here's what I came up with. It's missing indexing support and stuff, but that could be added if people like the approach. It solves this problem by implementing -(lsn,lsn) = numeric (not int8, that can overflow since it is not unsigned), which allows an lsn = numeric conversion by just subtracting '0/0'::lsn. Interesting approach. I don't want to go that far. If so, you want to change all of those functions that deal with LSNs and add some implicit conversion between text and lsn data types (for backward compatibility). As of int8, I'm As long as you have the conversion, you don't really need to change them, do you? It might be nice in some ways, but this is still a pretty internal operation, so I don't see it as critical. not aware of any modern plataform that int8 is not 64 bits. I'm not against numeric use; I'm just saying that int8 is sufficient. The point isn't that int8 might not be 64 bits - of course it has to be 64 bits; that's why it's called int8 i.e. 8 bytes. The point is that a large enough LSN, represented as an int8, will come out as a negative values. int8 can only represent 2^63 *non-negative* values, because one bit is reserved for sign. Doing it in numeric should be perfectly fine. The only real reason to pick int8 over in this context would be performance, but it's not like this is something that's going to be called in really performance critical paths... -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.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] xlog location arithmetic
On 20-12-2011 07:27, Magnus Hagander wrote: On Tue, Dec 6, 2011 at 19:06, Robert Haas robertmh...@gmail.com wrote: On Tue, Dec 6, 2011 at 1:00 PM, Euler Taveira de Oliveira eu...@timbira.com wrote: On 06-12-2011 13:11, Robert Haas wrote: On Tue, Dec 6, 2011 at 5:14 AM, Magnus Hagander mag...@hagander.net wrote: I've been considering similar things, as you can find in the archives, but what I was thinking of was converting the number to just a plain bigint, then letting the user apply whatever arithmetic wanted at the SQL level. I never got around to acutally coding it, though. It could easily be extracted from your patch of course - and I think that's a more flexible approach. Is there some advantage to your method that I'm missing? I went so far as to put together an lsn data type. I didn't actually get all that far with it, which is why I haven't posted it sooner, but here's what I came up with. It's missing indexing support and stuff, but that could be added if people like the approach. It solves this problem by implementing -(lsn,lsn) = numeric (not int8, that can overflow since it is not unsigned), which allows an lsn = numeric conversion by just subtracting '0/0'::lsn. Interesting approach. I don't want to go that far. If so, you want to change all of those functions that deal with LSNs and add some implicit conversion between text and lsn data types (for backward compatibility). As of int8, I'm As long as you have the conversion, you don't really need to change them, do you? It might be nice in some ways, but this is still a pretty internal operation, so I don't see it as critical. For correctness, yes. At this point, my question is: do we want to support the lsn data type idea or a basic function that implements the difference between LSNs? -- Euler Taveira de Oliveira - Timbira http://www.timbira.com.br/ PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento -- 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] xlog location arithmetic
On Dec 6, 2011, at 12:06 PM, Robert Haas wrote: On Tue, Dec 6, 2011 at 1:00 PM, Euler Taveira de Oliveira eu...@timbira.com wrote: On 06-12-2011 13:11, Robert Haas wrote: On Tue, Dec 6, 2011 at 5:14 AM, Magnus Hagander mag...@hagander.net wrote: I've been considering similar things, as you can find in the archives, but what I was thinking of was converting the number to just a plain bigint, then letting the user apply whatever arithmetic wanted at the SQL level. I never got around to acutally coding it, though. It could easily be extracted from your patch of course - and I think that's a more flexible approach. Is there some advantage to your method that I'm missing? I went so far as to put together an lsn data type. I didn't actually get all that far with it, which is why I haven't posted it sooner, but here's what I came up with. It's missing indexing support and stuff, but that could be added if people like the approach. It solves this problem by implementing -(lsn,lsn) = numeric (not int8, that can overflow since it is not unsigned), which allows an lsn = numeric conversion by just subtracting '0/0'::lsn. Interesting approach. I don't want to go that far. If so, you want to change all of those functions that deal with LSNs and add some implicit conversion between text and lsn data types (for backward compatibility). As of int8, I'm not aware of any modern plataform that int8 is not 64 bits. I'm not against numeric use; I'm just saying that int8 is sufficient. The point isn't that int8 might not be 64 bits - of course it has to be 64 bits; that's why it's called int8 i.e. 8 bytes. The point is that a large enough LSN, represented as an int8, will come out as a negative values. int8 can only represent 2^63 *non-negative* values, because one bit is reserved for sign. I've often wondered about adding uint2/4/8... I suspect it's actually pretty uncommon for people to put negative numbers into int fields, since one of their biggest uses seems to be surrogate keys. I realize that this opens a can of worms with casting, but perhaps that can be kept under control by not doing any implicit casting between int and uint... that just means that we'd have to be smart about casting from unknown, but hopefully that's doable since we already have a similar concern with casting unknown to int2/4/8 vs numeric? -- Jim C. Nasby, Database Architect j...@nasby.net 512.569.9461 (cell) http://jim.nasby.net -- 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] xlog location arithmetic
On Tue, Dec 13, 2011 at 12:48 PM, Jim Nasby j...@nasby.net wrote: I've often wondered about adding uint2/4/8... I suspect it's actually pretty uncommon for people to put negative numbers into int fields, since one of their biggest uses seems to be surrogate keys. I realize that this opens a can of worms with casting, but perhaps that can be kept under control by not doing any implicit casting between int and uint... that just means that we'd have to be smart about casting from unknown, but hopefully that's doable since we already have a similar concern with casting unknown to int2/4/8 vs numeric? I've wondered about it too, but it seems like too large a can of worms to open just to address this case. Returning the value as numeric is hardly a disaster; the user can always downcast to int8 if they really want, and as long as it's 2^63 (which in practice it virtually always will be) it will work. It's not clear what the point of this is since for typical values numeric is going to take up less storage anyway (e.g. 101 is 7 bytes on disk as a numeric), not to mention that it only requires 4-byte alignment rather than 8-byte alignment, and probably no one does enough arithmetic with LSN values for any speed penalty to matter even slightly, but it should work. -- 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] xlog location arithmetic
On Tue, Dec 6, 2011 at 05:19, Euler Taveira de Oliveira eu...@timbira.com wrote: Hi, A while ago when blogging about WAL [1], I noticed a function to deal with xlog location arithmetic is wanted. I remembered Depez [2] mentioning it and after some questions during trainings and conferences I decided to translate my shell script function in C. The attached patch implements the function pg_xlog_location_diff (bikeshed colors are welcome). It calculates the difference between two given transaction log locations. Now that we have pg_stat_replication view, it will be easy to get the lag just passing columns as parameters. Also, the monitoring tools could take advantage of it instead of relying on a fragile routine to get the lag. I've been considering similar things, as you can find in the archives, but what I was thinking of was converting the number to just a plain bigint, then letting the user apply whatever arithmetic wanted at the SQL level. I never got around to acutally coding it, though. It could easily be extracted from your patch of course - and I think that's a more flexible approach. Is there some advantage to your method that I'm missing? Also, why do you use DirectFunctionCall to do the simple math, and not just do the math right there in the function? -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.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] xlog location arithmetic
On Tue, Dec 6, 2011 at 5:14 AM, Magnus Hagander mag...@hagander.net wrote: I've been considering similar things, as you can find in the archives, but what I was thinking of was converting the number to just a plain bigint, then letting the user apply whatever arithmetic wanted at the SQL level. I never got around to acutally coding it, though. It could easily be extracted from your patch of course - and I think that's a more flexible approach. Is there some advantage to your method that I'm missing? I went so far as to put together an lsn data type. I didn't actually get all that far with it, which is why I haven't posted it sooner, but here's what I came up with. It's missing indexing support and stuff, but that could be added if people like the approach. It solves this problem by implementing -(lsn,lsn) = numeric (not int8, that can overflow since it is not unsigned), which allows an lsn = numeric conversion by just subtracting '0/0'::lsn. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company lsn.patch Description: Binary data -- 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] xlog location arithmetic
On 06-12-2011 07:14, Magnus Hagander wrote: On Tue, Dec 6, 2011 at 05:19, Euler Taveira de Oliveira eu...@timbira.com wrote: Hi, A while ago when blogging about WAL [1], I noticed a function to deal with xlog location arithmetic is wanted. I remembered Depez [2] mentioning it and after some questions during trainings and conferences I decided to translate my shell script function in C. The attached patch implements the function pg_xlog_location_diff (bikeshed colors are welcome). It calculates the difference between two given transaction log locations. Now that we have pg_stat_replication view, it will be easy to get the lag just passing columns as parameters. Also, the monitoring tools could take advantage of it instead of relying on a fragile routine to get the lag. I've been considering similar things, as you can find in the archives, but what I was thinking of was converting the number to just a plain bigint, then letting the user apply whatever arithmetic wanted at the SQL level. I never got around to acutally coding it, though. It could easily be extracted from your patch of course - and I think that's a more flexible approach. Is there some advantage to your method that I'm missing? The only advantage is that you don't expose the arithmetic, e.g., user doesn't need to know the xlog internals (like I described in a recent blog post). If one day we consider changes in xlog arithmetic (for example, XLogFileSize), we don't need to worry too much about external tools. Also, why do you use DirectFunctionCall to do the simple math, and not just do the math right there in the function? I use it because I don't want to duplicate the overflow code. -- Euler Taveira de Oliveira - Timbira http://www.timbira.com.br/ PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento -- 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] xlog location arithmetic
On 06-12-2011 13:11, Robert Haas wrote: On Tue, Dec 6, 2011 at 5:14 AM, Magnus Hagander mag...@hagander.net wrote: I've been considering similar things, as you can find in the archives, but what I was thinking of was converting the number to just a plain bigint, then letting the user apply whatever arithmetic wanted at the SQL level. I never got around to acutally coding it, though. It could easily be extracted from your patch of course - and I think that's a more flexible approach. Is there some advantage to your method that I'm missing? I went so far as to put together an lsn data type. I didn't actually get all that far with it, which is why I haven't posted it sooner, but here's what I came up with. It's missing indexing support and stuff, but that could be added if people like the approach. It solves this problem by implementing -(lsn,lsn) = numeric (not int8, that can overflow since it is not unsigned), which allows an lsn = numeric conversion by just subtracting '0/0'::lsn. Interesting approach. I don't want to go that far. If so, you want to change all of those functions that deal with LSNs and add some implicit conversion between text and lsn data types (for backward compatibility). As of int8, I'm not aware of any modern plataform that int8 is not 64 bits. I'm not against numeric use; I'm just saying that int8 is sufficient. -- Euler Taveira de Oliveira - Timbira http://www.timbira.com.br/ PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento -- 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] xlog location arithmetic
On Tue, Dec 6, 2011 at 1:00 PM, Euler Taveira de Oliveira eu...@timbira.com wrote: On 06-12-2011 13:11, Robert Haas wrote: On Tue, Dec 6, 2011 at 5:14 AM, Magnus Hagander mag...@hagander.net wrote: I've been considering similar things, as you can find in the archives, but what I was thinking of was converting the number to just a plain bigint, then letting the user apply whatever arithmetic wanted at the SQL level. I never got around to acutally coding it, though. It could easily be extracted from your patch of course - and I think that's a more flexible approach. Is there some advantage to your method that I'm missing? I went so far as to put together an lsn data type. I didn't actually get all that far with it, which is why I haven't posted it sooner, but here's what I came up with. It's missing indexing support and stuff, but that could be added if people like the approach. It solves this problem by implementing -(lsn,lsn) = numeric (not int8, that can overflow since it is not unsigned), which allows an lsn = numeric conversion by just subtracting '0/0'::lsn. Interesting approach. I don't want to go that far. If so, you want to change all of those functions that deal with LSNs and add some implicit conversion between text and lsn data types (for backward compatibility). As of int8, I'm not aware of any modern plataform that int8 is not 64 bits. I'm not against numeric use; I'm just saying that int8 is sufficient. The point isn't that int8 might not be 64 bits - of course it has to be 64 bits; that's why it's called int8 i.e. 8 bytes. The point is that a large enough LSN, represented as an int8, will come out as a negative values. int8 can only represent 2^63 *non-negative* values, because one bit is reserved for sign. -- 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