Re: [HACKERS] walprotocol.h vs frontends

2011-08-16 Thread Magnus Hagander
On Tue, Aug 16, 2011 at 00:05, Simon Riggs si...@2ndquadrant.com wrote:
 On Mon, Aug 15, 2011 at 10:32 PM, Magnus Hagander mag...@hagander.net wrote:

 At present the WALSender only sends from one file at a time, so
 sending a message when we open a new file would be straightforward.

 Are you sure? We can receive a single message spanning multiple files...

 You're right. That was the way the original code ran but I thought we
 had stopped that when we introduced MAX_SEND_SIZE. The comment says
 don't cross a logfile boundary within one message. What that
 actually does is prevent us incrementing a logid value, which happens
 every 255 files. I read that as meaning WAL file which is not what
 it means at all.

That has bitten me many times. log file != wal file.. It's not exactly
intuitive.


 So right now what we do is allow a single packet to span multiple
 files, but since MAX_SEND_SIZE is 128KB it will always be smaller than
 a single file, so we can only ever span two files at most.

Unless someone has tweaked their xlog segment size to insane values.

I ended up writing a loop that can deal with it spanning 2 files as
well - but that also turned out to be less code than the special case
for 2 files, so I'll be keeping that :-)


 That is all just a little bizarre, especially since libpq sends data
 in 8KB chunks anyway.

 So I suggest we change XLogSend() so that it only ever sends up to the
 end of a file. That way all w messages will relate to just one file,
 and we can have another message to initiate a new file. And then, as
 you say, give full metadata for the new file.

That could be an idea in itself. It is not necessary for the log
receiver, since it can use the #define FRONTEND hack (which only works
if you build inside the source tree of course, not if it's an
extension) but it might make the protocol simpler to deal with for
third parties who want to do something similar.


-- 
 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] walprotocol.h vs frontends

2011-08-16 Thread Peter Eisentraut
On mån, 2011-08-15 at 18:39 +0100, Peter Geoghegan wrote:
  If you want to upgrade a system running 8.3 (that uses float based
 timestamps) in using
  pg_upgrade you must compile 9.0 (or 8.4 or 9.1) with
  --disable-integer-datetimes.  If at some point in the future you
 then want
  to upgrade to 9.2 with pg_upgrade you will again need to build 9.2
 with
  --disable-integer-datetimes.If we remove the code for floating
 point
  representations of datetime then you won't be able to do that.
 
 I'm pretty surprised that pg_upgrade pushes that onus onto its users -
 for many users, the need to build their own binaries is a greater
 barrier to upgrading than doing a logical restore. Maybe that's simply
 considered a matter for package managers to worry about, but that
 doesn't sit well with me.

Well, pg_upgrade only moves the heap files, it doesn't look into them or
change them.

Possibly, this sort of issue could be better handled in the future by
making this a cluster, database, or table flag instead of a compile-time
option.  That way, at least newly created things could move to the new
recommended behavior.  The way it is set up now, we will possibly never
get rid of the legacy behavior, unless we break pg_upgrade at some
point.



-- 
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] walprotocol.h vs frontends

2011-08-16 Thread Simon Riggs
On Tue, Aug 16, 2011 at 9:35 AM, Magnus Hagander mag...@hagander.net wrote:

 So right now what we do is allow a single packet to span multiple
 files, but since MAX_SEND_SIZE is 128KB it will always be smaller than
 a single file, so we can only ever span two files at most.

 Unless someone has tweaked their xlog segment size to insane values.

We should limit MAX_SEND_SIZE to be same or less than WAL_SEGMENT_SIZE.

We gain nothing by continuing to allow the existing code to work in
the way it does.

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

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


[HACKERS] walprotocol.h vs frontends

2011-08-15 Thread Magnus Hagander
I'm trying to make my streaming log receiver work properly with 9.1,
and have come across a couple of things. The first one that's causing
trouble is that the definition of the protocol is currently in
walprotocol.h, which is not include:able in a frontend application.
AFAICT, this is because it includes utils/timestamp.h, which doesn't
work. AFAICT, this means that anybody other than our own backend who
wants to talk our replication protocol has to copy the specific struct
defines they want in their own code. This seems like a really bad
idea. (In my case, it's the StandbyReplyMessage that I need, so I can
make my client not get killed by the default settings for timeout)

