pgsql: Sigh ...

Started by Nonameover 17 years ago15 messages
#1Noname
tgl@postgresql.org

Log Message:
-----------
Sigh ... pg_config.h.win32 needs to define BLCKSZ and RELSEG_SIZE now.

Modified Files:
--------------
pgsql/src/include:
pg_config.h.win32 (r1.53 -> r1.54)
(http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/include/pg_config.h.win32?r1=1.53&r2=1.54)

#2Andrew Dunstan
andrew@dunslane.net
In reply to: Noname (#1)
Re: pgsql: Sigh ...

Tom Lane wrote:

Log Message:
-----------
Sigh ... pg_config.h.win32 needs to define BLCKSZ and RELSEG_SIZE now.

Tom,

this reminds me a bit of Talleyrand's reply to the beggar who said to
him "Monseigneur, il faut que je vive," ... "Je n'en vois pas la
n�cessit�.".

This fix is surely wrong for several reasons:
. the configure changes only broke MSVC builds, not all Windows builds
(see narwhal, for example), but this change applies to both.
. fixing a change that adds a configure option by hardcoding it in
pg_config.h.win32 is simply the wrong fix - the right fix is to add the
equivalent logic to src/tools/Solution.pm.

I don't mind if you ask someone (realistically that will usually be
Magnus or me) to unbreak MSVC builds due to a configure change, because
you are not set up to test it yourself. But I do mind the wrong solution
being applied just to unbreak the buildfarm.

I make corrective surgery in the morning.

cheers

andrew

#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andrew Dunstan (#2)
Re: [COMMITTERS] pgsql: Sigh ...

Andrew Dunstan <andrew@dunslane.net> writes:

This fix is surely wrong for several reasons:
. the configure changes only broke MSVC builds, not all Windows builds
(see narwhal, for example), but this change applies to both.
. fixing a change that adds a configure option by hardcoding it in
pg_config.h.win32 is simply the wrong fix - the right fix is to add the
equivalent logic to src/tools/Solution.pm.

Well, maybe the right answer is to take a step back and figure out what
pg_config.h.win32's excuse for living is at all. I do not understand
our Windows configuration setup, and what I do understand is is that
it's a pile of horrid kluges that break anytime anyone looks at them
sideways.

I will be quite happy to never touch any Windows configuration stuff
again. If you want me to, you had better redesign and/or document it
so that people other than you and Magnus have some idea of what connects
to what.

regards, tom lane

#4Andrew Dunstan
andrew@dunslane.net
In reply to: Tom Lane (#3)
Re: [COMMITTERS] pgsql: Sigh ...

Tom Lane wrote:

Andrew Dunstan <andrew@dunslane.net> writes:

This fix is surely wrong for several reasons:
. the configure changes only broke MSVC builds, not all Windows builds
(see narwhal, for example), but this change applies to both.
. fixing a change that adds a configure option by hardcoding it in
pg_config.h.win32 is simply the wrong fix - the right fix is to add the
equivalent logic to src/tools/Solution.pm.

Well, maybe the right answer is to take a step back and figure out what
pg_config.h.win32's excuse for living is at all. I do not understand
our Windows configuration setup, and what I do understand is is that
it's a pile of horrid kluges that break anytime anyone looks at them
sideways.

I will be quite happy to never touch any Windows configuration stuff
again. If you want me to, you had better redesign and/or document it
so that people other than you and Magnus have some idea of what connects
to what.

Oops, the file is only used by MSVC/BCC, not by MinGW. Sorry for the
mistake about that.

However, all the values are hardcoded, so nothing in it should relate to
settings that come from configure, I believe. These should be dealt with
in src/tools/msvc/Solution.pm (mostly in GenerateFiles() ).

cheers

andrew

#5Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andrew Dunstan (#4)
Re: [COMMITTERS] pgsql: Sigh ...

Andrew Dunstan <andrew@dunslane.net> writes:

Tom Lane wrote:

Well, maybe the right answer is to take a step back and figure out what
pg_config.h.win32's excuse for living is at all.

Oops, the file is only used by MSVC/BCC, not by MinGW. Sorry for the
mistake about that.

Well, I'm still wondering why it is an input for MSVC. If we have a
configure substitute for MSVC, why isn't it working off pg_config.h.in?

The original idea of pg_config.h.win32 was to support the pre-8.0
method for building native libpq.dll. I'd like to see us obsolete that
method altogether and get rid of pg_config.h.win32.

However, all the values are hardcoded, so nothing in it should relate to
settings that come from configure, I believe. These should be dealt with
in src/tools/msvc/Solution.pm (mostly in GenerateFiles() ).

FYI, I'm about to commit changes moving XLOG_BLCKSZ and XLOG_SEG_SIZE
into the domain of configurable stuff, too.

regards, tom lane

#6Andrew Dunstan
andrew@dunslane.net
In reply to: Tom Lane (#5)
Re: [COMMITTERS] pgsql: Sigh ...

Tom Lane wrote:

Andrew Dunstan <andrew@dunslane.net> writes:

Tom Lane wrote:

Well, maybe the right answer is to take a step back and figure out what
pg_config.h.win32's excuse for living is at all.

Oops, the file is only used by MSVC/BCC, not by MinGW. Sorry for the
mistake about that.

Well, I'm still wondering why it is an input for MSVC. If we have a
configure substitute for MSVC, why isn't it working off pg_config.h.in?

The original idea of pg_config.h.win32 was to support the pre-8.0
method for building native libpq.dll. I'd like to see us obsolete that
method altogether and get rid of pg_config.h.win32.

That makes sense - I'll look at that after I have cleaned up the current
stuff.

However, all the values are hardcoded, so nothing in it should relate to
settings that come from configure, I believe. These should be dealt with
in src/tools/msvc/Solution.pm (mostly in GenerateFiles() ).

FYI, I'm about to commit changes moving XLOG_BLCKSZ and XLOG_SEG_SIZE
into the domain of configurable stuff, too.

So I gathered. I'll pick those up as part of the fixes I'm currently coding.

cheers

andrew

#7Andrew Dunstan
andrew@dunslane.net
In reply to: Tom Lane (#5)
1 attachment(s)
Re: [COMMITTERS] pgsql: Sigh ...

Tom Lane wrote:

However, all the values are hardcoded, so nothing in it should relate to
settings that come from configure, I believe. These should be dealt with
in src/tools/msvc/Solution.pm (mostly in GenerateFiles() ).

FYI, I'm about to commit changes moving XLOG_BLCKSZ and XLOG_SEG_SIZE
into the domain of configurable stuff, too.

This patch should fix things for both sets of changes. And it
demonstrates pretty much what you need to do for config options for MSVC.

If there's no objection I will apply shortly.

cheers

andrew

Attachments:

mscvfix.patchtext/x-patch; charset=iso-8859-1; name=mscvfix.patchDownload
Index: src/include/pg_config.h.win32
===================================================================
RCS file: /cvsroot/pgsql/src/include/pg_config.h.win32,v
retrieving revision 1.54
diff -c -r1.54 pg_config.h.win32
*** src/include/pg_config.h.win32	2 May 2008 03:41:46 -0000	1.54
--- src/include/pg_config.h.win32	2 May 2008 22:04:37 -0000
***************
*** 37,51 ****
  /* The alignment requirement of a `short'. */
  #define ALIGNOF_SHORT 2
  
- /* Size of a disk block --- this also limits the size of a tuple. You can set
-    it bigger if you need bigger tuples (although TOAST should reduce the need
-    to have large tuples, since fields can be spread across multiple tuples).
-    BLCKSZ must be a power of 2. The maximum possible value of BLCKSZ is
-    currently 2^15 (32768). This is determined by the 15-bit widths of the
-    lp_off and lp_len fields in ItemIdData (see include/storage/itemid.h).
-    Changing BLCKSZ requires an initdb. */
- #define BLCKSZ 8192
- 
  /* Define to the default TCP port number on which the server listens and to
     which clients will try to connect. This can be overridden at run-time, but
     it's convenient if your clients have the right default compiled in.
--- 37,42 ----
***************
*** 600,618 ****
     your system. */
  /* #undef PTHREAD_CREATE_JOINABLE */
  
- /* RELSEG_SIZE is the maximum number of blocks allowed in one disk file. Thus,
-    the maximum size of a single file is RELSEG_SIZE * BLCKSZ; relations bigger
-    than that are divided into multiple files. RELSEG_SIZE * BLCKSZ must be
-    less than your OS' limit on file size. This is often 2 GB or 4GB in a
-    32-bit operating system, unless you have large file support enabled. By
-    default, we make the limit 1 GB to avoid any possible integer-overflow
-    problems within the OS. A limit smaller than necessary only means we divide
-    a large relation into more chunks than necessary, so it seems best to err
-    in the direction of a small limit. A power-of-2 value is recommended to
-    save a few cycles in md.c, but is not absolutely required. Changing
-    RELSEG_SIZE requires an initdb. */
- #define RELSEG_SIZE 131072
- 
  /* The size of a `size_t', as computed by sizeof. */
  #define SIZEOF_SIZE_T 4
  
--- 591,596 ----
Index: src/tools/msvc/Solution.pm
===================================================================
RCS file: /cvsroot/pgsql/src/tools/msvc/Solution.pm,v
retrieving revision 1.40
diff -c -r1.40 Solution.pm
*** src/tools/msvc/Solution.pm	21 Apr 2008 18:37:28 -0000	1.40
--- src/tools/msvc/Solution.pm	2 May 2008 22:04:39 -0000
***************
*** 34,39 ****
--- 34,56 ----
              die "XML requires both XSLT and ICONV\n";
          }
      }
+ 	$options->{blocksize} = 8
+ 		unless $options->{blocksize}; # undef or 0 means default
+ 	die "Bad blocksize $options->{blocksize}"
+ 		unless grep {$_ == $options->{blocksize}} (1,2,4,8,16,32);
+ 	$options->{segsize} = 1
+ 		unless $options->{segsize}; # undef or 0 means default
+ 	# only allow segsize 1 for now, as we can't do large files yet in windows
+ 	die "Bad segsize $options->{segsize}"
+ 		unless $options->{segsize} == 1;
+ 	$options->{wal_blocksize} = 8
+ 		unless $options->{wal_blocksize}; # undef or 0 means default
+ 	die "Bad wal_blocksize $options->{wal_blocksize}"
+ 		unless grep {$_ == $options->{wal_blocksize}} (1,2,4,8,16,32,64);
+ 	$options->{wal_segsize} = 16
+ 		unless $options->{wal_segsize}; # undef or 0 means default
+ 	die "Bad wal_segsize $options->{wal_segsize}"
+ 		unless grep {$_ == $options->{wal_segsize}} (1,2,4,8,16,32,64);
      return $self;
  }
  