The basic reason for this is that we're putting TimestampTz fields in
the protocol. This also means that the protocol actually changes
definition depending on if the server is compiled with integer or
float timestamps. While the replication itself breaks if these are
different, this seems like a bad thing to expose in the protocol. It
also makes life a lot harder on third party tools.

Any ideas on what to do with this, other than me copying the struct
defs and changing them to int64 (which does work in my lab, but seems
like a horrible kludge).

-- 
 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] walprotocol.h vs frontends

2011-08-15 Thread Peter Geoghegan
On 15 August 2011 12:22, Magnus Hagander mag...@hagander.net wrote:
 The basic reason for this is that we're putting TimestampTz fields in
 the protocol. This also means that the protocol actually changes
 definition depending on if the server is compiled with integer or
 float timestamps.

Without commenting on what should be done in your specific case, I
wonder whether it's time to fully retire the deprecated double
representation of timestamps. Is anyone actually expected to rely on
their availability when 9.2 is released? This also caused difficulties
for Johann Oskarsson recently, during work on PL/Java.

-- 
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] walprotocol.h vs frontends

2011-08-15 Thread Tom Lane
Magnus Hagander mag...@hagander.net writes:
 I'm trying to make my streaming log receiver work properly with 9.1,
 and have come across a couple of things. The first one that's causing
 trouble is that the definition of the protocol is currently in
 walprotocol.h, which is not include:able in a frontend application.
 AFAICT, this is because it includes utils/timestamp.h, which doesn't
 work. AFAICT, this means that anybody other than our own backend who
 wants to talk our replication protocol has to copy the specific struct
 defines they want in their own code. This seems like a really bad
 idea. (In my case, it's the StandbyReplyMessage that I need, so I can
 make my client not get killed by the default settings for timeout)

 The basic reason for this is that we're putting TimestampTz fields in
 the protocol. This also means that the protocol actually changes
 definition depending on if the server is compiled with integer or
 float timestamps. While the replication itself breaks if these are
 different, this seems like a bad thing to expose in the protocol. It
 also makes life a lot harder on third party tools.

I don't really see why it matters, given that the data to be shipped is
also dependent on the timestamp format?

However, for a narrow fix, I could see moving the data type definition
to someplace with fewer dependencies.  Perhaps split it into a separate
file timestamp_type.h, or something like 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] walprotocol.h vs frontends

2011-08-15 Thread Magnus Hagander
On Mon, Aug 15, 2011 at 16:20, Tom Lane t...@sss.pgh.pa.us wrote:
 Magnus Hagander mag...@hagander.net writes:
 I'm trying to make my streaming log receiver work properly with 9.1,
 and have come across a couple of things. The first one that's causing
 trouble is that the definition of the protocol is currently in
 walprotocol.h, which is not include:able in a frontend application.
 AFAICT, this is because it includes utils/timestamp.h, which doesn't
 work. AFAICT, this means that anybody other than our own backend who
 wants to talk our replication protocol has to copy the specific struct
 defines they want in their own code. This seems like a really bad
 idea. (In my case, it's the StandbyReplyMessage that I need, so I can
 make my client not get killed by the default settings for timeout)

 The basic reason for this is that we're putting TimestampTz fields in
 the protocol. This also means that the protocol actually changes
 definition depending on if the server is compiled with integer or
 float timestamps. While the replication itself breaks if these are
 different, this seems like a bad thing to expose in the protocol. It
 also makes life a lot harder on third party tools.

 I don't really see why it matters, given that the data to be shipped is
 also dependent on the timestamp format?

As an example, the stream receiver program needs to be able to
construct the status message and send back - thus needs to be able to
construct a TimestampTz. It never needs to look into the actual WAL
data - it just writes it to a file considering it to be a stream of
pure binary data.


 However, for a narrow fix, I could see moving the data type definition
 to someplace with fewer dependencies.  Perhaps split it into a separate
 file timestamp_type.h, or something like that.

Yes, that seems to fix the problem of timestamptz. See the attached
patch - seems ok?


I also ran into a similar problem with some WAL macro definitions that
are in xlog_internal.h. I've moved them to xlogdefs.h in the attached
xlog.diff file. Does that seem ok as well, or should I move them
somewhere else? (I don't need all those macros, but I moved the
complete block of macros wherever I needed one of them, because
otherwise it would be completely impossible to track). This is just a
simple move of the definitions, zero new logic added and zero removed.


-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/
diff --git a/src/include/replication/walprotocol.h b/src/include/replication/walprotocol.h
index 9414667..cdba21b 100644
--- a/src/include/replication/walprotocol.h
+++ b/src/include/replication/walprotocol.h
@@ -13,7 +13,7 @@
 #define _WALPROTOCOL_H
 
 #include access/xlogdefs.h
-#include utils/timestamp.h
+#include utils/timestamp_defs.h
 
 
 /*
diff --git a/src/include/utils/timestamp.h b/src/include/utils/timestamp.h
index 9e51b58..1a5ea0d 100644
--- a/src/include/utils/timestamp.h
+++ b/src/include/utils/timestamp.h
@@ -19,86 +19,7 @@
 
 #include fmgr.h
 #include pgtime.h
-#ifdef HAVE_INT64_TIMESTAMP
-#include utils/int8.h
-#endif
-
-/*
- * Timestamp represents absolute time.
- *
- * Interval represents delta time. Keep track of months (and years), days,
- * and hours/minutes/seconds separately since the elapsed time spanned is
- * unknown until instantiated relative to an absolute time.
- *
- * Note that Postgres uses time interval to mean a bounded interval,
- * consisting of a beginning and ending time, not a time span - thomas 97/03/20
- *
- * We have two implementations, one that uses int64 values with units of
- * microseconds, and one that uses double values with units of seconds.
- *
- * TimeOffset and fsec_t are convenience typedefs for temporary variables
- * that are of different types in the two cases.  Do not use fsec_t in values
- * stored on-disk, since it is not the same size in both implementations.
- * Also, fsec_t is only meant for *fractional* seconds; beware of overflow
- * if the value you need to store could be many seconds.
- */
-
-#ifdef HAVE_INT64_TIMESTAMP
-
-typedef int64 Timestamp;
-typedef int64 TimestampTz;
-typedef int64 TimeOffset;
-typedef int32 fsec_t;			/* fractional seconds (in microseconds) */
-#else
-
-typedef double Timestamp;
-typedef double TimestampTz;
-typedef double TimeOffset;
-typedef double fsec_t;			/* fractional seconds (in seconds) */
-#endif
-
-typedef struct
-{
-	TimeOffset	time;			/* all time units other than days, months and
- * years */
-	int32		day;			/* days, after time for alignment */
-	int32		month;			/* months and years, after time for alignment */
-} Interval;
-
-
-#define MAX_TIMESTAMP_PRECISION 6
-#define MAX_INTERVAL_PRECISION 6
-
-/* in both timestamp.h and ecpg/dt.h */
-#define DAYS_PER_YEAR	365.25	/* assumes leap year every four years */
-#define MONTHS_PER_YEAR 12
-/*
- *	DAYS_PER_MONTH is very imprecise.  The more accurate value is
- *	365.2425/12 = 30.436875, or '30 days 10:29:06'.  Right now we only
- *	return an 

Re: [HACKERS] walprotocol.h vs frontends

2011-08-15 Thread Tom Lane
Magnus Hagander mag...@hagander.net writes:
 On Mon, Aug 15, 2011 at 16:20, Tom Lane t...@sss.pgh.pa.us wrote:
 However, for a narrow fix, I could see moving the data type definition
 to someplace with fewer dependencies.  Perhaps split it into a separate
 file timestamp_type.h, or something like that.

 Yes, that seems to fix the problem of timestamptz. See the attached
 patch - seems ok?

Don't think you should expose fsec_t, nor most of those macros.  The
foo_per_bar values are just namespace clutter.

 I also ran into a similar problem with some WAL macro definitions that
 are in xlog_internal.h. I've moved them to xlogdefs.h in the attached
 xlog.diff file. Does that seem ok as well, or should I move them
 somewhere else?

I don't like the idea of exposing those to frontends, either.  What do
you actually *need* out of that, and why?

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] walprotocol.h vs frontends

2011-08-15 Thread Magnus Hagander
On Mon, Aug 15, 2011 at 16:53, Tom Lane t...@sss.pgh.pa.us wrote:
 Magnus Hagander mag...@hagander.net writes:
 On Mon, Aug 15, 2011 at 16:20, Tom Lane t...@sss.pgh.pa.us wrote:
 However, for a narrow fix, I could see moving the data type definition
 to someplace with fewer dependencies.  Perhaps split it into a separate
 file timestamp_type.h, or something like that.

 Yes, that seems to fix the problem of timestamptz. See the attached
 patch - seems ok?

 Don't think you should expose fsec_t, nor most of those macros.  The
 foo_per_bar values are just namespace clutter.

Hmm, ok. I just went for what seemed like a reasonable subset. I do
also need the SECS_PER_DAY and those constants in order to be able to
convert the timestamp value. That led me to include the other
foo_per_bar so they are all in one place - seems wrong to have them
split up.



 I also ran into a similar problem with some WAL macro definitions that
 are in xlog_internal.h. I've moved them to xlogdefs.h in the attached
 xlog.diff file. Does that seem ok as well, or should I move them
 somewhere else?

 I don't like the idea of exposing those to frontends, either.  What do
 you actually *need* out of that, and why?

PrevLogSeg - which also brings in XLogSegsPerFile.
XLogFileName
XLogFileSize - which brings in XLogSegsPerFile and XLogSegSize

PrevLogSeg should be self explaining, as should xlogfilename.

XLogFileSize is required by XLByteAdvance() which is already in
xlogdefs.h - so xlogdefs.h actually has a hidden dependency on
xlog_internal.h here today.

I can certainly separate those out, but it seemed more clean to move
the whole block they were in.

-- 
 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] walprotocol.h vs frontends

2011-08-15 Thread Steve Singer

On 11-08-15 10:00 AM, Peter Geoghegan wrote:


Without commenting on what should be done in your specific case, I
wonder whether it's time to fully retire the deprecated double
representation of timestamps. Is anyone actually expected to rely on
their availability when 9.2 is released? This also caused difficulties
for Johann Oskarsson recently, during work on PL/Java.


This would mean that anyone using the floating point timestamps today 
won't be able to use pg_upgrade to upgrade to whichever version we 
remove them from.  8.3 had float based timestamps as the default and I 
suspect many installations with the default 8.3 settings have been 
upgraded via pg_upgrade to 9.0 built the old timestamps representation.


Steve


--
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] walprotocol.h vs frontends

2011-08-15 Thread Heikki Linnakangas

On 15.08.2011 18:54, Magnus Hagander wrote:

On Mon, Aug 15, 2011 at 16:53, Tom Lanet...@sss.pgh.pa.us  wrote:

Magnus Hagandermag...@hagander.net  writes:

I also ran into a similar problem with some WAL macro definitions that
are in xlog_internal.h. I've moved them to xlogdefs.h in the attached
xlog.diff file. Does that seem ok as well, or should I move them
somewhere else?


I don't like the idea of exposing those to frontends, either.  What do
you actually *need* out of that, and why?


PrevLogSeg - which also brings in XLogSegsPerFile.
XLogFileName
XLogFileSize - which brings in XLogSegsPerFile and XLogSegSize

PrevLogSeg should be self explaining, as should xlogfilename.

XLogFileSize is required by XLByteAdvance() which is already in
xlogdefs.h - so xlogdefs.h actually has a hidden dependency on
xlog_internal.h here today.

I can certainly separate those out, but it seemed more clean to move
the whole block they were in.


Perhaps we should change the protocol so that it explicitly says which 
file the streamed piece of WAL belongs to. That way a client could write 
it to the correct file without knowing about all those macros.


--
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.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] walprotocol.h vs frontends

2011-08-15 Thread Tom Lane
Heikki Linnakangas heikki.linnakan...@enterprisedb.com writes:
 Perhaps we should change the protocol so that it explicitly says which 
 file the streamed piece of WAL belongs to. That way a client could write 
 it to the correct file without knowing about all those macros.

Yeah, maybe.  Otherwise, all that logic is not only implicitly part of
the protocol, but essentially impossible to change because it'll be
hard-coded into clients.  I wasn't too worried about this before because
I supposed that only walsender and walreceiver really needed to know
anything about it.  If Magnus is busy coding something else that speaks
that protocol, we have a problem.  (Assuming we believe his use-case is
sane, that is.)

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] walprotocol.h vs frontends

2011-08-15 Thread Magnus Hagander
On Mon, Aug 15, 2011 at 18:12, Tom Lane t...@sss.pgh.pa.us wrote:
 Heikki Linnakangas heikki.linnakan...@enterprisedb.com writes:
 Perhaps we should change the protocol so that it explicitly says which
 file the streamed piece of WAL belongs to. That way a client could write
 it to the correct file without knowing about all those macros.

 Yeah, maybe.  Otherwise, all that logic is not only implicitly part of
 the protocol, but essentially impossible to change because it'll be
 hard-coded into clients.  I wasn't too worried about this before because
 I supposed that only walsender and walreceiver really needed to know

Wouldn't doing that cause significant overhead? Every single packet
would have to contain the filename, since the wal stream isn't
depending onthe filenames in where it cuts off. Also, the way it is
now a single packet on the replication stream can span multiple WAL
files... (This is the bug in my previous version that I've been able
to fix now)

 anything about it.  If Magnus is busy coding something else that speaks
 that protocol, we have a problem.  (Assuming we believe his use-case is
 sane, that is.)

I am. And said program has existed for almost a year, IIRC. It has
even been on a commitfest for 9.1, but had a bug and I didn't have
time to fix it (it still worked in a lot of cases, but had
cornercases). And a lot of different people have requested said
functionality, so at least they believe it's a usecase (the usecase
being able to stream your backups at a smaller interval than 16Mb).


-- 
 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] walprotocol.h vs frontends

2011-08-15 Thread Peter Geoghegan
On 15 August 2011 16:56, Steve Singer ssin...@ca.afilias.info wrote:
 This would mean that anyone using the floating point timestamps today won't
 be able to use pg_upgrade to upgrade to whichever version we remove them
 from.  8.3 had float based timestamps as the default and I suspect many
 installations with the default 8.3 settings have been upgraded via
 pg_upgrade to 9.0 built the old timestamps representation.

Really? I find that slightly surprising, considering that a quick look
at master's timestamp.c suggests that the choice to use the in64
representation over the double representation is entirely a case of
compile time either/or. There is no apparent fall-back to the double
representation available to binaries built without
--disable-integer-datetimes.

-- 
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] walprotocol.h vs frontends

2011-08-15 Thread Tom Lane
Magnus Hagander mag...@hagander.net writes:
 On Mon, Aug 15, 2011 at 16:53, Tom Lane t...@sss.pgh.pa.us wrote:
 Don't think you should expose fsec_t, nor most of those macros.  The
 foo_per_bar values are just namespace clutter.

 Hmm, ok. I just went for what seemed like a reasonable subset. I do
 also need the SECS_PER_DAY and those constants in order to be able to
 convert the timestamp value.

Uh ... convert?  That seems to require a whole lot more knowledge
about timestamps than what is exposed by the proposed new header.
I thought you were just hoping to compile a struct containing a
TimestampTz field, not actually do anything with that field.

 I don't like the idea of exposing those to frontends, either.  What do
 you actually *need* out of that, and why?

 PrevLogSeg - which also brings in XLogSegsPerFile.
 XLogFileName
 XLogFileSize - which brings in XLogSegsPerFile and XLogSegSize

 PrevLogSeg should be self explaining, as should xlogfilename.

 XLogFileSize is required by XLByteAdvance() which is already in
 xlogdefs.h - so xlogdefs.h actually has a hidden dependency on
 xlog_internal.h here today.

Yeah, and the question remains why is frontend code using any of that?

I'm inclined to think that there is zero chance of this code being
frontend in the sense of being at any real arm's length from backend
implementation details, and so you should just forget these header
maneuvers and do what pg_resetxlog.c does, ie

/*
 * We have to use postgres.h not postgres_fe.h here, because there's so much
 * backend-only stuff in the XLOG include files we need.  But we need a
 * frontend-ish environment otherwise.  Hence this ugly hack.
 */