***************
*** 116,122 ****
          print O "#define USE_LDAP 1\n" if ($self->{options}->{ldap});
          print O "#define HAVE_LIBZ 1\n" if ($self->{options}->{zlib});
          print O "#define USE_SSL 1\n" if ($self->{options}->{openssl});
!         print O "#define ENABLE_NLS 1\n" if ($self->{options}->{nls});
          
          if ($self->{options}->{float4byval}) 
          {
--- 133,148 ----
          print O "#define USE_LDAP 1\n" if ($self->{options}->{ldap});
          print O "#define HAVE_LIBZ 1\n" if ($self->{options}->{zlib});
          print O "#define USE_SSL 1\n" if ($self->{options}->{openssl});
! 		print O "#define ENABLE_NLS 1\n" if ($self->{options}->{nls});
! 
! 		print O "#define BLCKSZ ",1024 * $self->{options}->{blocksize},"\n";
! 		print O "#define RELSEG_SIZE ",
! 			(1024 / $self->{options}->{blocksize}) * 
! 				$self->{options}->{segsize} * 1024, "\n";
! 		print O "#define XLOG_BLCKSZ ",
! 			1024 * $self->{options}->{wal_blocksize},"\n";
! 		print O "#define XLOG_SEG_SIZE (",
! 			$self->{options}->{wal_segsize}," * 1024 * 1024)\n";
          
          if ($self->{options}->{float4byval}) 
          {
Index: src/tools/msvc/config.pl
===================================================================
RCS file: /cvsroot/pgsql/src/tools/msvc/config.pl,v
retrieving revision 1.12
diff -c -r1.12 config.pl
*** src/tools/msvc/config.pl	21 Apr 2008 10:01:32 -0000	1.12
--- src/tools/msvc/config.pl	2 May 2008 22:04:39 -0000
***************
*** 7,12 ****
--- 7,15 ----
      # integer_datetimes=>1,   # --enable-integer-datetimes - on is now default
      # float4byval=>1,         # --disable-float4-byval, on by default
      # float8byval=>0,         # --disable-float8-byval, off by default
+     # blocksize => 8,         # --with-blocksize, 8kB by default
+     # wal_blocksize => 8,     # --with-wal-blocksize, 8kb by default
+     # wal_segsize => 16,      # --with-wal-segsize, 16MB by default
      nls=>undef,				# --enable-nls=<path>
      tcl=>'c:\tcl',		# --with-tls=<path>
      perl=>'c:\perl', 			# --with-perl
#8Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andrew Dunstan (#7)
Re: [COMMITTERS] pgsql: Sigh ...

Andrew Dunstan <andrew@dunslane.net> writes:

! print O "#define RELSEG_SIZE ",
! (1024 / $self->{options}->{blocksize}) *
! $self->{options}->{segsize} * 1024, "\n";

This doesn't look quite right; unless the arithmetic is being done in
floating point? I had it like this in configure.in:

RELSEG_SIZE=`expr '(' 1024 '*' ${segsize} / ${blocksize} ')' '*' 1024`

Also it looks like you missed adding segsize to the config.pl comments.

regards, tom lane

#9Andrew Dunstan
andrew@dunslane.net
In reply to: Tom Lane (#8)
Re: [COMMITTERS] pgsql: Sigh ...

Tom Lane wrote:

Andrew Dunstan <andrew@dunslane.net> writes:

! print O "#define RELSEG_SIZE ",
! (1024 / $self->{options}->{blocksize}) *
! $self->{options}->{segsize} * 1024, "\n";

This doesn't look quite right; unless the arithmetic is being done in
floating point? I had it like this in configure.in:

RELSEG_SIZE=`expr '(' 1024 '*' ${segsize} / ${blocksize} ')' '*' 1024`

blocksize is one of (1,2,4,8,16,32) so it should always be a factor of
1024 unless my arithmetic is awry. I did it that way because I dislike
expressions with unbracketed mixed operations - they make me think too
much.

Also it looks like you missed adding segsize to the config.pl comments.

That's deliberate - we are currently only allowing a value of 1 here, so
I don't see any point in putting it in the sample config file, even as a
comment. When we enable other seg sizes we can add it to the sample file.

cheers

andrew

#10Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andrew Dunstan (#9)
Re: [COMMITTERS] pgsql: Sigh ...

Andrew Dunstan <andrew@dunslane.net> writes:

Tom Lane wrote:

This doesn't look quite right; unless the arithmetic is being done in
floating point? I had it like this in configure.in:

RELSEG_SIZE=`expr '(' 1024 '*' ${segsize} / ${blocksize} ')' '*' 1024`

blocksize is one of (1,2,4,8,16,32) so it should always be a factor of
1024 unless my arithmetic is awry. I did it that way because I dislike
expressions with unbracketed mixed operations - they make me think too
much.

Well, if you dislike the original on style grounds, you should change it
to match. Doing the same thing in two different ways in two places
isn't good.

Also it looks like you missed adding segsize to the config.pl comments.

That's deliberate - we are currently only allowing a value of 1 here, so
I don't see any point in putting it in the sample config file, even as a
comment. When we enable other seg sizes we can add it to the sample file.

Ah, OK.

regards, tom lane

#11Andrew Dunstan
andrew@dunslane.net
In reply to: Tom Lane (#10)
Re: [COMMITTERS] pgsql: Sigh ...

Tom Lane wrote:

Andrew Dunstan <andrew@dunslane.net> writes:

Tom Lane wrote:

This doesn't look quite right; unless the arithmetic is being done in
floating point? I had it like this in configure.in:

RELSEG_SIZE=`expr '(' 1024 '*' ${segsize} / ${blocksize} ')' '*' 1024`

blocksize is one of (1,2,4,8,16,32) so it should always be a factor of
1024 unless my arithmetic is awry. I did it that way because I dislike
expressions with unbracketed mixed operations - they make me think too
much.

Well, if you dislike the original on style grounds, you should change it
to match. Doing the same thing in two different ways in two places
isn't good.

OK, done. Patch applied with that addition (it was time I deployed
autoconf 2.61 anyway).

cheers

andrew

#12Peter Eisentraut
peter_e@gmx.net
In reply to: Andrew Dunstan (#7)
Re: [COMMITTERS] pgsql: Sigh ...

Andrew Dunstan wrote:

This patch should fix things for both sets of changes. And it
demonstrates pretty much what you need to do for config options for MSVC.

Btw., it is quite easily possible to use the autom4te tracing facility to
parse the configure.ac file, in case you are interested in generating some of
the Windows build code automatically.

For example:

$ autoconf -t 'AC_ARG_ENABLE:$1' configure.in
integer-datetimes
nls
shared
rpath
spinlocks
debug
profiling
dtrace
segmented-files
depend
cassert
thread-safety
thread-safety
thread-safety-force
largefile

Let me know if you want to do something with that.

#13Andrew Dunstan
andrew@dunslane.net
In reply to: Peter Eisentraut (#12)
Re: [COMMITTERS] pgsql: Sigh ...

Peter Eisentraut wrote:

Andrew Dunstan wrote:

This patch should fix things for both sets of changes. And it
demonstrates pretty much what you need to do for config options for MSVC.

Btw., it is quite easily possible to use the autom4te tracing facility to
parse the configure.ac file, in case you are interested in generating some of
the Windows build code automatically.

For example:

$ autoconf -t 'AC_ARG_ENABLE:$1' configure.in
integer-datetimes
nls
shared
rpath
spinlocks
debug
profiling
dtrace
segmented-files
depend
cassert
thread-safety
thread-safety
thread-safety-force
largefile

Let me know if you want to do something with that.

Yeah, maybe. Let's fix the issue pg_config.h.win32 that Tom raised first.

cheers

andrew

#14Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andrew Dunstan (#13)
Re: [COMMITTERS] pgsql: Sigh ...

Andrew Dunstan <andrew@dunslane.net> writes:

Peter Eisentraut wrote:

Btw., it is quite easily possible to use the autom4te tracing facility to
parse the configure.ac file, in case you are interested in generating some of
the Windows build code automatically.

Yeah, maybe. Let's fix the issue pg_config.h.win32 that Tom raised first.

+1 for both. We really need to eliminate as much redundancy as we can
between the two build systems, because it'll keep biting us until we do.

regards, tom lane

#15Magnus Hagander
magnus@hagander.net
In reply to: Tom Lane (#14)
Re: [COMMITTERS] pgsql: Sigh ...

Tom Lane wrote:

Andrew Dunstan <andrew@dunslane.net> writes:

Peter Eisentraut wrote:

Btw., it is quite easily possible to use the autom4te tracing facility to
parse the configure.ac file, in case you are interested in generating some of
the Windows build code automatically.

Yeah, maybe. Let's fix the issue pg_config.h.win32 that Tom raised first.

+1 for both. We really need to eliminate as much redundancy as we can
between the two build systems, because it'll keep biting us until we do.

+1 from here as well, if that wasn' obvious :-)

(will get back to the actual issues at hand when I get back in a more
connected mode in a couple of days, unless they are already solved by then)

//Magnus