#define FRONTEND 1

#include postgres.h


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] walprotocol.h vs frontends

2011-08-15 Thread Magnus Hagander
On Mon, Aug 15, 2011 at 18:40, Tom Lane t...@sss.pgh.pa.us wrote:
 Magnus Hagander mag...@hagander.net writes:
 On Mon, Aug 15, 2011 at 16:53, Tom Lane t...@sss.pgh.pa.us wrote:
 Don't think you should expose fsec_t, nor most of those macros.  The
 foo_per_bar values are just namespace clutter.

 Hmm, ok. I just went for what seemed like a reasonable subset. I do
 also need the SECS_PER_DAY and those constants in order to be able to
 convert the timestamp value.

 Uh ... convert?  That seems to require a whole lot more knowledge
 about timestamps than what is exposed by the proposed new header.
 I thought you were just hoping to compile a struct containing a
 TimestampTz field, not actually do anything with that field.

Um, bad choice of words. I don't need to convert, really, but I do
need to stick the current timestamp into a TimestampTz field.

 I don't like the idea of exposing those to frontends, either.  What do
 you actually *need* out of that, and why?

 PrevLogSeg - which also brings in XLogSegsPerFile.
 XLogFileName
 XLogFileSize - which brings in XLogSegsPerFile and XLogSegSize

 PrevLogSeg should be self explaining, as should xlogfilename.

 XLogFileSize is required by XLByteAdvance() which is already in
 xlogdefs.h - so xlogdefs.h actually has a hidden dependency on
 xlog_internal.h here today.

 Yeah, and the question remains why is frontend code using any of that?

 I'm inclined to think that there is zero chance of this code being
 frontend in the sense of being at any real arm's length from backend
 implementation details, and so you should just forget these header
 maneuvers and do what pg_resetxlog.c does, ie

 /*
  * We have to use postgres.h not postgres_fe.h here, because there's so much
  * backend-only stuff in the XLOG include files we need.  But we need a
  * frontend-ish environment otherwise.  Hence this ugly hack.
  */
 #define FRONTEND 1

 #include postgres.h

That does seem to work, yes. I completely forgot about that hack. And
I agree that's probably th eway to do it...

Now I just need to figure out how to get to GetCurrentTimestamp() to
be accessible - or just copy it over under a different name...

-- 
 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] walprotocol.h vs frontends

2011-08-15 Thread Steve Singer

On 11-08-15 12:33 PM, Peter Geoghegan wrote:

On 15 August 2011 16:56, Steve Singerssin...@ca.afilias.info  wrote:

This would mean that anyone using the floating point timestamps today won't
be able to use pg_upgrade to upgrade to whichever version we remove them
from.  8.3 had float based timestamps as the default and I suspect many
installations with the default 8.3 settings have been upgraded via
pg_upgrade to 9.0 built the old timestamps representation.


Really? I find that slightly surprising, considering that a quick look
at master's timestamp.c suggests that the choice to use the in64
representation over the double representation is entirely a case of
compile time either/or. There is no apparent fall-back to the double
representation available to binaries built without
--disable-integer-datetimes.



I *think* the default on 8.3 was float based timestamps.  If you want to 
upgrade a system running 8.3 (that uses float based timestamps) in 
using pg_upgrade you must compile 9.0 (or 8.4 or 9.1) with 
--disable-integer-datetimes.  If at some point in the future you then 
want to upgrade to 9.2 with pg_upgrade you will again need to build 9.2 
with --disable-integer-datetimes.If we remove the code for floating 
point representations of datetime then you won't be able to do that.



Steve

--
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] walprotocol.h vs frontends

2011-08-15 Thread Tom Lane
Steve Singer ssin...@ca.afilias.info writes:
 On 11-08-15 12:33 PM, Peter Geoghegan wrote:
 On 15 August 2011 16:56, Steve Singerssin...@ca.afilias.info  wrote:
 This would mean that anyone using the floating point timestamps today won't
 be able to use pg_upgrade to upgrade to whichever version we remove them
 from.  8.3 had float based timestamps as the default and I suspect many
 installations with the default 8.3 settings have been upgraded via
 pg_upgrade to 9.0 built the old timestamps representation.

 Really?

 I *think* the default on 8.3 was float based timestamps.

Yeah, it was.  Also, the fact that the source distribution switched
defaults at 8.4 has not got a lot to do with what individual binary
distributions did --- some switched earlier, some may still not have
done so.  And of course individual users building from source were
and still are free to make their own choice.

It's going to be quite a long time before we can contemplate removing
float timestamp support altogether.

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] walprotocol.h vs frontends

2011-08-15 Thread Peter Geoghegan
On 15 August 2011 18:09, Steve Singer ssin...@ca.afilias.info wrote:
 Really? I find that slightly surprising, considering that a quick look
 at master's timestamp.c suggests that the choice to use the in64
 representation over the double representation is entirely a case of
 compile time either/or. There is no apparent fall-back to the double
 representation available to binaries built without
 --disable-integer-datetimes.


 I *think* the default on 8.3 was float based timestamps.

Yes, it is.

 If you want to upgrade a system running 8.3 (that uses float based 
 timestamps) in using
 pg_upgrade you must compile 9.0 (or 8.4 or 9.1) with
 --disable-integer-datetimes.  If at some point in the future you then want
 to upgrade to 9.2 with pg_upgrade you will again need to build 9.2 with
 --disable-integer-datetimes.    If we remove the code for floating point
 representations of datetime then you won't be able to do that.

I'm pretty surprised that pg_upgrade pushes that onus onto its users -
for many users, the need to build their own binaries is a greater
barrier to upgrading than doing a logical restore. Maybe that's simply
considered a matter for package managers to worry about, but that
doesn't sit well with me.

-- 
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] walprotocol.h vs frontends

2011-08-15 Thread Simon Riggs
On Mon, Aug 15, 2011 at 5:15 PM, Magnus Hagander mag...@hagander.net wrote:
 On Mon, Aug 15, 2011 at 18:12, Tom Lane t...@sss.pgh.pa.us wrote:
 Heikki Linnakangas heikki.linnakan...@enterprisedb.com writes:
 Perhaps we should change the protocol so that it explicitly says which
 file the streamed piece of WAL belongs to. That way a client could write
 it to the correct file without knowing about all those macros.

 Yeah, maybe.  Otherwise, all that logic is not only implicitly part of
 the protocol, but essentially impossible to change because it'll be
 hard-coded into clients.  I wasn't too worried about this before because
 I supposed that only walsender and walreceiver really needed to know

 Wouldn't doing that cause significant overhead? Every single packet
 would have to contain the filename, since the wal stream isn't
 depending onthe filenames in where it cuts off. Also, the way it is
 now a single packet on the replication stream can span multiple WAL
 files... (This is the bug in my previous version that I've been able
 to fix now)

Why not have a specific protocol message to indicate a change of filename?

I don't mean a WAL message, I mean a streaming protocol message.

At present the WALSender only sends from one file at a time, so
sending a message when we open a new file would be straightforward.

Magnus needs some things to make this work for 9.0/9.1, but we don't
need to change 9.2 to allow that, we just need to copy the definitions
for those now-fixed releases.

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

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


Re: [HACKERS] walprotocol.h vs frontends

2011-08-15 Thread Magnus Hagander
On Mon, Aug 15, 2011 at 21:12, Simon Riggs si...@2ndquadrant.com wrote:
 On Mon, Aug 15, 2011 at 5:15 PM, Magnus Hagander mag...@hagander.net wrote:
 On Mon, Aug 15, 2011 at 18:12, Tom Lane t...@sss.pgh.pa.us wrote:
 Heikki Linnakangas heikki.linnakan...@enterprisedb.com writes:
 Perhaps we should change the protocol so that it explicitly says which
 file the streamed piece of WAL belongs to. That way a client could write
 it to the correct file without knowing about all those macros.

 Yeah, maybe.  Otherwise, all that logic is not only implicitly part of
 the protocol, but essentially impossible to change because it'll be
 hard-coded into clients.  I wasn't too worried about this before because
 I supposed that only walsender and walreceiver really needed to know

 Wouldn't doing that cause significant overhead? Every single packet
 would have to contain the filename, since the wal stream isn't
 depending onthe filenames in where it cuts off. Also, the way it is
 now a single packet on the replication stream can span multiple WAL
 files... (This is the bug in my previous version that I've been able
 to fix now)

 Why not have a specific protocol message to indicate a change of filename?

 I don't mean a WAL message, I mean a streaming protocol message.

That we could have, and it would work as long as it's always sent as
the first packet in any stream.

If we do that, we should probably make it a general metadata package -
including things like segment size as well...


 At present the WALSender only sends from one file at a time, so
 sending a message when we open a new file would be straightforward.

Are you sure? We can receive a single message spanning multiple files...


 Magnus needs some things to make this work for 9.0/9.1, but we don't
 need to change 9.2 to allow that, we just need to copy the definitions
 for those now-fixed releases.

The hack from pg_resetxlog with a manual #define FRONTEND 1 will work
for current releases, I think. And it will work for future ones as
well - but you'll be in a position where using the log streaming tool
built with different parameters (like xlog seg size) than the server
will not work - I'm not sure that's a problem big enough to worry
about, though...

-- 
 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] walprotocol.h vs frontends

2011-08-15 Thread Dimitri Fontaine
Magnus Hagander mag...@hagander.net writes:
 Why not have a specific protocol message to indicate a change of filename?
 I don't mean a WAL message, I mean a streaming protocol message.

 That we could have, and it would work as long as it's always sent as
 the first packet in any stream.

 If we do that, we should probably make it a general metadata package -
 including things like segment size as well...

I think this could be made an opt-in: the streaming client would have to
ask for it when connecting to the walsender (or maybe whenever in the
connection stream, I don't know).

Then it can even be backported, and it won't add anything to core
clients streaming data.

Regards,
-- 
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support

-- 
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] walprotocol.h vs frontends

2011-08-15 Thread Simon Riggs
On Mon, Aug 15, 2011 at 10:32 PM, Magnus Hagander mag...@hagander.net wrote:

 At present the WALSender only sends from one file at a time, so
 sending a message when we open a new file would be straightforward.

 Are you sure? We can receive a single message spanning multiple files...

You're right. That was the way the original code ran but I thought we
had stopped that when we introduced MAX_SEND_SIZE. The comment says
don't cross a logfile boundary within one message. What that
actually does is prevent us incrementing a logid value, which happens
every 255 files. I read that as meaning WAL file which is not what
it means at all.

So right now what we do is allow a single packet to span multiple
files, but since MAX_SEND_SIZE is 128KB it will always be smaller than
a single file, so we can only ever span two files at most.

That is all just a little bizarre, especially since libpq sends data
in 8KB chunks anyway.

So I suggest we change XLogSend() so that it only ever sends up to the
end of a file. That way all w messages will relate to just one file,
and we can have another message to initiate a new file. And then, as
you say, give full metadata for the new file.

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

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