Re: [HACKERS] PostgreSQL 10 (latest beta) and older ICU

[Victor Wagner]

On Tue, 1 Aug 2017 08:16:54 -0400
Peter Eisentraut <peter.eisentr...@2ndquadrant.com> wrote:

> On 8/1/17 02:12, Victor Wagner wrote:
> >> We are only calling uloc_toLanguageTag() with keyword/value
> >> combinations that ICU itself previously told us were supported.  So
> >> just ignoring errors doesn't seem proper in this case.
> >>  
> > We know that this version of ICU is broken. But what choice we
> > have?  
> 
> I don't know that we can already reach that conclusion.  Maybe the

Because it was fixed in subsequent versions.

And 4.2 is first version where this function appeared.
So, we still have problems with SLES11 which use even older version and
have to be supported at least two more years.


> APIs have changed or we are not using them correctly.  Are there any
> bug reports or changelog entries related to this?  I don't think we
> should just give up and ignore errors.

We also can provide configuration option or command-line option for
initdb which would restrict list of languages for which collation
sequences would be created. This would help for all but two languages.





-- 
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] PostgreSQL 10 (latest beta) and older ICU

[Victor Wagner]

On Mon, 31 Jul 2017 19:42:30 -0400
Peter Eisentraut <peter.eisentr...@2ndquadrant.com> wrote:

> On 7/25/17 15:20, Victor Wagner wrote:
> > It turns out, that PostgreSQL enumerates collations for all ICU
> > locales and passes it into uloc_toLanguageTag function with strict
> > argument of this function set to TRUE. But for some locales
> > (es*@collation=tradtiional, si*@collation=dictionary) only call with
> > strict=FALSE succeeds in ICU 4.2. In newer versions of ICU all
> > locales can be resolved with strict=TRUE.  
> 
> We are only calling uloc_toLanguageTag() with keyword/value
> combinations that ICU itself previously told us were supported.  So
> just ignoring errors doesn't seem proper in this case.
> 

We know that this version of ICU is broken. But what choice we have?
Locale code in the glibc is much more broken.
So we just have to work around bugs in underlaying  libraries anyway.
In case of ICU we just need to omit some collations.

It might cause incompatibility with newer systems where these
collations are used, but if we fall back to glibc locale, there would
be much more incompatibilites. And also there is bug in strxfrm, which
prevents use of abbreviated keys.

-- 


-- 
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] PostgreSQL 10 (latest beta) and older ICU

[Victor Wagner]

On Tue, 25 Jul 2017 16:50:38 -0400
Tom Lane <t...@sss.pgh.pa.us> wrote:

> Victor Wagner <vi...@wagner.pp.ru> writes:
> > PostgreSQL 10 when build with --with-icu requires ICU 4.6 and above.
> > (because it searches for libicu using pkgconfig, and pgkconfig
> > support significantly changed in ICU  version 4.6)  
> 
> > However, there are some reasons to build PostgreSQL with older
> > versions of ICU.   
> 
> No doubt, but making that happen seems like a feature, and IMO we're
> too late in the v10 beta test cycle to be taking new features on

May be PGDG people could integrate it as a patch for RPMS for
particular systems which are affected by the problem.

I'd rather say that it is bugfix. Relaxing too strict checking.
If we choose approach to allow non-strict language tags, it is oneline
patch.

If we choose more safe approach to ignore non-strict collations, it
would take about five lines 
1. Replace one ereport(ERROR with ereport(WARINING
2. Return special value (NULL) after issuing this warning
3. Handle this special value in calling function by continuing to next
loop iteration
4,5 - curly braces around 1 and 2. see patch at the end

> board, especially ones with inherently-large portability risks.  We

I don't think that there are so large portability risks. Patch can be
done such way that it would behave exactly as now if ICU version is 4.6
or above. Moreover, it is easy to make old ICU support separate
configure option (because another configure test is needed anyway).

> could maybe consider patches for this for v11 ... but will anyone
> still care by then?

Undoubtedly will. v11 is expedted to be released in 2018. And RHEL 6
is supported until 2020, and extended support would end in Nov 2023.

And it is possible that some derived distribution would last longer.


> (Concretely, my concern is that the alpha and beta testing that's
> happened so far hasn't covered pre-4.6 ICU at all.  I'd be surprised
> if the incompatibility you found so far is the only one.)

diff --git a/src/backend/commands/collationcmds.c 
b/src/backend/commands/collationcmds.c
index b6c14c9..9e5da98 100644
--- a/src/backend/commands/collationcmds.c
+++ b/src/backend/commands/collationcmds.c
@@ -442,11 +442,13 @@ get_icu_language_tag(const char *localename)
 
status = U_ZERO_ERROR;
uloc_toLanguageTag(localename, buf, sizeof(buf), TRUE, );
-   if (U_FAILURE(status))
-   ereport(ERROR,
+   if (U_FAILURE(status)) 
+   {
+   ereport(WARNING,
(errmsg("could not convert locale name \"%s\" 
to language tag: %s",
localename, 
u_errorName(status;
-
+   return NULL;
+   }
return pstrdup(buf);
 }

@@ -733,6 +735,10 @@ pg_import_system_collations(PG_FUNCTION_ARGS)
char   *localeid = 
psprintf("%s@collation=%s", name, val);
 
langtag = get_icu_language_tag(localeid);
+   if (langtag == NULL) {
+   /* No such collation in library, skip */
+   continue;
+   }
collcollate = U_ICU_VERSION_MAJOR_NUM >= 54 ? 
langtag : localeid;
 
/*






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

[HACKERS] PostgreSQL 10 (latest beta) and older ICU

[Victor Wagner]

Collegues,

PostgreSQL 10 when build with --with-icu requires ICU 4.6 and above.
(because it searches for libicu using pkgconfig, and pgkconfig support
significantly changed in ICU  version 4.6)

However, there are some reasons to build PostgreSQL with older versions
of ICU. 

For instance such Linux distributions as RHEL 6 ships ICU 4.2, and SLES
11 also ships older ICU. Sometimes, it is not feasible to compile newer
ICU from sources on the servers with these (and derived) distributions.

It is possible to compile PostgreSQL 10 with these versions of ICU
by specifying ICU_CFLAGS and ICU_LIBS explicitely.

However, backend startup fails with errors on some Spanish and
Singalese locales.

It turns out, that PostgreSQL enumerates collations for all ICU locales
and passes it into uloc_toLanguageTag function with strict argument of
this function set to TRUE. But for some locales
(es*@collation=tradtiional, si*@collation=dictionary) only call with
strict=FALSE succeeds in ICU 4.2. In newer versions of ICU all locales
can be resolved with strict=TRUE.

ICU docs state that if strict=FALSE, some locale fields can be
incomplete.

What solition is better:

1. Just skip locale/collation combinations which cannot be resolved
with strict=TRUE and issue warning instead of error if
uloc_toLanguageTag fails on some locale.

It would cause some ICU collations be missiong from the databases
running on the systems with old ICU.

2. If we see ICU version earlier than 4.6, pass strict=FALSE to
ucol_toLanguageTag.  It would cause databases on such systems use
fallback collation sequence for these collations.

Personally I prefer first solition, because I doubt that any of my
users would ever need Singalese collation, and probably they can
survive without traditional Spanish too.

-- 
   Victor Wagner <vi...@wagner.pp.ru>


-- 
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] Explicit subtransactions for PL/Tcl

[Victor Wagner]

On Thu, 9 Mar 2017 12:04:31 +0100
Pavel Stehule <pavel.steh...@gmail.com> wrote:


> > Now test demonstrate how errors uncaught on the Tcl level interact
> > with postgresql error system.
> >
> 
> you can catch the exception outside and write own message

OK, here is patch with tests which don't depend on function OIDs

They ignore stack trace ($::errorinfo global variable) completely,
and analyze just error message.


-- 
       Victor Wagner <vi...@wagner.pp.ru>
diff --git a/doc/src/sgml/pltcl.sgml b/doc/src/sgml/pltcl.sgml
index ad216dd..87bc4ad 100644
--- a/doc/src/sgml/pltcl.sgml
+++ b/doc/src/sgml/pltcl.sgml
@@ -901,6 +901,79 @@ if {[catch { spi_exec $sql_command }]} {
  is a global variable.)
 

+   
+   Explicit Subtransactions
+  
+   Recovering from errors caused by database access as described in
+can lead to an undesirable
+   situation where some operations succeed before one of them fails,
+   and after recovering from that error the data is left in an
+   inconsistent state.  PL/Tcl offers a solution to this problem in
+   the form of explicit subtransactions.
+  
+   
+Consider a function that implements a transfer between two
+accounts:
+
+CREATE FUNCTION transfer_funds() RETURNS void AS $$
+if [catch {
+spi_exec "UPDATE accounts SET balance = balance - 100 WHERE account_name = 'joe'"
+spi_exec "UPDATE accounts SET balance = balance + 100 WHERE account_name = 'mary'"
+} errormsg] {
+set result [format "error transferring funds: %s" $errormsg ]
+} else {
+set result "funds transferred correctly"
+}
+set plan [spi_prepare "INSERT INTO operations (result) VALUES ($1)"]
+spi_execp -count 1 $plan, [list $result)
+$$ LANGUAGE pltclu;
+
+If the second UPDATE statement results in an
+exception being raised, this function will report the error, but
+the result of the first UPDATE will
+nevertheless be committed.  In other words, the funds will be
+withdrawn from Joe's account, but will not be transferred to
+Mary's account.
+   
+   
+To avoid such issues, you can wrap your
+spi_exec calls in an explicit
+subtransaction.  The PL/Tcl provides a
+commmand subtransaction to manage explicit
+	subtransactions.
+ Using explicit subtransactions
+we can rewrite our function as:
+
+CREATE FUNCTION transfer_funds2() RETURNS void AS $$
+if [catch {
+	subtransaction {
+spi_exec "UPDATE accounts SET balance = balance - 100 WHERE account_name = 'joe'"
+spi_exec "UPDATE accounts SET balance = balance + 100 WHERE account_name = 'mary'"
+	}
+	} errormsg] {
+		set result [format "error transferring funds: %s" $errormsg]
+	} else {
+   set result "funds transferred correctly"
+	}
+set plan  [spi_prepare "INSERT INTO operations (result) VALUES ($1)"]
+spi_execp $plan, [list $result]
+$$ LANGUAGE pltclu;
+
+Note that the use of catch is still
+required.  Otherwise the exception would propagate to the top of
+the  stack and would cause the whole function to abort with
+a PostgreSQL error, so that the
+operations table would not have any row
+inserted into it.  The subtransaction command does not
+trap errors, it only assures that all database operations executed
+inside its scope will be atomically committed or rolled back.  A
+rollback of the subtransaction block occurs on any kind of
+exception exit, not only ones caused by errors originating from
+database access.  A regular Tcl exception raised inside an
+explicit subtransaction block would also cause the subtransaction
+to be rolled back.
+   
+  
 

 PL/Tcl Configuration
diff --git a/src/pl/tcl/Makefile b/src/pl/tcl/Makefile
index 1096c4f..b6b6b19 100644
--- a/src/pl/tcl/Makefile
+++ b/src/pl/tcl/Makefile
@@ -28,7 +28,7 @@ DATA = pltcl.control pltcl--1.0.sql pltcl--unpackaged--1.0.sql \
pltclu.control pltclu--1.0.sql pltclu--unpackaged--1.0.sql
 
 REGRESS_OPTS = --dbname=$(PL_TESTDB) --load-extension=pltcl
-REGRESS = pltcl_setup pltcl_queries pltcl_start_proc pltcl_unicode
+REGRESS = pltcl_setup pltcl_queries pltcl_start_proc pltcl_unicode pltcl_subxact
 
 # Tcl on win32 ships with import libraries only for Microsoft Visual C++,
 # which are not compatible with mingw gcc. Therefore we need to build a
diff --git a/src/pl/tcl/expected/pltcl_subxact.out b/src/pl/tcl/expected/pltcl_subxact.out
new file mode 100644
index 000..3ea3ef3
--- /dev/null
+++ b/src/pl/tcl/expected/pltcl_subxact.out
@@ -0,0 +1,146 @@
+--
+-- Test explicit subtransactions
+--
+CREATE TABLE subtransaction_tbl (
+i integer
+);
+-- test subtransaction successfully commited
+CREATE FUNCTION subtransaction_ctx_success() RETURNS text
+AS $$
+	spi_exec "INSERT INTO subtransaction_tbl VALUES(1)"
+subtransaction {
+		spi_exec "

Re: [HACKERS] Explicit subtransactions for PL/Tcl

[Victor Wagner]

On Thu, 9 Mar 2017 11:12:09 +0100
Pavel Stehule <pavel.steh...@gmail.com> wrote:


> the regress tests is unstable
> 
> the proc name has mutable pid
> 
> ! (procedure "__PLTcl_proc_16503" line 3)
>   invoked from within
> ! "__PLTcl_proc_16503 SPI"
> 
> Regards

Really, I don't know what can be done with it, short of rewriting all
tests as tap-tests.

Definitely this patch is not the right place for reversing desing
decision of PL/Tcl authors to add a numeric suffix to the proc names.
(it is not PID. It is OID of the function).

Of course, I can catch all the errors inside Tcl code and return
just message, but it would sufficiently narrow test functionality.

Now test demonstrate how errors uncaught on the Tcl level interact with
postgresql error system.



     With best regards, Victor

--
 Victor Wagner <vi...@wagner.pp.ru>



-- 
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] Explicit subtransactions for PL/Tcl

[Victor Wagner]

On Thu, 9 Mar 2017 09:25:14 +0100
Pavel Stehule <pavel.steh...@gmail.com> wrote:


> >
> is this patch complete? I don't see new regress tests

Oh, really! I've forgot that git diff doesn't include files which are
not added into git.

So, no old regress tests as well.

Sorry for posting incomplete patch.

Attached fixed version of patch with regress tests and couple more
whitespace issues fixed.

 With best regards, Victor

--
        Victor Wagner <vi...@wagner.pp.ru>


diff --git a/doc/src/sgml/pltcl.sgml b/doc/src/sgml/pltcl.sgml
index ad216dd..87bc4ad 100644
--- a/doc/src/sgml/pltcl.sgml
+++ b/doc/src/sgml/pltcl.sgml
@@ -901,6 +901,79 @@ if {[catch { spi_exec $sql_command }]} {
  is a global variable.)
 

+   
+   Explicit Subtransactions
+  
+   Recovering from errors caused by database access as described in
+can lead to an undesirable
+   situation where some operations succeed before one of them fails,
+   and after recovering from that error the data is left in an
+   inconsistent state.  PL/Tcl offers a solution to this problem in
+   the form of explicit subtransactions.
+  
+   
+Consider a function that implements a transfer between two
+accounts:
+
+CREATE FUNCTION transfer_funds() RETURNS void AS $$
+if [catch {
+spi_exec "UPDATE accounts SET balance = balance - 100 WHERE account_name = 'joe'"
+spi_exec "UPDATE accounts SET balance = balance + 100 WHERE account_name = 'mary'"
+} errormsg] {
+set result [format "error transferring funds: %s" $errormsg ]
+} else {
+set result "funds transferred correctly"
+}
+set plan [spi_prepare "INSERT INTO operations (result) VALUES ($1)"]
+spi_execp -count 1 $plan, [list $result)
+$$ LANGUAGE pltclu;
+
+If the second UPDATE statement results in an
+exception being raised, this function will report the error, but
+the result of the first UPDATE will
+nevertheless be committed.  In other words, the funds will be
+withdrawn from Joe's account, but will not be transferred to
+Mary's account.
+   
+   
+To avoid such issues, you can wrap your
+spi_exec calls in an explicit
+subtransaction.  The PL/Tcl provides a
+commmand subtransaction to manage explicit
+	subtransactions.
+ Using explicit subtransactions
+we can rewrite our function as:
+
+CREATE FUNCTION transfer_funds2() RETURNS void AS $$
+if [catch {
+	subtransaction {
+spi_exec "UPDATE accounts SET balance = balance - 100 WHERE account_name = 'joe'"
+spi_exec "UPDATE accounts SET balance = balance + 100 WHERE account_name = 'mary'"
+	}
+	} errormsg] {
+		set result [format "error transferring funds: %s" $errormsg]
+	} else {
+   set result "funds transferred correctly"
+	}
+set plan  [spi_prepare "INSERT INTO operations (result) VALUES ($1)"]
+spi_execp $plan, [list $result]
+$$ LANGUAGE pltclu;
+
+Note that the use of catch is still
+required.  Otherwise the exception would propagate to the top of
+the  stack and would cause the whole function to abort with
+a PostgreSQL error, so that the
+operations table would not have any row
+inserted into it.  The subtransaction command does not
+trap errors, it only assures that all database operations executed
+inside its scope will be atomically committed or rolled back.  A
+rollback of the subtransaction block occurs on any kind of
+exception exit, not only ones caused by errors originating from
+database access.  A regular Tcl exception raised inside an
+explicit subtransaction block would also cause the subtransaction
+to be rolled back.
+   
+  
 

 PL/Tcl Configuration
diff --git a/src/pl/tcl/Makefile b/src/pl/tcl/Makefile
index 1096c4f..b6b6b19 100644
--- a/src/pl/tcl/Makefile
+++ b/src/pl/tcl/Makefile
@@ -28,7 +28,7 @@ DATA = pltcl.control pltcl--1.0.sql pltcl--unpackaged--1.0.sql \
pltclu.control pltclu--1.0.sql pltclu--unpackaged--1.0.sql
 
 REGRESS_OPTS = --dbname=$(PL_TESTDB) --load-extension=pltcl
-REGRESS = pltcl_setup pltcl_queries pltcl_start_proc pltcl_unicode
+REGRESS = pltcl_setup pltcl_queries pltcl_start_proc pltcl_unicode pltcl_subxact
 
 # Tcl on win32 ships with import libraries only for Microsoft Visual C++,
 # which are not compatible with mingw gcc. Therefore we need to build a
diff --git a/src/pl/tcl/expected/pltcl_subxact.out b/src/pl/tcl/expected/pltcl_subxact.out
new file mode 100644
index 000..5d984f1
--- /dev/null
+++ b/src/pl/tcl/expected/pltcl_subxact.out
@@ -0,0 +1,171 @@
+--
+-- Test explicit subtransactions
+--
+CREATE TABLE subtransaction_tbl (
+i integer
+);
+-- test subtransaction successfully commited
+CREATE FUNCTION subtransaction_ctx_success() RETURNS text
+AS $$
+	spi_exec "INSERT INTO subtransaction_tbl VALUES(1)"
+subtransaction {
+		spi_exec "INSERT INTO sub

Re: [HACKERS] Explicit subtransactions for PL/Tcl

[Victor Wagner]

On Wed, 8 Mar 2017 16:49:33 +0100
Pavel Stehule <pavel.steh...@gmail.com> wrote:

> 
> I did a review of this patch
> 
I'm attaching new version of patch with the issues pointed by you fixed.

> 
> 4. A documentation is good - although I am not sure if it is well
> structured - is  level necessary? Probably there will not be
> any other similar command.

Removed . Although it is questionable - now there is no
subsection with name of the command in the title. But surrounding
section describes only one command,.



> 5. There are a basic regress tests, and all tests passed, but I miss a
> path, where subtransaction is commited - now rollback is every time

Added test with successful subtransaction commit. Just to be sure it is
really commited.

 
> 6. The code has some issues with white chars

Fixed where I've found it. Now, hopefully my code uses same identation
rules as other pltcl.c


> 7. I don't understand why TopMemoryContext is used there? Maybe some
> already used context should be there.
> 
> +<->BeginInternalSubTransaction(NULL);
> +<->MemoryContextSwitchTo(TopTransactionContext);
> +<->

It turns out that was really an error in the first version of patch.
This context manipulation was copied from PL/Python context manager by
mistake.

PL/Python changes to TopTransactionContext in order to manage
stack of subtransaction data (which we don't need in Tcl, because we
can use C language stack and store neccessary data in function
automatic variables. Really python doesn't need this stack to, because
Python context manager is a python language object and can keep state
inside it).

Although we still need to keep MemoryContext and ResourceOwner and
restore them upon  transaction finish.

With best regards, Victor

-- 
Victor Wagner <vi...@wagner.pp.ru>diff --git a/doc/src/sgml/pltcl.sgml b/doc/src/sgml/pltcl.sgml
index ad216dd..87bc4ad 100644
--- a/doc/src/sgml/pltcl.sgml
+++ b/doc/src/sgml/pltcl.sgml
@@ -901,6 +901,79 @@ if {[catch { spi_exec $sql_command }]} {
  is a global variable.)
 

+   
+   Explicit Subtransactions
+  
+   Recovering from errors caused by database access as described in
+can lead to an undesirable
+   situation where some operations succeed before one of them fails,
+   and after recovering from that error the data is left in an
+   inconsistent state.  PL/Tcl offers a solution to this problem in
+   the form of explicit subtransactions.
+  
+   
+Consider a function that implements a transfer between two
+accounts:
+
+CREATE FUNCTION transfer_funds() RETURNS void AS $$
+if [catch {
+spi_exec "UPDATE accounts SET balance = balance - 100 WHERE account_name = 'joe'"
+spi_exec "UPDATE accounts SET balance = balance + 100 WHERE account_name = 'mary'"
+} errormsg] {
+set result [format "error transferring funds: %s" $errormsg ]
+} else {
+set result "funds transferred correctly"
+}
+set plan [spi_prepare "INSERT INTO operations (result) VALUES ($1)"]
+spi_execp -count 1 $plan, [list $result)
+$$ LANGUAGE pltclu;
+
+If the second UPDATE statement results in an
+exception being raised, this function will report the error, but
+the result of the first UPDATE will
+nevertheless be committed.  In other words, the funds will be
+withdrawn from Joe's account, but will not be transferred to
+Mary's account.
+   
+   
+To avoid such issues, you can wrap your
+spi_exec calls in an explicit
+subtransaction.  The PL/Tcl provides a
+commmand subtransaction to manage explicit
+	subtransactions.
+ Using explicit subtransactions
+we can rewrite our function as:
+
+CREATE FUNCTION transfer_funds2() RETURNS void AS $$
+if [catch {
+	subtransaction {
+spi_exec "UPDATE accounts SET balance = balance - 100 WHERE account_name = 'joe'"
+spi_exec "UPDATE accounts SET balance = balance + 100 WHERE account_name = 'mary'"
+	}
+	} errormsg] {
+		set result [format "error transferring funds: %s" $errormsg]
+	} else {
+   set result "funds transferred correctly"
+	}
+set plan  [spi_prepare "INSERT INTO operations (result) VALUES ($1)"]
+spi_execp $plan, [list $result]
+$$ LANGUAGE pltclu;
+
+Note that the use of catch is still
+required.  Otherwise the exception would propagate to the top of
+the  stack and would cause the whole function to abort with
+a PostgreSQL error, so that the
+operations table would not have any row
+inserted into it.  The subtransaction command does not
+trap errors, it only assures that all database operations executed
+inside its scope will be atomically committed or rolled back.  A
+rollback of the subtransaction block occurs on any kind of
+exception exit, not only ones caused by errors originating from
+dat

Re: [HACKERS] Explicit subtransactions for PL/Tcl

[Victor Wagner]

On Wed, 8 Mar 2017 16:49:33 +0100
Pavel Stehule <pavel.steh...@gmail.com> wrote:


> 
> I did a review of this patch

First, thank you for you effort. I already begin to think that nobody
is really interesting in PL/Tcl stuff. 

> 1. This functionality has sense and nobody was against this feature.
> 
> 2. This patch does what is proposed - it introduce new TCL function
> that wraps PostgreSQL subtransaction
> 
> 3. This patch is really simple due massive using subtransactions
> already - every SPI called from TCL is wrapped to subtransaction.
> 
> 4. A documentation is good - although I am not sure if it is well
> structured - is  level necessary? Probably there will not be
> any other similar command.

You are right. At least sect2 can be added later whenever this second
command will appear.

> 5. There are a basic regress tests, and all tests passed, but I miss a
> path, where subtransaction is commited - now rollback is every time

Really, I haven't found such tests in PL/Python suite, so I haven't
translated it to Tcl. 

It might be good idea to add such test.

> 6. The code has some issues with white chars
> 
> 7. I don't understand why TopMemoryContext is used there? Maybe some
> already used context should be there.
> 
> +<->BeginInternalSubTransaction(NULL);
> +<->MemoryContextSwitchTo(TopTransactionContext);
> +<->

I've copied this code from PL/Python subtransaction object and 
it has following comment:
   /* Be sure that cells of explicit_subtransactions list are long-lived */
 
But in Tcl case wi don't have to maintain our own stack in the form of
list. We use C-language stack and keep our data in the local variables.
So, probably changing of memory contexts is not necessary at all.

But it require a bit more investigation and testing.

With best regards, Victor


-- 
   Victor Wagner <vi...@wagner.pp.ru>


-- 
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] Explicit subtransactions for PL/Tcl

[Victor Wagner]

On Sun, 8 Jan 2017 20:57:50 +0300
Victor Wagner <vi...@wagner.pp.ru> wrote:

> Collegues!
> 
> Recently I've found out that PL/Python have very nice feature -
> explicit subtransaction object, which allows to execute block of code
> in the context of subtransaction.
> 
[skip]

> 
> I'm attaching the patch which implements subtransaction command for

Sorry, unfortunately attached empty file instead of patch
-- 
       Victor Wagner <vi...@wagner.pp.ru>
diff --git a/doc/src/sgml/pltcl.sgml b/doc/src/sgml/pltcl.sgml
index 8afaf4a..7a532b7 100644
--- a/doc/src/sgml/pltcl.sgml
+++ b/doc/src/sgml/pltcl.sgml
@@ -901,6 +901,85 @@ if {[catch { spi_exec $sql_command }]} {
  is a global variable.)
 

+   
+   Explicit Subtransactions
+  
+   Recovering from errors caused by database access as described in
+can lead to an undesirable
+   situation where some operations succeed before one of them fails,
+   and after recovering from that error the data is left in an
+   inconsistent state.  PL/Tcl offers a solution to this problem in
+   the form of explicit subtransactions.
+  
+
+  
+   Subtransaction command
+
+   
+Consider a function that implements a transfer between two
+accounts:
+
+CREATE FUNCTION transfer_funds() RETURNS void AS $$
+if [catch {
+spi_exec "UPDATE accounts SET balance = balance - 100 WHERE account_name = 'joe'"
+spi_exec "UPDATE accounts SET balance = balance + 100 WHERE account_name = 'mary'"
+} errormsg] {
+set result [format "error transferring funds: %s" $errormsg ]
+} else {
+set result "funds transferred correctly"
+}
+set plan [spi_prepare "INSERT INTO operations (result) VALUES ($1)"]
+spi_execp -count 1 $plan, [list $result)
+$$ LANGUAGE pltclu;
+
+If the second UPDATE statement results in an
+exception being raised, this function will report the error, but
+the result of the first UPDATE will
+nevertheless be committed.  In other words, the funds will be
+withdrawn from Joe's account, but will not be transferred to
+Mary's account.
+   
+
+   
+To avoid such issues, you can wrap your
+spi_exec calls in an explicit
+subtransaction.  The PL/Tcl provides a
+commmand subtransaction to manage explicit
+	subtransactions.
+ Using explicit subtransactions
+we can rewrite our function as:
+
+CREATE FUNCTION transfer_funds2() RETURNS void AS $$
+if [catch {
+	subtransaction {
+spi_exec "UPDATE accounts SET balance = balance - 100 WHERE account_name = 'joe'"
+spi_exec "UPDATE accounts SET balance = balance + 100 WHERE account_name = 'mary'"
+	}
+	} errormsg] {
+		set result [format "error transferring funds: %s" $errormsg]
+	} else {
+   set result "funds transferred correctly"
+	}
+set plan  [spi_prepare "INSERT INTO operations (result) VALUES ($1)"]
+spi_execp $plan, [list $result]
+$$ LANGUAGE pltclu;
+
+Note that the use of catch is still
+required.  Otherwise the exception would propagate to the top of
+the  stack and would cause the whole function to abort with
+a PostgreSQL error, so that the
+operations table would not have any row
+inserted into it.  The subtransaction command does not
+trap errors, it only assures that all database operations executed
+inside its scope will be atomically committed or rolled back.  A
+rollback of the subtransaction block occurs on any kind of
+exception exit, not only ones caused by errors originating from
+database access.  A regular Tcl exception raised inside an
+explicit subtransaction block would also cause the subtransaction
+to be rolled back.
+   
+  
+  
 

Modules and the unknown Command
diff --git a/src/pl/tcl/Makefile b/src/pl/tcl/Makefile
index 25082ec..614385d 100644
--- a/src/pl/tcl/Makefile
+++ b/src/pl/tcl/Makefile
@@ -28,7 +28,7 @@ DATA = pltcl.control pltcl--1.0.sql pltcl--unpackaged--1.0.sql \
pltclu.control pltclu--1.0.sql pltclu--unpackaged--1.0.sql
 
 REGRESS_OPTS = --dbname=$(PL_TESTDB) --load-extension=pltcl
-REGRESS = pltcl_setup pltcl_queries pltcl_unicode
+REGRESS = pltcl_setup pltcl_queries pltcl_unicode pltcl_subxact
 
 # Tcl on win32 ships with import libraries only for Microsoft Visual C++,
 # which are not compatible with mingw gcc. Therefore we need to build a
diff --git a/src/pl/tcl/expected/pltcl_subxact.out b/src/pl/tcl/expected/pltcl_subxact.out
new file mode 100644
index 000..17b9d90
--- /dev/null
+++ b/src/pl/tcl/expected/pltcl_subxact.out
@@ -0,0 +1,145 @@
+--
+-- Test explicit subtransactions
+--
+-- Test table to see if transactions get properly rolled back
+CREATE TABLE subtransaction_tbl (
+i integer
+);
+CREATE FUNCTION subtransaction_ctx_test(what_error text = NULL) RETURNS text
+AS $$
+subtransaction {
+spi_exec "INSERT IN

[HACKERS] Explicit subtransactions for PL/Tcl

[Victor Wagner]

Collegues!

Recently I've found out that PL/Python have very nice feature - explicit
subtransaction object, which allows to execute block of code in the
context of subtransaction.

I've quite surprised that other PL languages, shipped with PostgreSQL do
not have such useful construction. 

If it might require considerable trickery to add such functionality into
PL/Perl, Tcl allows to add new control stuctures very easily.

I'm attaching the patch which implements subtransaction command for
PL/Tcl which does almost same as PL/Python plpy.subtransction context
manager object does: executes a block of Tcl code in the context of
subtransaction, rolls subtransaction back if error occures and commits
it otherwise.

It looks like

subtransaction {
 ...some Tcl code...
}

Typically one would use it inside Tcl catch statement:

if [catch {
subtransaction {
spi_exec "insert into..."
...
}
} errormsg] {
   # Handle an error
}

See documentation and tests included in the patch for more complete
examples.

Just like corresponding Python construction, this command doesn't
replace language  builtin exception handling, just adds subtransaction 
support to it.

Patch includes sufficiently less tests than python subtransaction tests,
because Tcl implementation is way simpler than python one, and doesn't
have syntactic variatons which depend on language version.

Also entering and exiting subtransactions are in the same piece of code
rather than in separate __enter__ and __exit__ methods as in Python, so
there is no possibility to call exit without enter or vice versa.

-- 
       Victor Wagner <vi...@wagner.pp.ru>

-- 
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] Random PGDLLIMPORTing

[Victor Wagner]

On Thu, 24 Nov 2016 15:01:33 +0100
Magnus Hagander  wrote:

> On Thu, Nov 24, 2016 at 2:58 PM, Craig Ringer 
> wrote:
> 
> > Hi all
> >
> > Noticed this while reading something unrelated
> >
> > extern PGDLLIMPORT pid_t PostmasterPid;
> > extern bool IsPostmasterEnvironment;
> > extern PGDLLIMPORT bool IsUnderPostmaster;
> > extern bool IsBackgroundWorker;
> > extern PGDLLIMPORT bool IsBinaryUpgrade;
> >
> > I don't see any sane reason for some of those to be PGDLLIMPORT but
> > not others. In particular it's pretty silly for IsBackgroundWorker
> > not to be PGDLLIMPORT.
> >
> > Too trivial to be worth making an actual patch, it'd be more work to
> > apply it than edit directly.
> >  
> 
> My guess is that PGDLLIMPORT has been added explicitly when somebody
> needed it for something, without any actual thought. I can't say I
> see any reason not to export the other ones as well -- more that
> maybe there are even more that are needed?
> 

It worth checking actual variable definitions, not just declarations.
I've found recently, that at least in MSVC build system, only
initialized variables are included into postgres.def file, and so are
actually exported from the backend binary.

But in this case both IsPostmasterEnvironment and IsBackgroundWorker
are initialized. So probably they are not marked as importable only for
historic reason.





-- 
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] Password identifiers, protocol aging and SCRAM protocol

[Victor Wagner]

On Wed, 9 Nov 2016 15:23:11 +0900
Michael Paquier  wrote:


> 
> (This is about patch 0007, not 0001)
> Thanks, you are right. That's not good as-is. So this basically means
> that the characters here should be from 32 to 127 included.

Really, most important is to exclude comma from the list of allowed
characters. And this prevents us from using a range.

I'd do something like:

char prinables="0123456789ABCDE...xyz!@#*&+";
unsigned int r;

for (i=0;i generate_nonce needs just to be made smarter in the way it selects the
> character bytes.



-- 
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] Password identifiers, protocol aging and SCRAM protocol

[Victor Wagner]

On Tue, 18 Oct 2016 16:35:27 +0900
Michael Paquier  wrote:

 Hi
> Attached is a rebased patch set for SCRAM, with the following things:
> - 0001, moving all the SHA2 functions to src/common/ and introducing a
> PG-like interface. No actual changes here.

It seems, that client nonce generation in this patch is not
RFC-compliant.

RFC 5802 states that SCRAM nonce should be

a sequence of random printable ASCII
  characters excluding ','

while this patch uses sequence of random bytes from pg_strong_random
function with zero byte appended.

It could cause following problems

1. If zero byte happens inside random sequence, nonce would be shorter
than expected, or even empty.

2. If one of bytes happens to be ASCII Code of comma, than server
to the client-first message, which includes copy of client nonce,
appended by server nonce,
as one of unquoted comman-separated field, would be parsed incorrectly.


Regards, Victor
-- 





-- 
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] Patch: Implement failover on libpq connect level.

[Victor Wagner]

On Wed, 19 Oct 2016 12:04:27 -0400
Robert Haas <robertmh...@gmail.com> wrote:

> On Thu, Oct 13, 2016 at 5:53 AM, Victor Wagner <vi...@wagner.pp.ru>
> wrote:
> > On Thu, 13 Oct 2016 12:30:59 +0530
> > Mithun Cy <mithun...@enterprisedb.com> wrote:  
> >> On Fri, Sep 30, 2016 at 2:14 PM, Victor Wagner <vi...@wagner.pp.ru>
> >> wrote:
> >> Okay but for me consistency is also important. Since we agree to
> >> disagree on some of the comments and others have not expressed any
> >> problem I am moving it to committer.  
> >
> > Thank you for your efforts improving my patch  
> 
> I haven't deeply reviewed this patch, but on a quick read-through it
> certainly seems to need a lot of cleanup work.
> 

I've did some cleanup, i.e. running pgindent, spell-checking comments
and indenting sgml and attaching cleaned up version here.

I've also re-added test file t/001-multihost.pl which get lost from
previous post.

Thanks for your suggestions.



-- 
   Victor Wagner <vi...@wagner.pp.ru>
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 4e34f00..b7c62d2 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -792,7 +792,7 @@ host=localhost port=5432 dbname=mydb connect_timeout=10

The general form for a connection URI is:
 
-postgresql://[user[:password]@][netloc][:port][/dbname][?param1=value1...]
+postgresql://[user[:password]@][netloc][:port][,netloc[:port]...][/dbname][?param1=value1...]
 

 
@@ -809,6 +809,7 @@ postgresql://localhost/mydb
 postgresql://user@localhost
 postgresql://user:secret@localhost
 postgresql://other@localhost/otherdb?connect_timeout=10application_name=myapp
+postgresql://node1,node2:5433,node3:4432,node4/mydb?hostorder=randomtarget_server_type=any
 
 Components of the hierarchical part of the URI can also
 be given as parameters.  For example:
@@ -831,7 +832,9 @@ postgresql:///mydb?host=localhostport=5433

 For improved compatibility with JDBC connection URIs,
 instances of parameter ssl=true are translated into
-sslmode=require.
+sslmode=require and
+loadBalanceHosts=true  into
+hostorder=random.

 

@@ -841,6 +844,10 @@ postgresql:///mydb?host=localhostport=5433
 postgresql://[2001:db8::1234]/database
 

+   
+   There can be several host specifications, optionally accompanied
+   with port, separated by comma.
+   
 

 The host component is interpreted as described for the parameter PostgreSQL was built). On machines without
 Unix-domain sockets, the default is to connect to localhost.

+   
+There can be more than one host parameter in
+the connection string. In this case these hosts would be considered
+alternate entries into same database and if connection to first one
+fails, the second would be attemped, and so on. This can be used
+for high availability clusters or for load balancing. See the
+ parameter. This feature
+works for TCP/IP host names only.
+   
+   
+The network host name can be accompanied by a port number, separated by
+colon. This port number is used only when connected to
+this host. If there is no port number, the port specified in the
+ parameter would be used instead.
+   
   
  
 
@@ -933,7 +955,6 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
 is used to identify the connection in ~/.pgpass (see
 ).

-

 Without either a host name or host address,
 libpq will connect using a
@@ -943,6 +964,43 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname

   
 
+  
+   hostorder
+   
+   
+Specifies how to choose the host from the list of alternate hosts,
+specified in the  parameter.
+   
+   
+If the value of this argument is sequential (the
+default) connections to the hosts will be attempted in the order in
+which they are specified.
+   
+   
+If the value is random, the host to connect to
+will be randomly picked from the list. It allows load balacing between
+several cluster nodes. However, PostgreSQL doesn't currently support
+multimaster clusters. So, without the use of third-party products,
+only read-only connections can take advantage from
+load-balancing. See 
+   
+   
+  
+
+  
+   target_server_type
+
+
+ If this parameter is master, upon
+ successful connection the host is checked to determine whether
+ it is in a recovery state.  If it is, it then tries next host
+ in the connection string. If this parameter is
+ any, connection to standby nodes are
+ considered successful. 
+

Re: [HACKERS] Patch: Implement failover on libpq connect level.

[Victor Wagner]

On Wed, 19 Oct 2016 20:15:38 -0400
Robert Haas  wrote:

> 
> Some more comments:
> 
> - I am pretty doubtful that the changes to connectOptions2() work as
> intended.  I think that's only going to be called before actualhost
> and actualport could possibly get set.  I don't think we keep
> reinvoking this function for every new host we try.

I would investigate this. Probably test cases for .pgpass file should
be added.

 
> - It's pretty clear that this isn't going to work if the host list
> includes a mix of hostnames and UNIX socket addresses.  The code that
> handles the UNIX socket address case is totally separate from the code
> that handles the multiple-hostname case.

Muitihost feature was never intended to work with unix-domain socket. 
May be it should be clarified in the docs, or even improved error
message emitted when unix socket occurs in the multiple host list.
 
> - This creates a sort of definitional problem for functions like
> PQhost().  The documentation says of this and other related functions:
> "These values are fixed for the life of the PGconn object."  But with
> this patch, that would no longer be true.  So at least the
> documentation needs an update.  Actually, though, I think the problem

I've missed this phrase, documentation should be updated.

> is more fundamental than that.  If the user asks for PQhost(conn), do
> they want the list of all hosts, or the one to which we're currently
> connected?  It might be either, depending on what they intend to do
> with the information.  If they mean to construct a new connection
> string, they might well want them all; but something like psql's
> \conninfo only shows the current one.  There's also the question of
> what "the current one" means if we're not connected to any server at
> all.  I'm not sure exactly how to solve these problems, but they need
> some thought.

I've thought that no one would need to reconstruct connect string from
conninfo object.  May be I've missed some use cases. 

Name PQhost suggest that it'll return just one host. So, I've decided
to interpret it as  'currently choosen host'. 

If we really need function which lists all the potentially connected
hosts, it should be new function. 

I hope this would introduce no backward incompatibility, because no
existing setup uses multihost connect strings.

Thanks for your critique.

Victor.


-- 
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] Patch: Implement failover on libpq connect level.

[Victor Wagner]

On Mon, 24 Oct 2016 11:36:31 +0100
Thom Brown  wrote:


> > - It's pretty clear that this isn't going to work if the host list
> > includes a mix of hostnames and UNIX socket addresses.  The code
> > that handles the UNIX socket address case is totally separate from
> > the code that handles the multiple-hostname case.  
> 
> So should it be the case that it disallows UNIX socket addresses
> entirely?  I can't imagine a list of UNIX socket addresses being that
> useful.

Really, patch was implemented as TCP-only from the beginning.

I know only one case, where list of UNIX sockets is useful - test suite.
I have to include modifications to PostgresNode.pm in the patch to
allow testing with TCP sockets.

Also, I can imagine that people would want include one unix socket into
list of TCP ones, i.e. one of replicas running on the same host as
application server. But patch doesn't support it. May be it should be
clarified in the documentation, that this is not supported.



-- 
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] Patch: Implement failover on libpq connect level.

[Victor Wagner]

On Thu, 13 Oct 2016 12:30:59 +0530
Mithun Cy <mithun...@enterprisedb.com> wrote:

> On Fri, Sep 30, 2016 at 2:14 PM, Victor Wagner <vi...@wagner.pp.ru>
> wrote:

> Okay but for me consistency is also important. Since we agree to
> disagree on some of the comments and others have not expressed any
> problem I am moving it to committer.


Thank you for your efforts improving my patch



-- 
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] [PATCH] Generic type subscription

[Victor Wagner]

On Sat, 1 Oct 2016 16:52:34 +0700
Dmitry Dolgov <9erthali...@gmail.com> wrote:

> > I've tried to compile this patch with current state of master
> > (commit 51c3e9fade76c12)  and found out that, when configured with  
> --enable-cassert,
> > it doesn't pass make check.  
> 
> Thanks for the feedback. Yes, unexpectedly for me, `ExecEvalExpr` can
> return expanded `jbvArray` and `jbvObject` instead `jbvBinary` in
> both cases. It's interesting, that this doesn't break anything, but
> obviously violates the `pushJsonbValueScalar` semantics. I don't
> think `ExecEvalExpr` should be changed for jsonb, we can handle this
> situation in `pushJsonbValue` instead. I've
> attached a new version of patch with this modification.

Thanks for you quick responce. I've not yet thoroughly investigated new
patch, but already noticed some minor promlems:

Git complains about whitespace in this version of patch:

$ git apply ../generic_type_subscription_v2.patch
../generic_type_subscription_v2.patch:218: tab in indent.
int first;
../generic_type_subscription_v2.patch:219: tab in indent.
int second;
../generic_type_subscription_v2.patch:225: tab in indent.
SubscriptionExecData*sbsdata = (SubscriptionExecData *) 
PG_GETARG_POINTER(1);
../generic_type_subscription_v2.patch:226: tab in indent.
Custom  *result = (Custom *) 
sbsdata->containerSource;
../generic_type_subscription_v2.patch:234: tab in indent.
SubscriptionRef*sbsref = (SubscriptionRef *) PG_GETARG_POINTER(0);
warning: squelched 29 whitespace errors
warning: 34 lines add whitespace errors.

It doesn't prevent me from further testing of patch, but worth noticing.

-- 

Victor



-- 
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] Patch: Implement failover on libpq connect level.

[Victor Wagner]

On Thu, 29 Sep 2016 23:45:52 +0530
Mithun Cy  wrote:

> This patch do not apply on latest code. it fails as follows

It's strange. I have no problems applying it to commit
fd321a1dfd64d30

Ok, some trailing whitespace and mixing of tabs and spaces
which git apply doesn't like really present in the patch.

I'm attached hear version with these issues resolved.

 
> I am slightly confused. As per this target_server_type=master means
> user is expecting failover. What if user pass target_server_type=any
> and request sequential connection isn't this also a case for
> failover?.  I think by default it should be "any" for any number of
> hosts. And parameter "sequential and random will decide whether we
> want just failover or with load-balance.

I don't agree with this. In the first versions of the patch it refuses 
connect to readonly server even if it is only one, because I think that
read-write connection is what user typically expect.

When user tries to connect to cluster (specifying many hosts in the
connect string), it can be by default assumed that he wants master. 

But backward compatibility is more
important thing, so I now assume, that user tries to connect just one
node, and this node is read only, user knows what he is doing.

 

diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 4e34f00..f0df877 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -792,7 +792,7 @@ host=localhost port=5432 dbname=mydb connect_timeout=10

The general form for a connection URI is:
 
-postgresql://[user[:password]@][netloc][:port][/dbname][?param1=value1...]
+postgresql://[user[:password]@][netloc][:port][,netloc[:port]...][/dbname][?param1=value1...]
 


@@ -809,6 +809,7 @@ postgresql://localhost/mydb
 postgresql://user@localhost
 postgresql://user:secret@localhost
 postgresql://other@localhost/otherdb?connect_timeout=10application_name=myapp
+postgresql://node1,node2:5433,node3:4432,node4/mydb?hostorder=randomtarget_server_type=any
 
 Components of the hierarchical part of the URI can also
 be given as parameters.  For example:
@@ -831,7 +832,9 @@ postgresql:///mydb?host=localhostport=5433

 For improved compatibility with JDBC connection URIs,
 instances of parameter ssl=true are translated into
-sslmode=require.
+sslmode=require and
+loadBalanceHosts=true  into
+hostorder=random.



@@ -841,6 +844,10 @@ postgresql:///mydb?host=localhostport=5433
 postgresql://[2001:db8::1234]/database
 

+   
+   There can be serveral host specifications, optionally accompanied
+   with port, separated by comma.
+   


 The host component is interpreted as described for the parameter PostgreSQL was built). On machines without
 Unix-domain sockets, the default is to connect to localhost.

+   
+   There can be more than one host parameter in
+   the connect string. In this case these hosts would be considered
+   alternate entries into same database and if connect to first one
+   fails, library would try to connect second etc. This can be used
+   for high availability cluster or for load balancing. See
+parameter.
+   
+   
+   Network host name can be accompanied with port number, separated by
+   colon. If so, this port number is used only when connected to
+   this host. If there is no port number, port specified in the
+parameter would be used.
+   
   
  

@@ -942,8 +963,44 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname


   
-
+
+  
+  hostorder
+  
+  
+  Specifies how to choose host from list of alternate hosts,
+  specified in the  parameter.
+  
+  
+  If value of this argument is sequential (the
+  default) library connects to the hosts in order they specified,
+  and tries to connect second one only if connection to the first
+  fails.
+  
+  
+  If value is random host to connect is randomly
+  picked from the list. It allows to balance load between several
+  cluster nodes. However, currently PostgreSQL doesn't support
+  multimaster clusters. So, without use of third-party products,
+  only read-only connections can take advantage from the
+  load-balancing. See 
+  
+  
+  
+  
+  target_server_type
+  
+  
+  If this parameter is master upon successful connection
+  library checks if host is in recovery state, and if it is so,
+  tries next host in the connect string. If this parameter is
+  any, connection to standby nodes are considered
+  successful.
+  
+  
+  
   
+
port


@@ -985,7 +1042,6 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
   
   
  
-
  
   connect_timeout
   
@@ -996,7 +1052,27 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
   
   
  
-
+ 
+ 

Re: [HACKERS] [PATCH] Generic type subscription

[Victor Wagner]

On Fri, 9 Sep 2016 18:29:23 +0700
Dmitry Dolgov <9erthali...@gmail.com> wrote:

> Hi,
> 
> Regarding to the previous conversations [1], [2], here is a patch
> (with some improvements from Nikita Glukhov) for the generic type
> subscription. It allows
> to define type-specific subscription logic for any data type and has
> implementations for the array and jsonb types. There are following
> changes in this
> patch:

I've tried to compile this patch with current state of master (commit
51c3e9fade76c12)  and found out that, when configured with
--enable-cassert, it doesn't pass make check.

LOG:  server process (PID 4643) was terminated by signal 6: Aborted
DETAIL:  Failed process was running: 
update test_jsonb_subscript set test_json['a'] = '{"b":
1}'::jsonb;



--
 Victor 


-- 
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] Patch: Implement failover on libpq connect level.

[Victor Wagner]

On Sun, 25 Sep 2016 17:31:53 +0530
Mithun Cy  wrote:

> I have some more comments on libpq-failover-8.patch
> 
> + /* FIXME need to check that port is numeric */
> 
> Is this still applicable?.
> 

Unfortunately, it was. I've fixed this problem in 9-th version of patch
(attached)
> 
> I think we need comments to know why we change default value based on
> number of elements in connection string. why default in not “any"
> when node count > 1.

Fixed.

> 
> + /* loop over all the host specs in the node variable */
> 
> + for (node = nodes; node->host != NULL || node->port != NULL; node++)
> 
>   {
> 
> I think instead of loop if we can move these code into a separate
> function say pg_add_to_addresslist(host, port, ) this helps in
> removing temp variables like "struct node info” and several heap
> calls around it.

For some reason DNS resolving was concentrated in one place before my
changes. So, I've tried to not change this decision.

 
> 3)
> 
> +static char *
> 
> +get_port_from_addrinfo(struct addrinfo * ai)
> 
> 
> Need some comments for this function.

Done.
 
> We use strdup in many places no where we handle memory allocation
> failure.

Added some more memory allocation error checks.
> 
> Comments not in sink with code.

Fixed.

diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 4e34f00..f0df877 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -792,7 +792,7 @@ host=localhost port=5432 dbname=mydb connect_timeout=10

The general form for a connection URI is:
 
-postgresql://[user[:password]@][netloc][:port][/dbname][?param1=value1...]
+postgresql://[user[:password]@][netloc][:port][,netloc[:port]...][/dbname][?param1=value1...]
 

 
@@ -809,6 +809,7 @@ postgresql://localhost/mydb
 postgresql://user@localhost
 postgresql://user:secret@localhost
 postgresql://other@localhost/otherdb?connect_timeout=10application_name=myapp
+postgresql://node1,node2:5433,node3:4432,node4/mydb?hostorder=randomtarget_server_type=any
 
 Components of the hierarchical part of the URI can also
 be given as parameters.  For example:
@@ -831,7 +832,9 @@ postgresql:///mydb?host=localhostport=5433

 For improved compatibility with JDBC connection URIs,
 instances of parameter ssl=true are translated into
-sslmode=require.
+sslmode=require and
+loadBalanceHosts=true  into
+hostorder=random.

 

@@ -841,6 +844,10 @@ postgresql:///mydb?host=localhostport=5433
 postgresql://[2001:db8::1234]/database
 

+   
+   There can be serveral host specifications, optionally accompanied
+   with port, separated by comma.
+   
 

 The host component is interpreted as described for the parameter PostgreSQL was built). On machines without
 Unix-domain sockets, the default is to connect to localhost.

+   
+   There can be more than one host parameter in
+   the connect string. In this case these hosts would be considered
+   alternate entries into same database and if connect to first one
+   fails, library would try to connect second etc. This can be used
+   for high availability cluster or for load balancing. See
+parameter.
+   
+   
+   Network host name can be accompanied with port number, separated by
+   colon. If so, this port number is used only when connected to
+   this host. If there is no port number, port specified in the
+parameter would be used.
+   
   
  
 
@@ -942,8 +963,44 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname


   
-
+
+  
+  hostorder
+  
+  
+  Specifies how to choose host from list of alternate hosts,
+  specified in the  parameter.
+  
+  
+  If value of this argument is sequential (the
+  default) library connects to the hosts in order they specified,
+  and tries to connect second one only if connection to the first
+  fails.
+  
+  
+  If value is random host to connect is randomly
+  picked from the list. It allows to balance load between several
+  cluster nodes. However, currently PostgreSQL doesn't support
+  multimaster clusters. So, without use of third-party products,
+  only read-only connections can take advantage from the
+  load-balancing. See 
+  
+  
+  
+  
+  target_server_type
+  
+  
+  If this parameter is master upon successful connection
+  library checks if host is in recovery state, and if it is so,
+  tries next host in the connect string. If this parameter is
+  any, connection to standby nodes are considered
+  successful.
+  
+  
+  
   
+
port


@@ -985,7 +1042,6 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
   
   
  
-
  
   connect_timeout
   
@@ -996,7 +1052,27 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
   
   
  
-
+ 

[HACKERS] Inheriting PostgresNode object

[Victor Wagner]

Hi hackers!

I've encountered need to extend functionality of PostgresNode class
from the TAP test framework. What I want can easily be done via perl
object inheritation. 

But documentation in the PostgresNode.pm recommends to use get_new_node
function rather than call PostgresNode constructor directly.

I see following ways to solve this problem:

1. Ignore this recommendation and write new class which calls
SUPER->new() from the constructor and call its constructor directly

2. Get the PostgresNode object from get_new_node function and bless it
into new class

3. Use delegation instead of inheritance. This would require some black
magic with AutoLoader module to avoid writing wrapper for each
PostgresNode method.

Which approach would people recommend to take?

-- 
   Victor Wagner <vi...@wagner.pp.ru>


-- 
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] Patch: Implement failover on libpq connect level.

[Victor Wagner]

On Fri, 9 Sep 2016 11:20:59 +0530
Mithun Cy  wrote:

> If concern is only about excluding address which was tried
> previously. Then I think we can try this way, randomly permute given
> address list (for example fisher-yates shuffle) before trying any of
> those address in it. After that you can treat the new list exactly
> like we do for sequential connections. I think this makes code less
> complex.

Random permutation is much more computationally complex than random
picking. 

Alexander suggests to use other random pick algorithm than I use.

I use algorithm which works with lists of unknown length and chooses
line of such list with equal probability. This algorithm requires one
random number for each element of the list, but we are using C library
rand function which uses quite cheap linear congruental pseudo random
numbers, so random number generation is infinitely cheap than network
connection attempt.

As this algorithm doesn't need beforehand knowledge of the list length,
it is easy to wrap this algorithm into loop, which modifies the list,
moving already tried elements out of scope of next picking.
(it is really quite alike fisher-yates algorithm).

Alexander suggests to store somewhere number of elements of the list,
than generate one random number and pick element with this number.

Unfortunately, it doesn't save much effort in the linked list.
At average we'll need to scan half of the list to find desired element.
So, it is O(N) anyway.

Now, the main point is that we have two different timeframes of
operation.

1. Attempt to connect. It is short period, and we can assume that state
of servers doesn't change. So if, server is down, we should ignore it
until the end of this attempt to connect.

2. Attempt to failover. If no good candidate was found, we might need
to retry connection until one of standbys would be promoted to master
(with targetServerType=master) or some host comes up. See documentation
of failover_timeout configuration parameter.

It means that we cannot just throw away those hosts from list which
didn't respond during this connect attempt.
 
So, using random permutation instead  of random pick wouln't help.
And keeping somewhere number of elements in the list wouldn't help
either unless we change linked list to completely different data
structure which allows random access.
-- 




-- 
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] Patch: Implement failover on libpq connect level.

[Victor Wagner]

On Tue, 6 Sep 2016 07:58:28 +0530
Mithun Cy  wrote:

> On Mon, Sep 5, 2016 at 4:33 PM, Aleksander Alekseev <
> a.aleks...@postgrespro.ru> wrote:
> >After a brief examination I've found following ways to improve the
> >patch.  
> Adding to above comments.
> 
I'm sending new version of patch.

I've replaced readonly option with target_server_type (with JDBC
compatibility alias targetServerType), 

use logic of setting defaults based on number of hosts in the connect
string instead of complicated condition in the state machine,

added checks for return values of memory allocation function.

Also, I've solved pg_usleep problem by linking pgsleep.c into libpq
along with some other libpgport objects which are already linked there.

Thus client applications don't need to link with libpgport and libpq
shared library is self-containted.
 diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 4e34f00..f0df877 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -792,7 +792,7 @@ host=localhost port=5432 dbname=mydb connect_timeout=10

The general form for a connection URI is:
 
-postgresql://[user[:password]@][netloc][:port][/dbname][?param1=value1...]
+postgresql://[user[:password]@][netloc][:port][,netloc[:port]...][/dbname][?param1=value1...]
 

 
@@ -809,6 +809,7 @@ postgresql://localhost/mydb
 postgresql://user@localhost
 postgresql://user:secret@localhost
 postgresql://other@localhost/otherdb?connect_timeout=10application_name=myapp
+postgresql://node1,node2:5433,node3:4432,node4/mydb?hostorder=randomtarget_server_type=any
 
 Components of the hierarchical part of the URI can also
 be given as parameters.  For example:
@@ -831,7 +832,9 @@ postgresql:///mydb?host=localhostport=5433

 For improved compatibility with JDBC connection URIs,
 instances of parameter ssl=true are translated into
-sslmode=require.
+sslmode=require and
+loadBalanceHosts=true  into
+hostorder=random.

 

@@ -841,6 +844,10 @@ postgresql:///mydb?host=localhostport=5433
 postgresql://[2001:db8::1234]/database
 

+   
+   There can be serveral host specifications, optionally accompanied
+   with port, separated by comma.
+   
 

 The host component is interpreted as described for the parameter PostgreSQL was built). On machines without
 Unix-domain sockets, the default is to connect to localhost.

+   
+   There can be more than one host parameter in
+   the connect string. In this case these hosts would be considered
+   alternate entries into same database and if connect to first one
+   fails, library would try to connect second etc. This can be used
+   for high availability cluster or for load balancing. See
+parameter.
+   
+   
+   Network host name can be accompanied with port number, separated by
+   colon. If so, this port number is used only when connected to
+   this host. If there is no port number, port specified in the
+parameter would be used.
+   
   
  
 
@@ -942,8 +963,44 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname


   
-
+
+  
+  hostorder
+  
+  
+  Specifies how to choose host from list of alternate hosts,
+  specified in the  parameter.
+  
+  
+  If value of this argument is sequential (the
+  default) library connects to the hosts in order they specified,
+  and tries to connect second one only if connection to the first
+  fails.
+  
+  
+  If value is random host to connect is randomly
+  picked from the list. It allows to balance load between several
+  cluster nodes. However, currently PostgreSQL doesn't support
+  multimaster clusters. So, without use of third-party products,
+  only read-only connections can take advantage from the
+  load-balancing. See 
+  
+  
+  
+  
+  target_server_type
+  
+  
+  If this parameter is master upon successful connection
+  library checks if host is in recovery state, and if it is so,
+  tries next host in the connect string. If this parameter is
+  any, connection to standby nodes are considered
+  successful.
+  
+  
+  
   
+
port


@@ -985,7 +1042,6 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
   
   
  
-
  
   connect_timeout
   
@@ -996,7 +1052,27 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
   
   
  
-
+ 
+ failover_timeout
+ 
+ 
+ Maximum time to cyclically retry all the hosts in connect string.
+ (as decimal integer number of seconds). If not specified, then
+ hosts are tried just once.
+ 
+ 
+ If we have replicating cluster, and master node fails, it might
+ take some time to promote one of standby nodes to the new master.
+ So clients which notice that connect to the 

Re: [HACKERS] Patch: Implement failover on libpq connect level.

[Victor Wagner]

On Mon, 5 Sep 2016 14:03:11 +0300
Aleksander Alekseev  wrote:

> Hello, Victor.
> 
> 
> 1) It looks like code is not properly formatted.
> 

Thanks for pointing to the documentation and formatting problems. I'll
fix them 
 
> >  static int 
> >  connectDBStart(PGconn *conn)
> >  {
> > +   struct nodeinfo
> > +   {
> > +   char   *host;
> > +   char   *port;
> > +   };  
> 
> Not sure if all compilers we support can parse this. I suggest to
> declare nodinfo structure outside of procedure, just to be safe.
> 

Block-local structs  are part of C language standard. I don't remember
when they appeared first (may be in K C), but at least since C89 AFAIR
they are allowed explicitely.

And using most local scope possible is always a good thing.

So, I'll leave this structure function local for now.


> 
> You should check return values of malloc, calloc and strdup
> procedures, or use safe pg_* equivalents.

Thanks, I'll fix this one.
 
> 6) 
> 
> > +   for (p = addrs; p->ai_next != NULL; p =
> >   p->ai_next);
> > +   p->ai_next = this_node_addrs;  
> 
> Looks a bit scary. I suggest to add an empty scope ( {} ) and a
> comment to make our intention clear here.

Ok, it really would make code more clear.
 
> 7) Code compiles with following warnings (CLang 3.8):
> 
> > 1 warning generated.
> > fe-connect.c:5239:39: warning: if statement has empty body
> > [-Wempty-body]
> >
> > errorMessage,
> > false, true));
> > 
> >   ^
> > fe-connect.c:5239:39: note: put the semicolon on a separate line to
> > silence this warning
> > 1 warning generated.  

Oops, it looks like logic error, which should be fixed. Thanks for
testing my patch by more picky compiler.

> 8) get_next_element procedure implementation is way too smart (read
> "complicated"). You could probably just store current list length and
> generate a random number between 0 and length-1.

No, algorithm here is more complicated. It must ensure that there would
not be second attempt to connect to host, for which unsuccessful
connection attempt was done. So, there is list rearrangement.

Algorithm for pick random list element by single pass is quite trivial.




-- 
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] [PATCH] Reload SSL certificates on SIGHUP

[Victor Wagner]

On Wed, 7 Sep 2016 17:09:17 +0900
Michael Paquier  wrote:

> On Sun, Sep 4, 2016 at 11:39 PM, Andreas Karlsson 
> wrote:
> > 1) Serialize the certificates, key, and CRL and write them to the
> > backend_var temp file and then deserialize everything in the
> > backends.
> >
> > Sounds like you would need to write some code for every SSL library
> > to support the serialization and deserialization, which I am not a
> > fan of doing just for one platform since I worry about little used
> > code paths. Additionally this would mean that we write a copy of
> > the private key to potentially another file system than the one
> > where the private key is stored, this sounds like a bad idea from a
> > security point of view.  
> 
> Yeah... This would result in something that is heavily SSL-dependent,
> which would be an additional maintenance pain when trying to support
> future OpenSSL versions.

OpenSSL has documented API for serializing/deserializing each and every
cryptographic format it supports. And this API quite unlikely to change
in future OpenSSL versions. Moreover, LibreSSL is compatible with this
API as far as I know.

Really, Apache does simular thing for ages - it starts as root, loads
certificates and keys, serializes them in memory, then forks and drops
privileges, and then uses these keys and certificates.

There are two problems with this approach

1. You have to carefully design data structures to store serialized
keys. Apache made a mistake there and didn't allow for future invention
of new public key algorithms.  So, in 2008 I had problems adding
russian GOST cryptography there.

2. You keys and certificates might not be stored in the filesystem at
all. They can live in some hardware cryptography module, which don't
let private keys out, just provide some handle.
(OpenSSL supports it via lodable engine modules, and Postgres allows to
use engine modules since 8.2, so people can use it with postgres).

Really OpenSSL/LibreSSL provide useful abstraction of memory BIO (Where
BIO stands for basic input/output) which can be used to store
serialized cryptographic data. And serializing/deserializing API is
designed to work with BIO anyway.




-- 
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] Patch: Implement failover on libpq connect level.

[Victor Wagner]

On Tue, 6 Sep 2016 07:58:28 +0530
Mithun Cy <mithun...@enterprisedb.com> wrote:

> 
> Now if only one host is in connection string and it ask for read_write
> connection(connect to master) I mean read_only is set 0 explicitly.
> With above logic we will allow it to connect to standby?. I still
> think psql connection to standby should be handled by changing the
> default value of readonly to 1 (which means connect to any). 

It would definitely be more logical, but I haven't found easy way to do
it. May be I should return to this place and rethink. I don't like and
idea to explicitely ignore connection string parameter due to some
condition. 

Really, I think, test for replication connection can be either
removed from this part of code now.  


> Thinking
> further probably replacing readonly parameter with
> targetServerType=any|master (with default being any) should clear
> some confusions and bring consistency since same is used in JDBC
> multi host connection string.

It seems that in this context change readonly=0|1 to
targetServerType=any|master  makes sense.


 
> 2)
> @@ -1460,33 +1538,80 @@ connectDBStart(PGconn *conn)
>portstr,
>(int) (UNIXSOCK_PATH_BUFLEN - 1));
>   conn->options_valid = false;
> + free(nodes->port);
> 
> nodes->port was not allocated at this point.
> 

I'll recheck it.

-- 
   Victor Wagner <vi...@wagner.pp.ru>


-- 
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] Patch: Implement failover on libpq connect level.

[Victor Wagner]

On Mon, 5 Sep 2016 07:59:02 +0530
Mithun Cy <mithun...@enterprisedb.com> wrote:

> On Aug 31, 2016 1:44 PM, "Victor Wagner" <vi...@wagner.pp.ru> wrote:
> > Thanks, I've added this to 7-th (yet unpublished here) version of my
> > patch.
> Hi victor, just wanted know what your plan for your patch 07. Would
> you like to submit it to the community. I have just signed up as a
> reviewer for your patch.

As community shows an interest with this patch, and it has been
included in the current commitfest, I've to submit it.

This version of patch includes

1. Most important - a tap test suite (really small enough and has a
room for improvements).

2. Compatibility with older versions - now readonly check is skipped if
only one hostname is specified in the connect string

3. pg_host and pg_port variables now show host name and port library
was connected to.

4. Your fix with compilation of ecpg tests.

Patch is attached.



-- 
   Victor Wagner <vi...@wagner.pp.ru>
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index f22e3da..835061d 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -792,7 +792,7 @@ host=localhost port=5432 dbname=mydb connect_timeout=10

The general form for a connection URI is:
 
-postgresql://[user[:password]@][netloc][:port][/dbname][?param1=value1...]
+postgresql://[user[:password]@][netloc][:port][,netloc[:port]...][/dbname][?param1=value1...]
 

 
@@ -809,6 +809,7 @@ postgresql://localhost/mydb
 postgresql://user@localhost
 postgresql://user:secret@localhost
 postgresql://other@localhost/otherdb?connect_timeout=10application_name=myapp
+postgresql://node1,node2:5433,node3:4432,node4/mydb?hostorder=randomreadonly=1
 
 Components of the hierarchical part of the URI can also
 be given as parameters.  For example:
@@ -831,7 +832,9 @@ postgresql:///mydb?host=localhostport=5433

 For improved compatibility with JDBC connection URIs,
 instances of parameter ssl=true are translated into
-sslmode=require.
+sslmode=require and
+loadBalanceHosts=true  into
+hostorder=random.

 

@@ -841,6 +844,10 @@ postgresql:///mydb?host=localhostport=5433
 postgresql://[2001:db8::1234]/database
 

+   
+   There can be serveral host specifications, optionally accompanied
+   with port, separated by comma.
+   
 

 The host component is interpreted as described for the parameter PostgreSQL was built). On machines without
 Unix-domain sockets, the default is to connect to localhost.

+   
+   There can be more than one host parameter in
+   the connect string. In this case these hosts would be considered
+   alternate entries into same database and if connect to first one
+   fails, library would try to connect second etc. This can be used
+   for high availability cluster or for load balancing. See
+parameter.
+   
+   
+   Network host name can be accompanied with port number, separated by
+   colon. If so, this port number is used only when connected to
+   this host. If there is no port number, port specified in the
+parameter would be used.
+   
   
  
 
@@ -942,8 +963,44 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname


   
-
+
+  
+  hostorder
+  
+  
+  Specifies how to choose host from list of alternate hosts,
+  specified in the  parameter.
+  
+  
+  If value of this argument is sequential (the
+  default) library connects to the hosts in order they specified,
+  and tries to connect second one only if connection to the first
+  fails.
+  
+  
+  If value is random host to connect is randomly
+  picked from the list. It allows to balance load between several
+  cluster nodes. However, currently PostgreSQL doesn't support
+  multimaster clusters. So, without use of third-party products,
+  only read-only connections can take advantage from the
+  load-balancing. See 
+  
+  
+  
+  
+  readonly
+  
+  
+  If this parameter is 0 (the default), upon successful connection
+  library checks if host is in recovery state, and if it is so,
+  tries next host in the connect string. If this parameter is
+  non-zero, connection to warm standby nodes are considered
+  successful.
+  
+  
+  
   
+
port


@@ -985,7 +1042,6 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
   
   
  
-
  
   connect_timeout
   
@@ -996,7 +1052,27 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
   
   
  
-
+ 
+ falover_timeout
+ 
+ 
+ Maximum time to cycilically retry all the hosts in commect string.
+ (as decimal integer number of seconds). If not specified, then
+ hosts are tried just once.
+ 
+ 
+ I

Re: [HACKERS] Patch: Implement failover on libpq connect level.

[Victor Wagner]

On Tue, 30 Aug 2016 14:54:57 +0530
Mithun Cy  wrote:

> On Fri, Aug 26, 2016 at 10:10 AM, Mithun Cy
>  wrote:
> >  
> > >rpath,'/home/mithun/edbsrc/patch6bin/lib',--enable-new-dtags
> > >-lecpg  
> > -lpgtypes -lpq -lpgcommon -lpgport -lz -lrt -lcrypt -ldl -lm   -o
> > test1  
> > >../../../../../src/interfaces/libpq/libpq.so: undefined reference
> > >to  
> > `pg_usleep'
> >  
> 
> As similar to psql I have added -lpq for compilation. So now ecpg
> tests compiles and make check-world has passed.

Thanks, I've added this to 7-th (yet unpublished here) version of my
patch, available on my site.



-- 
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] Patch: Implement failover on libpq connect level.

[Victor Wagner]

On Fri, 26 Aug 2016 10:10:33 +0530
Mithun Cy  wrote:

> On Thu, Mar 17, 2016 at 4:47 AM, David Steele 
> wrote:
> >Since there has been no response from the author I have marked this
> >patch  
> "returned with feedback".  Please feel free >to resubmit for 9.7!
> I have started to work on this patch, and tried to fix some of the
> issues discussed above. The most recent patch 06 has fixed many
> issues which was raised previously which include indefinite looping,
> crashes. And, some of the issues which remain pending are.
> 
> 1. Connection status functions PQport, PQhost do not give
> corresponding values of established connection. -- Have attached the
> patch for same.
> 
> 2. Default value of readonly parameter is 0, which means should
> connect to master only. So absence of parameter in connection string
> in simple cases like "./psql -p PORT database" fails to connect to
> hot standby server.  I think since if readonly=1 means connect to
> any. and readonly=0 means connect to master only, we should change
> the default value  to 1, to handle the cases when parameter is not
> specified.
> 
> JFYI Interestingly PostgreSql JDBC driver have following options
> targetServerType=any|master|slave|preferSlave for same purpose.
> 
> Also did a little bit of patch clean-up.
> 
> Also found some minor issues related to build which I am working on.
> 1. make check-world @src/interfaces/ecpg/test/connect fails with
> following error:
> gcc -Wall -Wmissing-prototypes -Wpointer-arith
> -Wdeclaration-after-statement -Wendif-labels
> -Wmissing-format-attribute -Wformat-security -fno-strict-aliasing
> -fwrapv -fexcess-precision=standard -g -O0 -pthread -D_REENTRANT
> -D_THREAD_SAFE -D_POSIX_PTHREAD_SEMANTICS test1.o -L../../ecpglib
> -L../../pgtypeslib -L../../../../../src/interfaces/libpq
> -L../../../../../src/port -L../../../../../src/common -Wl,--as-needed
> -Wl,-rpath,'/home/mithun/edbsrc/patch6bin/lib',--enable-new-dtags
> -lecpg -lpgtypes -lpq -lpgcommon -lpgport -lz -lrt -lcrypt -ldl -lm
> -o test1 ../../../../../src/interfaces/libpq/libpq.so: undefined
> reference to `pg_usleep'
> collect2: error: ld returned 1 exit status
> make[3]: *** [test1] Error 1
> make[3]: Leaving directory `/home/mithun/mynewpost/p1/
> src/interfaces/ecpg/test/connect'
> 
> patch has used pg_usleep() which is in L../../../../../src/port  I
> think dependency is not captured rightly.
> 
> Thanks and Regards
> Mithun C Y
> 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] Patch: Implement failover on libpq connect level.

[Victor Wagner]

On Fri, 26 Aug 2016 10:10:33 +0530
Mithun Cy <mithun...@enterprisedb.com> wrote:

> On Thu, Mar 17, 2016 at 4:47 AM, David Steele <da...@pgmasters.net>
> wrote:
> >Since there has been no response from the author I have marked this
> >patch
> "returned with feedback".  Please feel free >to resubmit for 9.7!
> I have started to work on this patch, and tried to fix some of the
> issues discussed above. The most recent patch 06 has fixed many
> issues which was raised previously which include indefinite looping,
> crashes. And, some of the issues which remain pending are.

Although I have not resubmutted my patch yet, I've continue to work on
it, so you can see my last version on 

https://www.wagner.pp.ru/fossil/pgfailover.

It seems that I've already fixed some of issues you mentioned below.
Also I've added testsuite, which is incomplete.

> JFYI Interestingly PostgreSql JDBC driver have following options
> targetServerType=any|master|slave|preferSlave for same purpose.

Probably we should havbe a comptibility alias.


-- 
   Victor Wagner <vi...@wagner.pp.ru>


-- 
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] UTF-8 docs?

[Victor Wagner]

On Mon, 22 Aug 2016 10:53:25 -0400
Peter Eisentraut  wrote:

> On 8/22/16 9:32 AM, Tatsuo Ishii wrote:
> > I don't know what kind of problem you are seeing with encoding
> > handling, but at least UTF-8 is working for Japanese, French and
> > Russian.  
> 
> Those translations are using DocBook XML.
> 

Russian translation by Postgres Professional does use DocBook SGML,
although it uses xml as intermediate representation when applying
gettext to the documentation. I've already posted URL where sources of
postgresql with russian documentation in SGML format included can be
downloaded.





-- 
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] UTF-8 docs?

[Victor Wagner]

On Mon, 22 Aug 2016 14:16:45 +0900 (JST)
Tatsuo Ishii  wrote:

> Just out of curiopusity, I wonder why we can't make the encoding of
> SGML docs to be UTF-8, rather than current ISO-8859-1.


What a reason of "make the encoding of sgml docs" to be something?
What actual change should be made and what problems it would solve?

There are various translations of postgreSQL docs, and they use various
encodings. Translated versions of docs on http://postgresql.org/docs
are just links to external sites where translations are maintained. 

English documentation uses ISO-8859-1 (actually ASCII),
Russian uses UTF-8 (you can download our source tarball from
http://repo.postgrespro.ru/pgpro-9.5/src and see postgres source
distribution with UTF-8 sgmls inside). 

Japanese documentation in HTML form is served from
http://www.postgresql.jp/document/9.5/html/
in utf-8 too.

I.e. everybody who need utf-8 to represent translation of
documentation, already uses it.

What exatly you proposes do be done?

Really, what change we need, it is conversion from SGML to XML format. 
It would solve some real problems, such as ability to include diagrams
in the docs, and also let everyone to explicitely specify encoding in
XML declaration (and probably cause switch to UTF-8 as side effect,
because most XML-based tools use UTF-8 as default).


-- 
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] Use pread and pwrite instead of lseek + write and read

[Victor Wagner]

On Wed, 17 Aug 2016 12:17:35 +0200
Magnus Hagander <mag...@hagander.net> wrote:

> On Wed, Aug 17, 2016 at 11:34 AM, Victor Wagner <vi...@wagner.pp.ru>
> wrote:

> > I don't think that all platforms, supported by PostgreSQL support
> > this API. Especially, I cannot find any mention of pread/pwrite in
> > the Win32 except this thread on stackoverflow:
> >
> >  
> Yeah, Windows does not have those API calls, but it shouldn't be
> rocket science to write a wrapper for it. The standard windows APIs
> can do the same thing -- but they'll need access to the HANDLE for
> the file and not the posix file descriptor.

There is _get_osfhandle function, which allows to find out Windows
HANDLE associated with posix file descriptor.

Really my question was - someone should write these wrappers into
src/port and add corresponding test to the configure and/or CMakefile 
for this patch to be complete.





-- 
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] Use pread and pwrite instead of lseek + write and read

[Victor Wagner]

On Wed, 17 Aug 2016 10:58:09 +0300
Oskari Saarenmaa  wrote:



> 
> The attached patch replaces FileWrite and FileRead with FileWriteAt
> and FileReadAt and removes most FileSeek calls.  FileSeek is still
> around so we can find the end of a file, but it's not used for
> anything else.

It seems that configure test for availability of pread/pwrite functions
and corresponding #define is needed.
 
I don't think that all platforms, supported by PostgreSQL support this
API. Especially, I cannot find any mention of pread/pwrite in the Win32 
except this thread on stackoverflow:


http://stackoverflow.com/questions/766477/are-there-equivalents-to-pread-on-different-platforms



-- 
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] handling unconvertible error messages

[Victor Wagner]

On Sat, 13 Aug 2016 12:02:30 -0400
Tom Lane <t...@sss.pgh.pa.us> wrote:

> Victor Wagner <vi...@wagner.pp.ru> writes:
> > I think it is better to avoid such a problem and fix system so
> > server would never send a message in the encoding, different from
> > client one.  
> 
> Don't hold your breath waiting for that to happen.
> 
> Quite aside from the question of whether we want to trust GUC settings
> from the startup packet before we've authenticated the user, there's a

What's wrong with trusting this particular setting? I cannot think of
any meaningful exploit. Really, there are lot of http servers out
there, which typically do accept connections from anywhere (which is
seldom case for postgresql servers) and trust Accept-Charset and
Accept-Language client header without any authentication.

There can be attacks that exploits errors in the message parsing, 
but startup message is parsed anyway before authentication.

> small problem that the server *can't* translate *any* encoding until
> it's successfully connected to a database and is able to read the
> pg_conversion catalog.

> 
> We might be able to institute some rule like "examine the startup
> packet and see if it specifies a client_encoding different from what
> we inherited from the postmaster.  If not, continue with current
> behavior (send messages localized per postmaster's settings).  If so,
> fall back to English messages/C locale until startup is complete."
> This would preserve current functionality in cases where it actually,
> er, functions, while giving something at least passable in the cases
> that are broken today.

I think that we can do a bit more than this. We use GNU gettext
library to provide message translation. These library are able to
perform limited set of encoding conversion itself.

So, we can have two-stage fallback here:

1. If encoding is different, but compatible with language, inherited
from postmaster, ask gettext via bind_textdomain_encoding function to
provide messages in this encoding.

2. If it is not possible, fall back to English messages, which are
compatible with any of supported encoding. The same goes for session
which doesn't specify encoding at all (i.e. CancelMessage).



> 
>   regards, tom lane
> 
> 



-- 
   Victor Wagner <vi...@wagner.pp.ru>


-- 
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] handling unconvertible error messages

[Victor Wagner]

On Sat, 13 Aug 2016 09:24:47 +
Vladimir Sitnikov <sitnikov.vladi...@gmail.com> wrote:

> Victor>We don't have 190 message  catalog translations in the
> Victor>PostgreSQL. So problem with encoding for messages is quite
> Victor>limited.
> 
> Even though the number of translations is limited, there's a problem
> when trying to tell one "one-byte-encoding" from another "one-byte"
> one. It would be so much better if ServerErrorMessages included
> encoding right in the message itself.

I think it is better to avoid such a problem and fix system so server
would never send a message in the encoding, different from client one.
It is not a client job to convert encodings.

In most cases server does know which encoding client requests from the
very first protocol message. (if it is startup message). 
So, server can easily tell if it is able to convert NLS messages into
the client desired encoding, and if not - fall back to untranslated
messages.





-- 
   Victor Wagner <vi...@wagner.pp.ru>


-- 
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] handling unconvertible error messages

[Victor Wagner]

On Wed, 10 Aug 2016 11:08:43 +0900 (Tokyo Standard Time)
Kyotaro HORIGUCHI <horiguchi.kyot...@lab.ntt.co.jp> wrote:

> Hello,
> 
> (I've recovered the lost Cc recipients so far)
> 
> At Mon, 8 Aug 2016 12:52:11 +0300, Victor Wagner <vi...@wagner.pp.ru>
> wrote in <20160808125211.1361c...@fafnir.local.vm>
> > On Mon, 08 Aug 2016 18:28:57 +0900 (Tokyo Standard Time)
> > Kyotaro HORIGUCHI <horiguchi.kyot...@lab.ntt.co.jp> wrote:  
> > > 
> > > I don't see charset compatibility to be easily detectable,  
> > 
> > In the worst case we can hardcode explicit compatibility table.  
> 
> We could have the language lists compatible with some
> language-bound encodings.  For example, LATIN1 (ISO/IEC 8859-1),
> according to Wikipedia
> (https://en.wikipedia.org/wiki/ISO/IEC_8859-1)
> 
> According to the list, we might have the following compatibility
> list of locales, maybe without region.
> 
> {{"UTF8", "LATIN1"}, "af", "sq", "eu", "da", "en", "fo", "en"}... and
> so.
> 
> The biggest problem for this is at least *I* cannot confirm the
> validity of the list. Both about perfectness of coverage of

I think that people from localization team can. At least authors of
particular translation can tell which encodings support their language.

> ISO639-1 seems to have about 190 languages and most of them are

We don't have 190 message  catalog translations in the PostgreSQL.
So problem with encoding for messages is quite limited.

> 
> I suppose that 'fallback' means "have a try then use English if
> failed" so I think it is sutable rather for message, not for
> data, and it doesn't need any a priori information about

Yes, I'm talking about messages, not about encoding conversion for
data. As far as my experience goes, data in the PostgreSQL are
converted more or less predictable way. May be it could be improved,
but it is possible to set up client and server such way it would do a
right job. 

Situation with messages, especially ones which are returned before
establishing of the session completes (or when it fails) now is a bit
worse.

> compatibility. It seems to me that PostgreSQL refuses to ignore

Alas, it does. At least with example given by Peter Eisentraut at the
start of this thread.

> or conceal conversion errors and return broken or unwanted byte
> sequence for data.  Things are different for error messages, it
> is preferable to be anyyhow readable than totally abandoned.
> 
> > I think that for now we can assume that the best effort is already
> > done for the data, and think how to improve situation with
> > messages.  
> 
> Is there any source to know the compatibility for any combination
> of language vs encoding? Maybe we need a ground for the list.
> 
> regards,
> 



-- 
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] handling unconvertible error messages

[Victor Wagner]

On Mon, 08 Aug 2016 18:28:57 +0900 (Tokyo Standard Time)
Kyotaro HORIGUCHI  wrote:

> 
> I don't see charset compatibility to be easily detectable,

In the worst case we can hardcode explicit compatibility table.
There is limited set of languages, which have translated error messages,
and limited (albeit wide) set of encodings, supported by PostgreSQL. So
it is possible to define complete list of encodings, compatible with
some translation. And fall back to untranslated messages if client
encoding is not in this list.

> because locale (or character set) is not a matter of PostgreSQL
> (except for some encodings bound to one particular character
> set)... So the conversion-fallback might be a only available
> solution.

Conversion fallback may be a solution for data. For NLS-messages I think
it is better to fall back to English (untranslated) messages than use of
transliteration or something alike.

I think that for now we can assume that the best effort is already done
for the data, and think how to improve situation with messages.



-- 
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] handling unconvertible error messages

[Victor Wagner]

On Mon, 08 Aug 2016 18:11:54 +0900 (Tokyo Standard Time)
Kyotaro HORIGUCHI  wrote:

> At Mon, 08 Aug 2016 17:18:21 +0900 (Tokyo Standard Time), Kyotaro
> HORIGUCHI  wrote in
> <20160808.171821.100221089.horiguchi.kyot...@lab.ntt.co.jp>
> 
> Somewhat wrong. The core problem is the procedures offered by
> PrepareClientEncoding is choosed only by encoding->encoding
> basis, not counting character set compatibility. So, currently
> this is not detectable before actually doing conversion of a
> character stream.

Yes, my idea was to check language/encoding compatibility. Make sure
that NLS messages can be represented in the client-specified encoding
in a readable way. As far, as I know, there is no platform-independent
bulletproof way to do so. 

On Unix you can try to initialize locale with given language and given
encoding, but it can fail even if encoding is compatible with language,
simply because corresponding locale is not generated on this system.

But this seems to be a problem of system administration and can be left
out to local sysadmins.

Once you have correctly initialized LC_MESSAGES, you don't need
encoding conversion routines for the NLS messages. You can use
bind_textdomain_codeset function to provide messages in the
client-desired encoding. (but this can cause problems with server logs,
where messages from different sessions would come in different
encodings)

On Windows things are more complicated. There is just one ANSI code
page, associated to given language, and locale initialization would
fail with any other codepage, including utf-8.



 
> regards,
> 



-- 
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] handling unconvertible error messages

[Victor Wagner]

On Mon, 08 Aug 2016 17:18:21 +0900 (Tokyo Standard Time)
Kyotaro HORIGUCHI  wrote:
> 
> I'm not sure what messages may be raised before authentication
> but it can be a more generic-solution. (Adding check during
> on-session.)

Definitely, there can be authentication error message, which is sent if
authentication didn't happen. Also, as far as I understand, message
"Database ... doesn't exists" is also send before authentication.


Also, there are CancelRequests, where normal authentication is not
used, and server key, provided in another session used instead.





-- 
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] handling unconvertible error messages

[Victor Wagner]

On Fri, 5 Aug 2016 11:21:44 -0400
Peter Eisentraut <peter.eisentr...@2ndquadrant.com> wrote:

> On 8/4/16 2:45 AM, Victor Wagner wrote:
> > 4. At the session startup try to reinitializie LC_MESSAGES locale
> > category with the combination
> > of the server (or better client-send) language and region and
> > client-supplied encoding, and if this failed, use untranslated error
> > message. Obvoisly, attempt to set locale to ru_RU.ISO8859-1 would
> > fail. so, if client would ask server with ru_RU.UTF-8 default
> > locale to use LATIN1 encoding, server would fallback to
> > untranslated messages.  
> 
> I think this is basically my solution (1), with the same problems.
> 



I think, that there is a big difference from server point of view.

You propose that both translated and untranslated message should be
passed around inside backend. It has some benefits, but requires
considerable reworking of server internals.

My solution doesn't require keeping both original message and
translated one during all call stack unwinding. It just checks if
combination of language and encoding is supported by the NLS subsystem,
and if not, falls back to untranslated message  for entire session.

It is much more local change and is comparable by complexity with one,
proposed by 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] handling unconvertible error messages

[Victor Wagner]

On Fri, 5 Aug 2016 11:23:37 -0400
Peter Eisentraut  wrote:

> On 8/4/16 9:42 AM, Tom Lane wrote:
> > I'm inclined to think that we should reset the message locale
> > to C as soon as we've forked away from the postmaster, and leave it
> > that way until we've absorbed settings from the startup packet.
> > Sending messages of this sort in English isn't great, but it's
> > better than sending completely-unreadable ones.  Or is that just my
> > English-centricity showing?  
> 
> Well, most of the time this all works, only if there are different
> client and server settings you might have problems.  We wouldn't want
> to partially disable the NLS feature for the normal case.
> 
There are cases, where client cannot tell server which encoding it
wants to use, and server cannot tell which encoding it uses, but it can
send error messages. For example, CancelRequest.

The only way to ensure that message is readable in this case is to fall
back to some encoding, definitely known by both client and server.
And for now it is US-ASCII. 

It is, as far as I understand, what Tom is proposing:
Fall back to the untranslated message at the beginning of session, and
return to NLS only when encoding is successfully negotiated between
client and server.

May be, there can be other solution - prepare client to be able to
accept UTF-8 messages from server regardless of encoding, i.e. if
message starts with BOM marker (0xFEFF unicode char, EF BB BF byte
sequence in utf-8), interpret it as UTF-8. It would require client to
support some kind of encoding conversion, and in some 8-bit
environments pose problems with displaying these messages.

-- 




-- 
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] handling unconvertible error messages

[Victor Wagner]

On Thu, 04 Aug 2016 14:25:52 -0400
Tom Lane <t...@sss.pgh.pa.us> wrote:

> Victor Wagner <vi...@wagner.pp.ru> writes:

> > Really, if this response is sent after backend has been forked,
> > problem probably can be easily fixed better way - StartupMessage
> > contain information about desired client encoding, so this
> > information just should be processed earlier than any other
> > information from this message, which can cause errors (such as
> > database name).  
> 
> I think that's wishful thinking.  There will *always* be errors that
> come out before we can examine the contents of the startup message.
> Moreover, until we've done authentication, we should be very wary of
> applying client-specified settings at all: they might be malicious.

I think that this case can be an exception from the rule "don't apply
settings from the untrusted source".

Let's consider possible threat model:

1. We anyway parse StartupMessage before authentication. There is
nothing we can do with it, so parser should be robust enough, to handle
untrusted input. As I can see from the quick glance, it is.

2. When encoding name is parsed, it is used to search in the array of
supported encoding. No possible attack here - either it is valid or not.

3. As far as I know, we don't allow client to change language, only
encoding, so it is not even possible that attacker could make messages
in the log unreadable for the system administartor.

So, if we would fix the problem, reported by Peter Eisentraut at the
begining of this thread, and fall back to untranslated messages
whenever client-requested encoding is unable to represent messages in
the server default language, this solution,  would be not worse than
your solution. 

There would be fallback to C locale in any case of doubt, but in the
case when NLS messages can be made readable, they would be readable.


Really, there is at least one case, when fallback to C locale should be
done unconditionally - a CancelRequest. In this case client cannot send
an encoding, so C locale should be used.

As far as I understand it is not the case with SSLRequest. Although it
doesn't contain encoding information as well as CancelRequest, errors
in subsequent SSL negotiations would be reported by client-side SSL
libraries, not by server.
-- 




> 
>   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] handling unconvertible error messages

[Victor Wagner]

On Thu, 04 Aug 2016 09:42:10 -0400
Tom Lane <t...@sss.pgh.pa.us> wrote:

> Victor Wagner <vi...@wagner.pp.ru> writes:
> > If error occurs during processing of StartMessage protocol message,
> > i.e. client request connection to unexisting database,
> > ErrorResponse would contain message in the server default locale,
> > despite of client encoding being specified in the StartMessage.  
> 
> Yeah.  I'm inclined to think that we should reset the message locale
> to C as soon as we've forked away from the postmaster, and leave it
> that way until we've absorbed settings from the startup packet.
> Sending messages of this sort in English isn't great, but it's better
> than sending completely-unreadable ones.  Or is that just my
> English-centricity showing?

From my russian point of view, english messages are definitely better
than transliteration of Russian  with latin letters (although it is
not completely unreadable), not to mention wrong encoding or lots of
question marks.

Really, if this response is sent after backend has been forked, problem
probably can be easily fixed better way - StartupMessage contain
information about desired client encoding, so this information just
should be processed earlier than any other information from this
message, which can cause errors (such as database name).

If this errors are sent from postmaster itself, things are worse,
because I don't think that locale subsystem is desined to be
reintitalized lots of times in the same process. 
But postmaster itself can use non-localized messaging. Its messages in
the logs are typically analyzed by more or less qualified DBA and
system admistrators, not by end user.





-- 
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] handling unconvertible error messages

[Victor Wagner]

On Mon, 25 Jul 2016 10:43:44 -0400
Peter Eisentraut  wrote:

e is
> a real error message, but it could not be converted.
> 
> I think ideally we could make this better in two ways:
> 
> 1) Send the original error message untranslated.  That would require
> saving the original error message in errmsg(), errdetail(), etc.  That
> would be a lot of work for only the occasional use.  But it would also
> facilitate an occasionally-requested feature of writing untranslated
> error messages into the server log or the csv log, while sending
> translated messages to the client (or some variant thereof).
> 
> 2) Send an indication that there was an encoding problem.  Maybe a
> NOTICE, or an error context?  Wiring all this into elog.c looks a bit
> tricky, however.
> 
> Ideas?

I think there are two more ways:

(3 was in the Craig's message)

4. At the session startup try to reinitializie LC_MESSAGES locale
category with the combination
of the server (or better client-send) language and region and
client-supplied encoding, and if this failed, use untranslated error
message. Obvoisly, attempt to set locale to ru_RU.ISO8859-1 would fail.
so, if client would ask server with ru_RU.UTF-8 default locale to use
LATIN1 encoding, server would fallback to untranslated messages.

This approach would have problems on windows, where locale is strictly
tied to the ANSI encoding of given language/territory. Even if we would
make UTF-8 a special case, attempt to connect with encoding KOI8 or
LATIN5 to the Windows postgresql server which runs in
Russian_Russia.1251 locale would result in the fallback to untranslated
message. But I think that this case is marginal and better to present
untranslated messages to the people (or applications) which require
non-default 8-bit encoding even if it is possible to represent
translated messages in this encoding, than to present unreadable
translated messages to anybody.

5. Use transliteration in case of encoding problem. Some iconv
implementations (such as Linux glibc iconv and GNU portable libiconv)
supports //TRANSLIT sufix for encoding and if this suffix specified
replace unrepresentable symbols with phonetically similar approximation.
I don't know how well it would work for Japanese, but for Russian it is
definitely better than lots of question marks.

-- 



> 



-- 
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] handling unconvertible error messages

[Victor Wagner]

On Mon, 25 Jul 2016 10:43:44 -0400
Peter Eisentraut  wrote:

> Example: I have a database cluster initialized with
> --locale=ru_RU.UTF-8 (built with NLS).  Let's say for some reason, I
> have client encoding set to LATIN1.  All error messages come back
> like this:
> 
> test=> select * from notthere;  
> ERROR:  character with byte sequence 0xd0 0x9e in encoding "UTF8" has
> no equivalent in encoding "LATIN1"
> 
> There is no straightforward way for the client to learn that there is
> a real error message, but it could not be converted.

Really, situation is a bit worse. There is at least one case, where
error message comes unreadble to the client, even if encodings are
compatible.

I.e. if server default locale is ru_RU.UTF-8 and client requestes
encoding WIN1251 which is able to handle cyrillic.

If error occurs during processing of StartMessage protocol message,
i.e. client request connection to unexisting database,
ErrorResponse would contain message in the server default locale,
despite of client encoding being specified in the StartMessage.

If session is correctly established with such parameters, error
messages are displayed correctly.

I haven't yet investigatged if it is just delayed initialization of
backend locale system or backend is not yet forked at the time of
generation of this message and wrongly encoded message is sent by
postmaster.





-- 
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] OpenSSL 1.1 breaks configure and more

[Victor Wagner]

On Fri, 1 Jul 2016 02:27:03 +0200
Andreas Karlsson  wrote:


> 0003-Remove-OpenSSL-1.1-deprecation-warnings.patch
> 
> Silence all warnings. This commit changes more things and is not 
> necessary for getting PostgreSQL to build against 1.1.

This patch breaks feature, which exists in PostgreSQL since 8.2 -
support for SSL ciphers, provided by loadable modules such as Russian
national standard (GOST) algorithms, and support for cryptographic
hardware tokens (which are also supported by loadble modules called
engines in OpenSSL).

Call for OPENSSL_config was added to PostgreSQL for this purpose - it
loads default OpenSSL configuration file, where such things as crypto
hardware modules can be configured.

If we wish to keep this functionality, we need to explicitely call

OPENSSL_init_ssl(INIT_LOAD_CONFIG,NULL) instead of deprecated
OPENSSL_config in 1.1.0








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

[HACKERS] Building PL/Perl with ActiveState Perl 5.22 and MSVC

[Victor Wagner]

Collegues,

Since patches to support building postgres itself with newest version
of ActivePerl was commited into REL9_5_STABLE branch, I've tried to
build postgres with PL/Perl using this version of Perl. I'm using
Visual Studio 2013 Community edition.

It turns out that while ActiveState seems to drop support of embedding
their perl into msvc-compiled appications, there are just few minor
issues which prevent PL-perl to compile.

1. ActiveState Perl doesn't ship MSVC-build import library perl522.lib
for their perl522.dll. Instead they ship MINGW-build library
libperl522.a.

Visual Studio 2012 is able to link against this library. It is only
matter of modifing search expresions in Mkvcbuild.pm to be able to find
this import library. Not sure if it stands true for all eariler
versions of Visual Studio, supported by Postgresql.

2. There is macro PERL_STATIC_INLINME in perl's lib/CORE/config.h file,
which produces compilation errors.

There are following comments there:

/* HAS_STATIC_INLINE:
 *  This symbol, if defined, indicates that the C compiler supports
 *  C99-style static inline.  That is, the function can't be called
 *  from another translation unit.
 */
/* PERL_STATIC_INLINE:
 *  This symbol gives the best-guess incantation to use for static
 *  inline functions.  If HAS_STATIC_INLINE is defined, this will
 *  give C99-style inline.  If HAS_STATIC_INLINE is not defined,
 *  this will give a plain 'static'.  It will always be defined
 *  to something that gives static linkage.
 *  Possibilities include
 *  static inline   (c99)
 *  static __inline__   (gcc -ansi)
 *  static __inline (MSVC)
 *  static _inline  (older MSVC)
 *  static  (c89 compilers)
 */
#define HAS_STATIC_INLINE   /**/
#define PERL_STATIC_INLINE static __inline__  /**/


Changing that macro to one described in the comments as "for MSVC" make
compilation errors to go away.  Unfortunately, I have no idea how to
fix this from within postgresql compilation process.

3. Fixing two issues above was enough to make build complete for 64-bit
windows target. With 32-bit build I'v got linking errors 
LINK2026: module unsafe for safeseh image

for all modules which are linked with perl DLL. I suspect that it is
not  a problem with DLL itself, it is rather related to way MINGW32
generates its import libraries. To fix this problem XML element
false
should be added inside  element of the two vcxproj files which
link with perl - plperl.vcxproj and hstore_plperl.vcxproj.

It seems that this option appeared in VC2003, so it should be safe to
add.

Hope that results of my experiments would be useful for someone.


-- 
       Victor Wagner <vi...@wagner.pp.ru>


-- 
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] Convert pltcl from strings to objects

[Victor Wagner]

On Tue, 23 Feb 2016 17:14:26 -0600
Jim Nasby <jim.na...@bluetreble.com> wrote:

> On 2/23/16 6:04 AM, Victor Wagner wrote:

> 
> > Please, send updated patch to the list in this thread, so it would
> > appear in the commitfest and I can mark your patch as "ready for
> > committer".  
> 
> Done!

Nice job. I've marking the patch as "Ready for committer".



-- 
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] Convert pltcl from strings to objects

[Victor Wagner]

On Mon, 22 Feb 2016 17:57:36 -0600
Jim Nasby <jim.na...@bluetreble.com> wrote:

> On 2/18/16 6:26 AM, Victor Wagner wrote:

> Are you referring to this comment?
> 
> > /
> >  * Add the proc description block to the
> > hashtable.  Note we do not
> >  * attempt to free any previously existing prodesc
> > block.  This is
> >  * annoying, but necessary since there could be
> > active calls using
> >  * the old prodesc.
> >  /  
> 
> That is not changed by the patch, and I don't think it's in the scope
> of this patch. I agree it would be nice to get rid of that and the
> related malloc() call, as well as what perm_fmgr_info() is doing, but
> those are unrelated to this work.

If it has been there, before, it is OK to leave it this way.
It was rather wild guess that use of referential counting which we now
have here, can solve this old problem.
 
> (BTW, I also disagree about using a Tcl list for prodesc. That's an 
> object in a *Postgres* hash table; as such it needs to be managed by 
> Postgres, not Tcl. AFAIK the Postgres ResourceOwner stuff would be
> the correct way to do that (but I'm not exactly an expert on that
> area).

As far as I understand ResourceOwner is completely different garbage
collection scheme, than reference counting. ResourceOwner mechanism lets
free everything associated with transaction when transaction is over.

Here we have another case. prodesc is a global thing. And it is shared
between different operations. Problem was that there is no partcular
owner, and we have to wait when last operation which deals with it
would finish. It looks like perfect job for reference counting.

Actually, it doesn't matter much, because it wastes some memory only on
procedure change, and typically in the heavy loaded production
environments server side code doesn't change too frequently.

> 
> > Function pltcl_elog have a big switch case to convert enum
> > logpriority, local to this function to PostgreSql log levels.
> >
> > It seems not a most readable solution, because this enumeration is
> > desined to be passed to Tcl_GetIndexFromObj, so it can 
[skip]
> 
> Done.

Glad that my suggestion was useful to you.

> 
> > It seems that you are losing some diagnostic information by
> > extracting just message field from ErrorData, which you do in
> > pltcl_elog and pltcl_subtrans_abort.
> >
> > Tcl has  mechanisms for passing around additional error information.
> > See Tcl_SetErrorCode and Tcl_AddErrorInfo functions
> >
> > pltcl_process_SPI_result might benefit from providing SPI result
> > code in machine readable way via Tcl_SetErrorCode too.  
> 
> Is there any backwards compatibility risk to these changes? Could
> having that new info break someone's existing code?

I don't think so. ErrorCode and ErrorInfo mechanisms present in the Tcl
for ages. As far as I remember, it predates Tcl_Obj API. Unfortunately
I've lost my copy of 2-nd edition of "Practical Programming on Tcl/Tk"
and have only 4-th edition handy, so it's quite hard to dig out Tcl 7.6
docs. 

Moreover. errorcode and errorinfo information travel behind the scenes
and cannot break any code which doesn't try to explicitely access it.


> > In some cases you use Tcl_SetObjResult(interp,
> > Tcl_NewStringObj("error message",-1)) to report error with constant
> > message, where in other places Tcl_SetResult(interp,"error
> > message",TCL_STATIC) left in place.
> >
> > I see no harm in using old API with static messages, especially if
> > Tcl_AppendResult is used, but mixing two patterns above seems to be
> > a bit inconsistent.  
> 
> Switched everything to using the new API.
> 
> > As far as I can understand, using Tcl_StringObj to represent all
> > postgresql primitive (non-array) types and making no attempt to
> > convert tuple data into integer, boolean or double objects is
> > design decision.
> >
> > It really can spare few unnecessary type conversions.  
> 
> Yeah, that's on the list. But IMHO it's outside the scope of this
> patch.

I've mean, that there is no need to be to aggressive in the type
conversion. You'll never know if Tcl code would actually access some
tuple elements or not. Let's leave conversion to the time of actual use
of values.
> 
> > Thanks for your effort.  
> 
> Thanks for the review!

Please, send updated patch to the list in this thread, so it would
appear in the commitfest and I can mark your patch as "ready for
committer".







-- 
   Victor Wagner <vi...@wagner.pp.ru>


-- 
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] Convert pltcl from strings to objects

[Victor Wagner]

On Tue, 9 Feb 2016 16:23:21 -0600
Jim Nasby  wrote:

> Currently, pl/tcl is implemented through nothing but string 
> manipulation. In other words, the C code is effectively creating a
> giant string that the tcl interpreter must re-parse every time the
> function is executed. Additionally, all arguments are treated as raw
> strings, instead of the far more efficient internal tcl object types.
> 
> The biggest win comes with how pltcl interfaces with SPI result sets. 
> Currently, the entire chunk of tcl code that is executed for each
> result row must be reparsed and recompiled from scratch. Now, the
> code is compiled once and the bytecode is stored.
> 
> This work is sponsored by Flight Aware (http://flightaware.com/).

I've looked to your patch. As far as I know Tcl Object API it looks
quite good.

Of course, it breaks compatibility with pre-8.0 versions of Tcl, but it
is almost twenty years since Tcl 8.0 come out.

I think that checking for pre-8.0 Tcl and defining Tcl_StringResult in
this case, should be removed from the code, because there is no object
API in pre-8.0 Tcl anyway, and it doesn't worth effort to maintain
compatibility. (line 51 of pltcl.c). 

There is suspicious  place at the end of compile_pltcl_fuction function,
where you've put comment that old prodesc cannot be deallocated,
because it might be used by other call.

It seems that reference counting mechanism which Tcl already has, can
help here. Would there be serious performance impact if you'll use Tcl
list instead of C structure to store procedure description? 
If not, you can rely on Tcl_IncrRefCount/Tcl_DecrRefCount to free a
procedure description when last active call finishes.

Function pltcl_elog have a big switch case to convert enum logpriority,
local to this function to PostgreSql log levels.

It seems not a most readable solution, because this enumeration is
desined to be passed to Tcl_GetIndexFromObj, so it can be used and is
used to index an array (logpriorities array of string representation).
You can define simular array with PostgreSQL integer constant and
replace page-sized switch with just two lines - this array definition
and getting value from it by index

static CONST84 int
loglevels[]={DEBUG,LOG,INFO,NOTICE,WARNING,ERROR,FATAL};


Tcl_GetIndexFromObj(

level=loglevels[priIndex];


It seems that you are losing some diagnostic information by
extracting just message field from ErrorData, which you do in 
pltcl_elog and pltcl_subtrans_abort.

Tcl has  mechanisms for passing around additional error information.
See Tcl_SetErrorCode and Tcl_AddErrorInfo functions

pltcl_process_SPI_result might benefit from providing SPI result code
in machine readable way via Tcl_SetErrorCode too.


In some cases you use Tcl_SetObjResult(interp, Tcl_NewStringObj("error
message",-1)) to report error with constant message, where in other
places Tcl_SetResult(interp,"error message",TCL_STATIC) left in place.

I see no harm in using old API with static messages, especially if
Tcl_AppendResult is used, but mixing two patterns above seems to be a
bit inconsistent.

As far as I can understand, using Tcl_StringObj to represent all
postgresql primitive (non-array) types and making no attempt to convert
tuple data into integer, boolean or double objects is design decision.

It really can spare few unnecessary type conversions. 

Thanks for your effort.

-- 



-- 
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] Small PATCH: check of 2 Perl modules

[Victor Wagner]

On Tue, 16 Feb 2016 12:23:56 -0300
Alvaro Herrera <alvhe...@2ndquadrant.com> wrote:

> ... but I agree with the point upthread that this should wait to see
> what happens with the CMake stuff, since this is not a newly
> introduced problem.

I doubt, that CMake stuff would be ready for 9.6. There is just one
commitfest left, and it would be quite a radical change.

And even if would appear in the next release, it cannot be easily
backpatched to 9.5. So we'll probably live with autoconf until the
end-of-life of 9.5 series at least.  



-- 
       Victor Wagner <vi...@wagner.pp.ru>


-- 
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] Small PATCH: check of 2 Perl modules

[Victor Wagner]

On Mon, 15 Feb 2016 08:34:11 -0500
Peter Eisentraut  wrote:

> On 2/12/16 8:20 AM, Eugene Kazakov wrote:
> > TAP-tests need two Perl modules: Test::More and IPC::Run.
> > 
> > The patch adds checking of modules in configure.in and configure.  
> 
> I think those modules are part of a standard Perl installation.

Not everyone have "standard perl installation" nowadays. Most Linux
users, for example, use Perl package from the distrubution, and
distributions love to strip down standard perl installation putting its
parts into separate packages, some of which might be optional.

For example, in Centos 6 it is part of perl-Test-Simple package. which
might be not included into docker images or simular minimal systems for
container-based deployment.

So, it worth few lines of the configure.in to remind user that "Your
perl installation is not standard enough".
-- 
 



-- 
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] In-core regression tests for replication, cascading, archiving, PITR, etc.

[Victor Wagner]

On Thu, 4 Feb 2016 12:59:03 +0300
Michael Paquier  wrote:

 for it?
> 
> > 1) Better to raise more meaningful error when IPC::Run is absent.  
> 
> This has been discussed before, and as far as I recall the current
> behavior has been concluded as being fine. That's where
> --enable-tap-tests becomes meaningful.

Really, it is not so hard to add configure checks for perl modules.
And we need to test not only for IPC::Run, but for Test::More too,
because some Linux distributions put modules which come with perl into
separate package.

The only problem that most m4 files with tests for perl modules, which
can be found in the Internet, have GPL license, so someone have either
to write his own and publish under PostgreSQL license or contact
author of one of them and ask to publish it under PostgreSQL license.

First seems to be much easier.
 



-- 
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] In-core regression tests for replication, cascading, archiving, PITR, etc.

[Victor Wagner]

On Thu, 4 Feb 2016 18:33:27 +0300
Michael Paquier <michael.paqu...@gmail.com> wrote:


> > Really, it is not so hard to add configure checks for perl modules.
> > And we need to test not only for IPC::Run, but for Test::More too,
> > because some Linux distributions put modules which come with perl
> > into separate package.  
> 
> The last time we discussed about that on this list we concluded that
> it was not really necessary to have such checks, for one it makes the
> code more simple, and because this is leveraged by the presence of
> --enable-tap-tests, tests which can get actually costly with
> check-world. But this is digressing the subject of this thread, which
> deals with the fact of having recovery tests integrated in core...

Of course, such configure tests should be run only if
--enable-tap-tests is passed to the configure script

It would look  like

if test "$enable_tap_tests" = "yes"; then
  AX_PROG_PERL_MODULES( Test::More, , AC_MSG_ERROR([Test::More is
  necessary to run TAP Tests])
  AX_PROG_PERL_MODULES( IPC::Run, , AC_MSG_ERROR([IPC::Run is
  necessary to run TAP Tests])
fi

in the configure.in

May be it is not strictly necessary, but it is really useful to see
such problems as clear error message during configure stage, rather
than successfully configure, compile, run tests and only then find out,
that something is forgotten.

I don't see why having such tests in the configure.in, makes code more
complex. It just prevents configure to finish successfully if
--enable-tap-tests is specified and required modules are not available.




-- 
   Victor Wagner <vi...@wagner.pp.ru>


-- 
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] [REVIEW] In-core regression tests for replication, cascading, archiving, PITR, etc.

[Victor Wagner]

This patch adds a long-awaited functionality to the PostgreSQL test
suite - testing of cluster configuration.

It contains bare minimum of replication and recovery test, but it should
be a good starting point for other people. 

Really, adding a much more tests for replication and recovery
is problematic, because these tests are resource-hungry, and take big
enough time even on relatively powerful machines, but  it seems to be
necessary, because they need to create several temporary installation.

So, set of tests, included into this patch is reasonably good choice. 

I think that readability of tests can be improved a bit, because these
tests would serve as an example for all tap test writers.

It's quite good that patch sets standard of using 'use strict; use
warnings;' in the test script.

It is bad, that Postgres-specific perl modules do not have embedded
documentation. It would be nice to see POD documentation in the
TestLib.pm and PostgresNode.pm instead of just comments. It would be
much easier to test writers to read documentation using perldoc utility,
rather than browse through the code.

I'll second Stas' suggestion about psql_ok/psql_fail functions.

1. psql_ok instead of just psql would provide visual feedback for the
reader of code. One would see 'here condition is tested, here is
something ended with _ok/_fail'.

It would be nice that seeing say "use Test::More tests => 4"
one can immediately see "Yes, there is three _ok's and one _fail in the
script'

2. I have use case for psql_fail code. In my libpq failover patch there
is number of cases, where it should be tested that connection is not
established,

But this is rather about further evolution of the tap test library, not
about this set of tests.

I think that this patch should be commited as soon as possible in its
current form (short of already reported reject in the PostgresNode.pm
init function).


-- 
       Victor Wagner <vi...@wagner.pp.ru>


-- 
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] Patch: Implement failover on libpq connect level.

[Victor Wagner]

On Fri, 22 Jan 2016 16:36:15 -0300
Alvaro Herrera <alvhe...@2ndquadrant.com> wrote:

> You're editing the expected file for the libpq-regress thingy, but you
> haven't added any new lines to test the new capability.  I think it'd
> be good to add some there.  (I already said this earlier in the
> thread; is there any reason you ignored it the first time?)

I seriously doubt that this program can be used to test new
capabilities.

All it does, it calls PQconninfoParse and than examines some fields of
PGconn structure.

The only reason I've to modify expected output is that I changed usage
of one of these field, and keep there comma-separated list of host:port
pairs instead of just hostname.

Thus contents this field after parsing of some existing URIs is
changed, while semantic of URIs is same.

If I add some new uris, than only thing I can test is that comma is
properly copied from the URI to this field. And may be that some syntax
errors are properly detected.

 
> If the test program requires improvement to handle the new stuff,
> let's do that.

The only improvement I can think of, is to examine list of the addrinfo
structures into which host list is eventually parsed. But it is quite
problematic, because it depends on many factors which are outside of
our control. 

It stores addresses resolved via OS's name resolver.

For example, if we specify 'localhost' there it can be parsed into one
or to records, depending on presence of IPv6 support.

If we use some other hostname here, we'll rely on internet connectivity
and DNS system. And we cannot ensure that any name to IP mapping will
persist for a long enough time. 

So, I think that new functionality need other approach for testing.
There should be test of real connection to real temporary cluster.
Probably, based on Perl TAP framework which is actively used in the
Postgres recently.
> 



-- 
       Victor Wagner <vi...@wagner.pp.ru>


-- 
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] Patch: Implement failover on libpq connect level.

[Victor Wagner]

On Sun, 24 Jan 2016 15:30:22 +
Thom Brown <t...@linux.com> wrote:

в
> Okay, I've tested this again with additional logging.  Again, I'm just
> running "psql -p 5531 postgres", which connects to a standby.  This
> immediately exits psql, and the logs show:

> 2016-01-24 15:04:59.886 GMT - thom - postgres LOG:  0: statement:
> SELECT pg_catalog.pg_is_in_recovery()
> This shouldn't be checking whether it's a standby.  I also noticed
> that with:

This is, of course, incompatibility with previous behavior. Probably,
I should modify this patch, so it would imply readonly flag if only one
host/port pair is specified in the command line.

Now it does check for standby regardless of number of hosts specified.



-- 
   Victor Wagner <vi...@wagner.pp.ru>


-- 
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] Patch: Implement failover on libpq connect level.

[Victor Wagner]

On Sun, 24 Jan 2016 15:58:10 +
Thom Brown <t...@linux.com> wrote:


> 
> Output of \set variables without patch:
> 
> HOST = '127.0.0.1'
> PORT =
> '5530,127.0.0.1:5531,127.0.0.1:5532,127.0.0.1:5533,127.0.0.1:5534,127.0.0.1:5535'
> 
> And with patch:
> 
> HOST =
> '127.0.0.1:5530,127.0.0.1:5531,127.0.0.1:5532,127.0.0.1:5533,127.0.0.1:5534,127.0.0.1:5535'
> PORT = '5488'
> 
> They're both wrong, but I'm hoping we can just show the right
> information here.

I think we should show right information here, but it is not so simple.

Problem is that I never keep symbolic representation of individual
host/port pair. And when we connect successfully, we have only struct
sockaddr representation of the it, which contain right IP
address, but doesn't contain symbolic host name.

Moreover, one hostname from connect string can produce more than one
addrinfo structures. For example, on the machines with IPv6 support,
'localhost' hostname is resolved into both 127.0.0.1 IPv4 address and
[::1] IPv6, and produces two records. 

So would do any name, which have both A and  records in DNS. And
nothing prevent domain administrator to put more than one A record for
same hostname into DNS zone.


So, it is just same information which can be retrieved from the backend
via 

select inet_client_addr();
select inet_client_port();

What is really interesting for HOST and PORT variable - it is the name
of host and port number used to make actual connection, as they were
specified in the connect string or service file.


> 
> Thom



-- 
   Victor Wagner <vi...@wagner.pp.ru>


-- 
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] Patch: Implement failover on libpq connect level.

[Victor Wagner]

On Tue, 19 Jan 2016 14:34:54 +
Thom Brown <t...@linux.com> wrote:

> 
> The segfault issue I originally reported now appears to be resolved.
> 
> But now I have another one:
> 
> psql
> 'postgresql://thom@127.0.0.1:5530,127.0.0.1:5531,127.0.0.1:5532,127.0.0.1:5533/postgres?hostorder=random=1_timeout=5'
> -c 'show port'

Here is new version of the patch. Now I've reworked hostorder=random and
it seems to work as well as sequential. failover_timeout works too.
I've also found a case when attempt to connect fail when receiving
FATAL message from server which is not properly up yet. So, it is fixed
too.

Addititonally, error messages from all failed connect attempts are not
accumulated now. Only last one is returned.





-- 
       Victor Wagner <vi...@wagner.pp.ru>
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 2328d8f..3c7bd15 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -792,7 +792,7 @@ host=localhost port=5432 dbname=mydb connect_timeout=10

The general form for a connection URI is:
 
-postgresql://[user[:password]@][netloc][:port][/dbname][?param1=value1...]
+postgresql://[user[:password]@][netloc][:port][,netloc[:port]...][/dbname][?param1=value1...]
 

 
@@ -809,6 +809,7 @@ postgresql://localhost/mydb
 postgresql://user@localhost
 postgresql://user:secret@localhost
 postgresql://other@localhost/otherdb?connect_timeout=10application_name=myapp
+postgresql://node1,node2:5433,node3:4432,node4/mydb?hostorder=randomreadonly=1
 
 Components of the hierarchical part of the URI can also
 be given as parameters.  For example:
@@ -831,7 +832,9 @@ postgresql:///mydb?host=localhostport=5433

 For improved compatibility with JDBC connection URIs,
 instances of parameter ssl=true are translated into
-sslmode=require.
+sslmode=require and
+loadBalanceHosts=true  into
+hostorder=random.

 

@@ -841,6 +844,10 @@ postgresql:///mydb?host=localhostport=5433
 postgresql://[2001:db8::1234]/database
 

+   
+   There can be serveral host specifications, optionally accompanied
+   with port, separated by comma.
+   
 

 The host component is interpreted as described for the parameter PostgreSQL was built). On machines without
 Unix-domain sockets, the default is to connect to localhost.

+   
+   There can be more than one host parameter in
+   the connect string. In this case these hosts would be considered
+   alternate entries into same database and if connect to first one
+   fails, library would try to connect second etc. This can be used
+   for high availability cluster or for load balancing. See
+parameter.
+   
+   
+   Network host name can be accompanied with port number, separated by
+   colon. If so, this port number is used only when connected to
+   this host. If there is no port number, port specified in the
+parameter would be used.
+   
   
  
 
@@ -942,8 +963,44 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname


   
-
+
+  
+  hostorder
+  
+  
+  Specifies how to choose host from list of alternate hosts,
+  specified in the  parameter.
+  
+  
+  If value of this argument is sequential (the
+  default) library connects to the hosts in order they specified,
+  and tries to connect second one only if connection to the first
+  fails.
+  
+  
+  If value is random host to connect is randomly
+  picked from the list. It allows to balance load between several
+  cluster nodes. However, currently PostgreSQL doesn't support
+  multimaster clusters. So, without use of third-party products,
+  only read-only connections can take advantage from the
+  load-balancing. See 
+  
+  
+  
+  
+  readonly
+  
+  
+  If this parameter is 0 (the default), upon successful connection
+  library checks if host is in recovery state, and if it is so,
+  tries next host in the connect string. If this parameter is
+  non-zero, connection to warm standby nodes are considered
+  successful.
+  
+  
+  
   
+
port


@@ -985,7 +1042,6 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
   
   
  
-
  
   connect_timeout
   
@@ -996,7 +1052,27 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
   
   
  
-
+ 
+ falover_timeout
+ 
+ 
+ Maximum time to cycilically retry all the hosts in commect string.
+ (as decimal integer number of seconds). If not specified, then
+ hosts are tried just once.
+ 
+ 
+ If we have replicating cluster, and master node fails, it might
+ take some time to promote one of standby nodes to the new master.
+ So clients which notice that connect to the master fails, can
+ al

Re: [HACKERS] Patch: Implement failover on libpq connect level.

[Victor Wagner]

On Tue, 19 Jan 2016 14:34:54 +
Thom Brown  wrote:

> The segfault issue I originally reported now appears to be resolved.
> 
> But now I have another one:
> 
> psql
> 'postgresql://thom@127.0.0.1:5530,127.0.0.1:5531,127.0.0.1:5532,127.0.0.1:5533/postgres?hostorder=random=1_timeout=5'
> -c 'show port'
> 
> Segmentation fault

This segfault appears to be a simple programming error. 
which comes out only when  we need to retry address list (which happens
when no node is available at the time of first scan)

There is code which prevents random hostorder to connect same node
twice a row

while (current != NULL)
{
if (current == conn->addr_cur)
{
current = current->ai_next;
}
count++;
if ((rand()&0x) < 0x1 / count)
{
choice = current;
}
current = current->ai_next;
}


There should be continue;  after first current = current->ai_next;
 
> This is where no nodes are available.  If I remove hostorder=random,
> or replace it with hostorder=sequential, it doesn't segfault.
> However, it then tries to connect to PGHOST on PGPORT repeatedly, even
> if I bring up one of the nodes it should be looking for.  Not only
> this, but it seems to do it forever if failover_timeout is greater
> than 0.

But this turns out to be more complicated matter, so I'm not ready to
provide next version of patch yet.







-- 
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] Patch: Implement failover on libpq connect level.

[Victor Wagner]

On Mon, 21 Dec 2015 17:18:37 +0300
Teodor Sigaev  wrote:

> Sorry, but there is something wrong with your patch:
> % patch -p1 -C < ~/Downloads/libpq-failover-5.patch

Really, somehow broken version of the patch got into message.

Here is correct one.

diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 9c0e4c8..162bd4f 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -792,7 +792,7 @@ host=localhost port=5432 dbname=mydb connect_timeout=10

The general form for a connection URI is:
 
-postgresql://[user[:password]@][netloc][:port][/dbname][?param1=value1...]
+postgresql://[user[:password]@][netloc][:port][,netloc[:port]...][/dbname][?param1=value1...]
 

 
@@ -809,6 +809,7 @@ postgresql://localhost/mydb
 postgresql://user@localhost
 postgresql://user:secret@localhost
 postgresql://other@localhost/otherdb?connect_timeout=10application_name=myapp
+postgresql://node1,node2:5433,node3:4432,node4/mydb?hostorder=randomreadonly=1
 
 Components of the hierarchical part of the URI can also
 be given as parameters.  For example:
@@ -831,7 +832,9 @@ postgresql:///mydb?host=localhostport=5433

 For improved compatibility with JDBC connection URIs,
 instances of parameter ssl=true are translated into
-sslmode=require.
+sslmode=require and
+loadBalanceHosts=true  into
+hostorder=random.

 

@@ -841,6 +844,10 @@ postgresql:///mydb?host=localhostport=5433
 postgresql://[2001:db8::1234]/database
 

+   
+   There can be serveral host specifications, optionally accompanied
+   with port, separated by comma.
+   
 

 The host component is interpreted as described for the parameter PostgreSQL was built). On machines without
 Unix-domain sockets, the default is to connect to localhost.

+   
+   There can be more than one host parameter in
+   the connect string. In this case these hosts would be considered
+   alternate entries into same database and if connect to first one
+   fails, library would try to connect second etc. This can be used
+   for high availability cluster or for load balancing. See
+parameter.
+   
+   
+   Network host name can be accompanied with port number, separated by
+   colon. If so, this port number is used only when connected to
+   this host. If there is no port number, port specified in the
+parameter would be used.
+   
   
  
 
@@ -942,8 +963,44 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname


   
-
+
+  
+  hostorder
+  
+  
+  Specifies how to choose host from list of alternate hosts,
+  specified in the  parameter.
+  
+  
+  If value of this argument is sequential (the
+  default) library connects to the hosts in order they specified,
+  and tries to connect second one only if connection to the first
+  fails.
+  
+  
+  If value is random host to connect is randomly
+  picked from the list. It allows to balance load between several
+  cluster nodes. However, currently PostgreSQL doesn't support
+  multimaster clusters. So, without use of third-party products,
+  only read-only connections can take advantage from the
+  load-balancing. See 
+  
+  
+  
+  
+  readonly
+  
+  
+  If this parameter is 0 (the default), upon successful connection
+  library checks if host is in recovery state, and if it is so,
+  tries next host in the connect string. If this parameter is
+  non-zero, connection to warm standby nodes are considered
+  successful.
+  
+  
+  
   
+
port


@@ -985,7 +1042,6 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
   
   
  
-
  
   connect_timeout
   
@@ -996,7 +1052,27 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
   
   
  
-
+ 
+ falover_timeout
+ 
+ 
+ Maximum time to cycilically retry all the hosts in commect string.
+ (as decimal integer number of seconds). If not specified, then
+ hosts are tried just once.
+ 
+ 
+ If we have replicating cluster, and master node fails, it might
+ take some time to promote one of standby nodes to the new master.
+ So clients which notice that connect to the master fails, can
+ already give up attempt to reestablish a connection when new master
+ became available. 
+ 
+ 
+ Setting this parameter to reasonable time makes library try to
+ reconnect all the host in cyclically until new master appears.
+ 
+ 
+ 
  
   client_encoding
   
@@ -7165,6 +7241,18 @@ user=admin
An example file is provided at
share/pg_service.conf.sample.
   
+  
+  If more than one host option present in the section of service file, it
+  is interpeted as alternate servers for failover or load-balancing. See
+   option 

[HACKERS] Is postgresql on Windows compatible with flex 2.6.0?

[Victor Wagner]

Collegues,

Recently (about a month ago) flex 2.6.0 have been released.

But when we tried to compile PostgreSQL 9.5 beta 1 from git with it on
Windows, we've encountered that src/tools/msvc/pgflex.pl explicitely 
checks that minor version of flex is equal to 5.

unless ($verparts[0] == 2 && $verparts[1] == 5 && $verparts[2]
>=31) { print "WARNING! Flex install not found, or unsupported Flex
   version.\n"; print "echo Attempting to build without.\n";
   exit 0;
}

Is this check intentional, or it just seats here from the time when
version 2.6.0 didn't exist?

Postgres seems to compile fine with flex 2.6.0.

--

Victor Wagner


-- 
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] Is postgresql on Windows compatible with flex 2.6.0?

[Victor Wagner]

On Thu, 10 Dec 2015 21:19:46 +0900
Michael Paquier <michael.paqu...@gmail.com> wrote:

> On Thu, Dec 10, 2015 at 5:45 PM, Victor Wagner <vi...@wagner.pp.ru>

> > Postgres seems to compile fine with flex 2.6.0.  
> 
> It seems that 32f15d05 missed to update its equivalent in
> src/tools/msvc. Per se the patch attached as you already found out.

May be it is better to use Perl standard module version to compare
version number.

something like:

use version;

unless (version->parse($flexver) > version->parse("2.5.31"))

Postgres requires relatively recent Perl.  Version objects appeared in
Perl 5.10 and most people would have something newer. 

-- 
 Victor Wagner 


-- 
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] Patch: Implement failover on libpq connect level.

[Victor Wagner]

On Mon, 07 Dec 2015 15:26:33 -0500
Korry Douglas <korry.doug...@enterprisedb.com> wrote:


> The problem seems to be in PQconnectPoll() in the case for 
> CONNECTION_AUTH_OK, specifically this code:
> 
>/* We can release the address list now. */
>pg_freeaddrinfo_all(conn->addrlist_family, conn->addrlist);
>conn->addrlist = NULL;
>conn->addr_cur = NULL;
> That frees up the list of alternative host addresses.  The state
> machine then progresses to CONNECTION_CHECK_RO (which invokes 
> pg_is_in_recovery()), then CONNECTION_CHECK_RW (waiting for the

Thank you for pointing to this problem. I've overlooked it. Probably
I should improve my testing scenario.

I', attaching new version of the patch, which, hopefully, handles
address list freeing correctly.




-- 
   Victor Wagner <vi...@wagner.pp.ru>
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 9c0e4c8..162bd4f 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -792,7 +792,7 @@ host=localhost port=5432 dbname=mydb connect_timeout=10

The general form for a connection URI is:
 
-postgresql://[user[:password]@][netloc][:port][/dbname][?param1=value1...]
+postgresql://[user[:password]@][netloc][:port][,netloc[:port]...][/dbname][?param1=value1...]
 

 
@@ -809,6 +809,7 @@ postgresql://localhost/mydb
 postgresql://user@localhost
 postgresql://user:secret@localhost
 postgresql://other@localhost/otherdb?connect_timeout=10application_name=myapp
+postgresql://node1,node2:5433,node3:4432,node4/mydb?hostorder=randomreadonly=1
 
 Components of the hierarchical part of the URI can also
 be given as parameters.  For example:
@@ -831,7 +832,9 @@ postgresql:///mydb?host=localhostport=5433

 For improved compatibility with JDBC connection URIs,
 instances of parameter ssl=true are translated into
-sslmode=require.
+sslmode=require and
+loadBalanceHosts=true  into
+hostorder=random.

 

@@ -841,6 +844,10 @@ postgresql:///mydb?host=localhostport=5433
 postgresql://[2001:db8::1234]/database
 

+   
+   There can be serveral host specifications, optionally accompanied
+   with port, separated by comma.
+   
 

 The host component is interpreted as described for the parameter PostgreSQL was built). On machines without
 Unix-domain sockets, the default is to connect to localhost.

+   
+   There can be more than one host parameter in
+   the connect string. In this case these hosts would be considered
+   alternate entries into same database and if connect to first one
+   fails, library would try to connect second etc. This can be used
+   for high availability cluster or for load balancing. See
+parameter.
+   
+   
+   Network host name can be accompanied with port number, separated by
+   colon. If so, this port number is used only when connected to
+   this host. If there is no port number, port specified in the
+parameter would be used.
+   
   
  
 
@@ -942,8 +963,44 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname


   
-
+
+  
+  hostorder
+  
+  
+  Specifies how to choose host from list of alternate hosts,
+  specified in the  parameter.
+  
+  
+  If value of this argument is sequential (the
+  default) library connects to the hosts in order they specified,
+  and tries to connect second one only if connection to the first
+  fails.
+  
+  
+  If value is random host to connect is randomly
+  picked from the list. It allows to balance load between several
+  cluster nodes. However, currently PostgreSQL doesn't support
+  multimaster clusters. So, without use of third-party products,
+  only read-only connections can take advantage from the
+  load-balancing. See 
+  
+  
+  
+  
+  readonly
+  
+  
+  If this parameter is 0 (the default), upon successful connection
+  library checks if host is in recovery state, and if it is so,
+  tries next host in the connect string. If this parameter is
+  non-zero, connection to warm standby nodes are considered
+  successful.
+  
+  
+  
   
+
port


@@ -985,7 +1042,6 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
   
   
  
-
  
   connect_timeout
   
@@ -996,7 +1052,27 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
   
   
  
-
+ 
+ falover_timeout
+ 
+ 
+ Maximum time to cycilically retry all the hosts in commect string.
+ (as decimal integer number of seconds). If not specified, then
+ hosts are tried just once.
+ 
+ 
+ If we have replicating cluster, and master node fails, it might
+ take some time to promote one of standby nodes to the new master.
+ So

[HACKERS] What .gitignore files do in the tarball?

[Victor Wagner]

Collegues,

I've noticed that source distribution archive of the postgresql contain
more than hundred of .gitignore files and one .gitattributes.

Is it just a bug nobody bothered to fix, or these files can make
any sense outside git repository?

Fix of the problem is quite trivial:

diff --git a/GNUmakefile.in b/GNUmakefile.in
index 15fba9f..beef51a 100644
--- a/GNUmakefile.in
+++ b/GNUmakefile.in
@@ -93,7 +93,7 @@ distdir-location:
 
 distdir:
rm -rf $(distdir)* $(dummy)
-   for x in `cd $(top_srcdir) && find . \( -name CVS -prune \) -o
\( -name .git -prune \) -o -print`; do \
+   for x in `cd $(top_srcdir) && find . \( -name CVS -prune \) -o
\( -name .git -prune \) -o \( \! -name ".git*" -print \)`; do \
file=`expr X$$x : 'X\./\(.*\)'`; \ if test -d "$(top_srcdir)/$$file" ;
then \ mkdir "$(distdir)/$$file" && chmod 777 "$(distdir)/$$file"; \
-- 
      Victor Wagner



-- 
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] Patch (3): Implement failover on libpq connect level.

[Victor Wagner]

On Tue, 17 Nov 2015 19:42:42 +
Thom Brown  wrote:

> 
> This patch doesn't apply.  On line 636, this appears:


It doesn't apply to current state of the master branch due
to changes in fe-connect.c made by Tom Lane in the commit c40591885.
Unfortunately these changes appears in the source just line by line
with my changes. I'm attaching here updated version of the patch,
applicable to master later that 11-12-2015.


diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 0ee018e..ec12fce 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -792,7 +792,7 @@ host=localhost port=5432 dbname=mydb connect_timeout=10

The general form for a connection URI is:
 
-postgresql://[user[:password]@][netloc][:port][/dbname][?param1=value1...]
+postgresql://[user[:password]@][netloc][:port][,netloc[:port]...][/dbname][?param1=value1...]
 

 
@@ -809,6 +809,7 @@ postgresql://localhost/mydb
 postgresql://user@localhost
 postgresql://user:secret@localhost
 postgresql://other@localhost/otherdb?connect_timeout=10application_name=myapp
+postgresql://node1,node2:5433,node3:4432,node4/mydb?hostorder=randomreadonly=1
 
 Components of the hierarchical part of the URI can also
 be given as parameters.  For example:
@@ -831,7 +832,9 @@ postgresql:///mydb?host=localhostport=5433

 For improved compatibility with JDBC connection URIs,
 instances of parameter ssl=true are translated into
-sslmode=require.
+sslmode=require and
+loadBalanceHosts=true  into
+hostorder=random.

 

@@ -841,6 +844,10 @@ postgresql:///mydb?host=localhostport=5433
 postgresql://[2001:db8::1234]/database
 

+   
+   There can be serveral host specifications, optionally accompanied
+   with port, separated by comma.
+   
 

 The host component is interpreted as described for the parameter PostgreSQL was built). On machines without
 Unix-domain sockets, the default is to connect to localhost.

+   
+   There can be more than one host parameter in
+   the connect string. In this case these hosts would be considered
+   alternate entries into same database and if connect to first one
+   fails, library would try to connect second etc. This can be used
+   for high availability cluster or for load balancing. See
+parameter.
+   
+   
+   Network host name can be accompanied with port number, separated by
+   colon. If so, this port number is used only when connected to
+   this host. If there is no port number, port specified in the
+parameter would be used.
+   
   
  
 
@@ -942,8 +963,44 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname


   
-
+
+  
+  hostorder
+  
+  
+  Specifies how to choose host from list of alternate hosts,
+  specified in the  parameter.
+  
+  
+  If value of this argument is sequential (the
+  default) library connects to the hosts in order they specified,
+  and tries to connect second one only if connection to the first
+  fails.
+  
+  
+  If value is random host to connect is randomly
+  picked from the list. It allows to balance load between several
+  cluster nodes. However, currently PostgreSQL doesn't support
+  multimaster clusters. So, without use of third-party products,
+  only read-only connections can take advantage from the
+  load-balancing. See 
+  
+  
+  
+  
+  readonly
+  
+  
+  If this parameter is 0 (the default), upon successful connection
+  library checks if host is in recovery state, and if it is so,
+  tries next host in the connect string. If this parameter is
+  non-zero, connection to warm standby nodes are considered
+  successful.
+  
+  
+  
   
+
port


@@ -985,7 +1042,6 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
   
   
  
-
  
   connect_timeout
   
@@ -996,7 +1052,27 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
   
   
  
-
+ 
+ falover_timeout
+ 
+ 
+ Maximum time to cycilically retry all the hosts in commect string.
+ (as decimal integer number of seconds). If not specified, then
+ hosts are tried just once.
+ 
+ 
+ If we have replicating cluster, and master node fails, it might
+ take some time to promote one of standby nodes to the new master.
+ So clients which notice that connect to the master fails, can
+ already give up attempt to reestablish a connection when new master
+ became available. 
+ 
+ 
+ Setting this parameter to reasonable time makes library try to
+ reconnect all the host in cyclically until new master appears.
+ 
+ 
+ 
  
   client_encoding
   
@@ -7161,6 +7237,18 @@ user=admin
An example file is provided at
share/pg_service.conf.sample.

Re: [HACKERS] Patch: Implement failover on libpq connect level.

[Victor Wagner]

On Mon, 9 Nov 2015 15:14:02 +0800
Craig Ringer  wrote:


> 
> I'd rather not see convoluted and complex connstrings, per my prior
> post. The JDBC extended URL style seems good enough.

It is what my patch implements.
 
> IMO the biggest problem is that client-side failover is way more
> complex than "got -ECONNREFUSED, try next host". I think we're all
> focusing on not being able to twiddle arbitrary settings while
> ignoring the real problem with client failover, i.e. making it
> actually work in the real world.
> 
I've tried to deal with some of these problems. 

My patch have support for following things:

1. Check whether database instance is in the recovery/standby mode and
try to find another one if so.
2. Let cluster management software to have some time to promote one of
the standbys to master. I.e. there can be failover timeout specified to
allow retry after some time if no working master found.

Really there is room for some improvements in handling of connect
timeout (which became much more important thing when ability to try
next host appears). Now it is handled only by blocking-mode connect
functions, not by async state machine. But I decided to publish patch
without these improvements to get feedback from community.




-- 
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] Patch: Implement failover on libpq connect level.

[Victor Wagner]

On 2015.11.02 at 12:15:48 -0500, Peter Eisentraut wrote:

>> That's true, but doesn't allowing every parameter to be multiply
>> specified greatly increase the implementation complexity for a pretty
>> marginal benefit?  

>Well, the way I would have approached is that after all the parsing you
>end up with a list of PQconninfoOption arrays and you you loop over that
>list and call connectDBStart() until you have a connection that
>satisfies you.

>I see that the patch doesn't do that and it looks quite messy as a
>result.

There are some drawbacks with this approach

1. You are suggesting usage of two nested loops, instead of one straight
loop - outer loop over the URLs in the connect string, inner loop over
the IP addresses each URL resolves into. (This inner loop already
presents there for several versions of the postgres).

2. You are suggesting that it should be possible to specify all options 
separately for each of the fallback hosts. 

But some options control how
next host should be choosen (i.e. use random order for load-balancing
or sequential order for high availability), so they should be specified
only once per connect string.

Some other options are
connection level rather then node-level (i.e. if you are using
replication mode of the connection, there is no sense to specify it for
one host and not specify for other). These options should be better
specified such way, that it would be not possible to use conflicting
values.

Third category of options are specified per-cluster much more often
than per node. But are quite often changed from compiled in default.
Typically there are authentication credentials (even you use
trigger-based replication, user information would probably be
replicated), database name (if you use WAL-level replication), client
encoding (which depends on more on the client than on server), etc.

It would be pain if we would need specify them for each host separately.
Syntax, implemented in the my patch allows even to specify
nonstandard-port once for all hosts (using port= parameter in the query
string) and override it for particular host in the list using colon
syntax.

3. Last, but not least. There is more than one postgresql client
implementation. Apart of libpq we have jdbc, native GO language client
etc. And I think that maintaining compatibility of the connect string
between them is good thing. So, I've modelled syntax of multihost
feature in the libpq URL-style syntax after jdbc syntax.
-- 
       Victor Wagner <vi...@wagner.pp.ru>


-- 
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] Patch: Implement failover on libpq connect level.

[Victor Wagner]

On Fri, 30 Oct 2015 14:26:45 +0100
Robert Haas  wrote:

> On Thu, Oct 29, 2015 at 8:29 PM, Peter Eisentraut 
> wrote:

> 
> That's true, but doesn't allowing every parameter to be multiply
> specified greatly increase the implementation complexity for a pretty
> marginal benefit?  I think host and IP would hit 98% of the use cases
> here.

As far as I can tell from the experience of writing this patch, it
would greatly increase complexity.

If there should be only need to have multiple hosts, I could almost
completely incapsulate changes into DNS resolving code (which already
allows to handle several addresses). Need to support different port for
each host already required change of internal storage, and as a
consequence changes in the regression test suite
(src/interfaces/libpq/test/regress.out)

But both host and port are used in the same place - in the connect
system call. If we add possibility to different values per host for some
parameter, such as database name, which should be used significantly
later, i.e. during sending of first protocol message, size of patch
would grow may be twice.



-- 
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] Patch: Implement failover on libpq connect level.

[Victor Wagner]

On Mon, 26 Oct 2015 16:25:57 -0400
Peter Eisentraut  wrote:

> Also, this assumes that all the components other than host and port
> are the same.  Earlier there was a discussion about why the ports
> would ever need to be different.  Well, why can't the database names
> be different? I could have use for that.

Because of way postgresql replication is implemented.

This multihost feature is for clusters. Either for automatic switch
from one cluster node to another when master node fails and one of the
standbys get promoted to master, or for load balancing between hot
standby nodes.

Postgresql allows different replicas listen on different ports, because
each replica has its own postgresql conf, but repication works on the
node level, not on the database level, so all the databases from master
node would be replicated with their original names.



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

[HACKERS] Patch (3): Implement failover on libpq connect level.

[Victor Wagner]

On 2015.10.14 at 13:41:51 +0300, Victor Wagner wrote:

> Attached patch which implements client library failover and
> loadbalancing as was described in the proposal
> <20150818041850.ga5...@wagner.pp.ru>.

New version of patch

1. Handles replication connections correctly (i.e doesn't attempt to
check readwrite mode if replication option is on)
2. Some minor improvement recommended by Korry Douglas  in
<562a9259.4060...@enterprisedb.com>

-- 
       Victor Wagner <vi...@wagner.pp.ru>
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 0ee018e..ec12fce 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -792,7 +792,7 @@ host=localhost port=5432 dbname=mydb connect_timeout=10

The general form for a connection URI is:
 
-postgresql://[user[:password]@][netloc][:port][/dbname][?param1=value1...]
+postgresql://[user[:password]@][netloc][:port][,netloc[:port]...][/dbname][?param1=value1...]
 

 
@@ -809,6 +809,7 @@ postgresql://localhost/mydb
 postgresql://user@localhost
 postgresql://user:secret@localhost
 postgresql://other@localhost/otherdb?connect_timeout=10application_name=myapp
+postgresql://node1,node2:5433,node3:4432,node4/mydb?hostorder=randomreadonly=1
 
 Components of the hierarchical part of the URI can also
 be given as parameters.  For example:
@@ -831,7 +832,9 @@ postgresql:///mydb?host=localhostport=5433

 For improved compatibility with JDBC connection URIs,
 instances of parameter ssl=true are translated into
-sslmode=require.
+sslmode=require and
+loadBalanceHosts=true  into
+hostorder=random.

 

@@ -841,6 +844,10 @@ postgresql:///mydb?host=localhostport=5433
 postgresql://[2001:db8::1234]/database
 

+   
+   There can be serveral host specifications, optionally accompanied
+   with port, separated by comma.
+   
 

 The host component is interpreted as described for the parameter PostgreSQL was built). On machines without
 Unix-domain sockets, the default is to connect to localhost.

+   
+   There can be more than one host parameter in
+   the connect string. In this case these hosts would be considered
+   alternate entries into same database and if connect to first one
+   fails, library would try to connect second etc. This can be used
+   for high availability cluster or for load balancing. See
+parameter.
+   
+   
+   Network host name can be accompanied with port number, separated by
+   colon. If so, this port number is used only when connected to
+   this host. If there is no port number, port specified in the
+parameter would be used.
+   
   
  
 
@@ -942,8 +963,44 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname


   
-
+
+  
+  hostorder
+  
+  
+  Specifies how to choose host from list of alternate hosts,
+  specified in the  parameter.
+  
+  
+  If value of this argument is sequential (the
+  default) library connects to the hosts in order they specified,
+  and tries to connect second one only if connection to the first
+  fails.
+  
+  
+  If value is random host to connect is randomly
+  picked from the list. It allows to balance load between several
+  cluster nodes. However, currently PostgreSQL doesn't support
+  multimaster clusters. So, without use of third-party products,
+  only read-only connections can take advantage from the
+  load-balancing. See 
+  
+  
+  
+  
+  readonly
+  
+  
+  If this parameter is 0 (the default), upon successful connection
+  library checks if host is in recovery state, and if it is so,
+  tries next host in the connect string. If this parameter is
+  non-zero, connection to warm standby nodes are considered
+  successful.
+  
+  
+  
   
+
port


@@ -985,7 +1042,6 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
   
   
  
-
  
   connect_timeout
   
@@ -996,7 +1052,27 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
   
   
  
-
+ 
+ falover_timeout
+ 
+ 
+ Maximum time to cycilically retry all the hosts in commect string.
+ (as decimal integer number of seconds). If not specified, then
+ hosts are tried just once.
+ 
+ 
+ If we have replicating cluster, and master node fails, it might
+ take some time to promote one of standby nodes to the new master.
+ So clients which notice that connect to the master fails, can
+ already give up attempt to reestablish a connection when new master
+ became available. 
+ 
+ 
+ Setting this parameter to reasonable time makes library try to
+ reconnect all the host in cyclically until new master appears.
+ 
+ 
+ 
  
   client_encoding

Re: [HACKERS] Patch (2): Implement failover on libpq connect level.

[Victor Wagner]

В Fri, 23 Oct 2015 22:14:56 +0100
Thom Brown <t...@linux.com> пишет:

c> >
> > pg_basebackup -v -x -D standby1 \
> >   -d "host=localhost port=5532 user=rep_user readonly=1"
> 
> Yes, this works:
> 
> $ pg_basebackup -v -x -D standby1 -d "host=localhost port=5532
> user=rep_user readonly=1"
> transaction log start point: 0/228 on timeline 1
> transaction log end point: 0/2000130
> pg_basebackup: base backup completed
> 
> Thom

Thanks for your help.

I was unable to reproduce segfault with this situation on Linux x86-64..

I got message:

row number 0 is out of range 0..-1

and connection failed. Can you tell me a bit more of your setup.
At least which architecture you are compiling Postgres for.


-- 
   Victor Wagner <vi...@wagner.pp.ru>


-- 
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] Patch (2): Implement failover on libpq connect level.

[Victor Wagner]

В Fri, 23 Oct 2015 16:02:33 -0400
Korry Douglas <korry.doug...@enterprisedb.com> пишет:

d> > Now support for service files is implemented and multiple host
> > statements in the service file are allowed.
> 
> A couple of minor nits:
> 
> When you call pg_is_in_recovery(), you should schema-qualify the 
> function name, just in case some other version of that function
> exists in the search_path.
> 
> Also, pg_is_in_recovery() returns a boolean value - PQgetvalue() will 
> not return "true" or "false", it will return "t" or "f".
> 
> And, you have a bit of garbage in the patch (the patch inserts 
> UNIXSOCK_PATH(portstr, portnum, conn->pgunixsocket); in the header 
> comment at the top of fe-connect.c).
> 

Thanks, I'd address this issues in the next version of path.
 



-- 
   Victor Wagner <vi...@wagner.pp.ru>


-- 
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] Patch (2): Implement failover on libpq connect level.

[Victor Wagner]

On Thu, 22 Oct 2015 14:33:11 +0100
Thom Brown <t...@linux.com> wrote:

> On 21 October 2015 at 10:07, Victor Wagner <vi...@wagner.pp.ru> wrote:
> > On 2015.10.14 at 13:41:51 +0300, Victor Wagner wrote:
> >
> >>
> >> Attached patch which implements client library failover and
> >> loadbalancing as was described in the proposal
> >> <20150818041850.ga5...@wagner.pp.ru>.
> >>
> >
> > I'm sending imporoved verison of patch. As Olexander Shulgin noted,
> > previous version of patch lacks support for service files.
> >
> > Now support for service files is implemented and multiple host
> > statements in the service file are allowed.
> 
> This is causing breakage:
> 
> $ pg_basebackup -v -x -D standby1 -h localhost -p 5532 -U rep_user
> row number 0 is out of range 0..-1

It seems that pg_basebackup should always specify readonly attribute
for the connection. 

Your data directory is named standby1, so I suspect
that you are trying to make backup from read-only standby instance of
the base. 

Can you check if problem persists in your setup with command

pg_basebackup -v -x -D standby1 \
  -d "host=localhost port=5532 user=rep_user readonly=1"


> Segmentation fault

Of course, this shouldn't happen even if libpq is severely misused.
And it is almost evident what should be done to fix it.
But making sure that my patch doesn't interfere with operation of
pg_basebackup is a bit more complicated.




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

[HACKERS] Patch (2): Implement failover on libpq connect level.

[Victor Wagner]

On 2015.10.14 at 13:41:51 +0300, Victor Wagner wrote:

> 
> Attached patch which implements client library failover and
> loadbalancing as was described in the proposal
> <20150818041850.ga5...@wagner.pp.ru>.
> 

I'm sending imporoved verison of patch. As Olexander Shulgin noted,
previous version of patch lacks support for service files.

Now support for service files is implemented and multiple host
statements in the service file are allowed.
-- 
       Victor Wagner <vi...@wagner.pp.ru>
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 0ee018e..ec12fce 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -792,7 +792,7 @@ host=localhost port=5432 dbname=mydb connect_timeout=10

The general form for a connection URI is:
 
-postgresql://[user[:password]@][netloc][:port][/dbname][?param1=value1...]
+postgresql://[user[:password]@][netloc][:port][,netloc[:port]...][/dbname][?param1=value1...]
 

 
@@ -809,6 +809,7 @@ postgresql://localhost/mydb
 postgresql://user@localhost
 postgresql://user:secret@localhost
 postgresql://other@localhost/otherdb?connect_timeout=10application_name=myapp
+postgresql://node1,node2:5433,node3:4432,node4/mydb?hostorder=randomreadonly=1
 
 Components of the hierarchical part of the URI can also
 be given as parameters.  For example:
@@ -831,7 +832,9 @@ postgresql:///mydb?host=localhostport=5433

 For improved compatibility with JDBC connection URIs,
 instances of parameter ssl=true are translated into
-sslmode=require.
+sslmode=require and
+loadBalanceHosts=true  into
+hostorder=random.

 

@@ -841,6 +844,10 @@ postgresql:///mydb?host=localhostport=5433
 postgresql://[2001:db8::1234]/database
 

+   
+   There can be serveral host specifications, optionally accompanied
+   with port, separated by comma.
+   
 

 The host component is interpreted as described for the parameter PostgreSQL was built). On machines without
 Unix-domain sockets, the default is to connect to localhost.

+   
+   There can be more than one host parameter in
+   the connect string. In this case these hosts would be considered
+   alternate entries into same database and if connect to first one
+   fails, library would try to connect second etc. This can be used
+   for high availability cluster or for load balancing. See
+parameter.
+   
+   
+   Network host name can be accompanied with port number, separated by
+   colon. If so, this port number is used only when connected to
+   this host. If there is no port number, port specified in the
+parameter would be used.
+   
   
  
 
@@ -942,8 +963,44 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname


   
-
+
+  
+  hostorder
+  
+  
+  Specifies how to choose host from list of alternate hosts,
+  specified in the  parameter.
+  
+  
+  If value of this argument is sequential (the
+  default) library connects to the hosts in order they specified,
+  and tries to connect second one only if connection to the first
+  fails.
+  
+  
+  If value is random host to connect is randomly
+  picked from the list. It allows to balance load between several
+  cluster nodes. However, currently PostgreSQL doesn't support
+  multimaster clusters. So, without use of third-party products,
+  only read-only connections can take advantage from the
+  load-balancing. See 
+  
+  
+  
+  
+  readonly
+  
+  
+  If this parameter is 0 (the default), upon successful connection
+  library checks if host is in recovery state, and if it is so,
+  tries next host in the connect string. If this parameter is
+  non-zero, connection to warm standby nodes are considered
+  successful.
+  
+  
+  
   
+
port


@@ -985,7 +1042,6 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
   
   
  
-
  
   connect_timeout
   
@@ -996,7 +1052,27 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
   
   
  
-
+ 
+ falover_timeout
+ 
+ 
+ Maximum time to cycilically retry all the hosts in commect string.
+ (as decimal integer number of seconds). If not specified, then
+ hosts are tried just once.
+ 
+ 
+ If we have replicating cluster, and master node fails, it might
+ take some time to promote one of standby nodes to the new master.
+ So clients which notice that connect to the master fails, can
+ already give up attempt to reestablish a connection when new master
+ became available. 
+ 
+ 
+ Setting this parameter to reasonable time makes library try to
+ reconnect all the host in cyclically until new master appears.
+ 
+ 
+ 
  
   client_encoding

Re: [HACKERS] Patch: Implement failover on libpq connect level.

[Victor Wagner]

On 2015.10.14 at 14:47:46 +0200, Shulgin, Oleksandr wrote:

> 
> So what exactly would happen with this command: PGHOST=host1 psql -h host2
> 
> Or this one: PGHOST=host1 psql host=host2

The right thing - value of the PGHOST environment variable is ignored,
and explicitely specified host is used.

> What about service files?

Oops, I've missed something here. Multiple host lines in the service
file do not produce list of alternate hosts.

I'll address this case in the next version of patch.

However, comma-separated list of host in the one host string in the
service file works.


> 
>  trying postgresql://[::1]:12345/db
> -dbname='db' host='::1' port='12345' (inet)
> +dbname='db' host='[::1]:12345' (inet)
> 
> Such a change doesn't look really nice to me.

Really, any alternative would be worse. Only alternative I can imagine
is to replace pghost field in the PGConn structure by array or linked
list of structures with host and port fields.

Now I decided to keep such array local to connectDBStart function, and
store list of host-port pair as string. 

It takes some CPU cycles to reparse this list each time when
reconnection is needed, but this is negligible compared with DNS
resolution of each host name. And we cannot avoid repeated DNS
resolution, because we have to take into account possibility of
DNS-based failover.


> > 2. Url with colon  but no port number after the host no more considered
> > valid.
> >
> 
> We could live with that, but is there a good reason for backward
> compatibility break in this case?

Because we can. Really, I don't see any useful semantic of host with
colon at the end. Null host name with colon and port have useful
semantic (if there is just one host in the  connect string), so it kept
as it was.

In the old version of connection-string parsing code stripping lone
colon of the host name was done by generic code (because port number
have gone to another variable anyway). Now handling of lone
colon is special case, because we just check syntax of host-port pair
list. 

And I have to choose how to handle it - either complain or silently
strip it away. Complaining was similier. We compute port number and
check if it is between 1 and 65535. Zero-length port number is not
integer between 1 and 65535.


> --
> Alex

-- 
   Victor Wagner <vi...@wagner.pp.ru>


-- 
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] Patch: Implement failover on libpq connect level.

[Victor Wagner]

On 2015.10.15 at 17:13:46 +0800, Craig Ringer wrote:

> On 14 October 2015 at 18:41, Victor Wagner <vi...@wagner.pp.ru> wrote:
> 
> > 5. Added new parameter readonly=boolean. If this parameter is false (the
> > default) upon successful connection check is performed that backend is
> > not in the recovery state. If so, connection is not considered usable
> > and next host is tried.
> 
> What constitutes "failed" as far as this is concerned?

failed means "select pg_is_in_recovery() returns true". 
The only reason of adding this parameter is to state that we 
can use  connection to warm-backup slave (or if we cannot).

> Like the PgJDBC approach I wonder how much this'll handle in practice
> and how it'll go with less clear-cut failures like disk-full on a
> replica that's a member of the failover pool, long delays before
> no-route-to-host errors, dns problems, etc.

Really I don't think that disk-full and other such node errors are
problems of client library. It is cluster management software which
should check for these errors and shut down erroneous nodes or at least
disable their listening for connection.

As for long-timeouts on network failures, they can be handled by
existing connect_timeout parameter. 

May be it is worth effort to rewrite this parameter handling. Now it
only used in blocking functions which call connectDBComplete
(PQconnectdb[Params], PQreset and PQsetdbLogin). May be it should be hidden into
PQconnectPoll state machine, so event-driven applications could get its
handling for free.

Really, my initial proposal included hostorder=parallel especially for
this situation. But I've decided not to implement it right now, because
it requires changes in the libpq public API.
 
> Had any chance to simulate network failures?

Testing failover code is really tricky thing. All testing I've done yet
was in the mode "Shudown one of the nodes and see what happens".
It is not too hard to simulate network failures using iptables, but
requires considerable planning of the test scenario.


-- 
   Victor Wagner <vi...@wagner.pp.ru>


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

[HACKERS] Patch: Implement failover on libpq connect level.

[Victor Wagner]

On 2015.08.18 at 07:18:50 +0300, Victor Wagner wrote:

> Rationale
> =
> 
> Since introduction of the WAL-based replication into the PostgreSQL, it is
> possible to create high-availability and load-balancing clusters.
> 
> However, there is no support for failover in the client libraries. So, only
> way to provide transparent for client application failover is IP address
> migration. This approach has some limitation, i.e. it requires that
> master and backup servers reside in the same subnet or may not be
> feasible for other reasons.
> 
> Commercial RDBMS, such as Oracle, employ more flexible approach. They
> allow to specify multiple servers in the connect string, so if primary
> server is not available, client library tries to connect to other ones.
> 
> This approach allows to use geographically distributed failover clusters
> and also is a cheap way to implement load-balancing (which is not
> possible with IP address migration).
> 

Attached patch which implements client library failover and
loadbalancing as was described in the proposal
<20150818041850.ga5...@wagner.pp.ru>.

This patch implements following fuctionality:

1. It is allowed to specify several hosts in the connect string, either
in URL-style (separated by comma) or in param=value form (several host
parameters).

2. Each host parameter can be accompanied with port specifier separated
by colon. Port, specified such way takes precedence of port parameter or
default port for this particular host only.

3. There is hostorder parameter with two possible valies 'sequential'
and 'random' (default sequential). 'parallel' hostorder described in the
proposal is not yet implemented in this version of patch.

4. In the URL-style connect string parameter loadBalanceHosts=true is
considered equal to 'hostorder=random' for compatibility with jdbc.

5. Added new parameter readonly=boolean. If this parameter is false (the
default) upon successful connection check is performed that backend is
not in the recovery state. If so, connection is not considered usable
and next host is tried.

6. Added new parameter falover_timeout. If no usable host is found and
this parameter is specified, hosts are retried cyclically until this
timeout expires. It gives some time for claster management software to
promote one of standbys to master if master fails. By default there is
no retries.

Some implementation notes:

1. Field  pghost in the PGconn structure now stores comma separated list
of hosts, which is parsed in the connectDBStart. So, expected results of 
some tests in src/interfaces/libpq/test are changed. 

2. Url with colon  but no port number after the host no more considered
valid.

3. With hostorder=random we have to seed libc random number gernerator.
Some care was taken to not to lose entropy if generator was
initialized by app before connection, and to ensure different random
values if several connections are made from same application in one
second (even in single thread). RNG is seeded by xor of current time,
random value from this rng before seeding (if it was seeded earlier, it
keeps entropy) and address of the connection structure. May be it worth
effort adding thread id or pid, but there is no portable way to doing
so, so it would need testing on all supported platform.



diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 0ee018e..79a99ec 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -792,7 +792,7 @@ host=localhost port=5432 dbname=mydb connect_timeout=10

The general form for a connection URI is:
 
-postgresql://[user[:password]@][netloc][:port][/dbname][?param1=value1...]
+postgresql://[user[:password]@][netloc][:port][,netloc[:port]...][/dbname][?param1=value1...]
 

 
@@ -809,6 +809,7 @@ postgresql://localhost/mydb
 postgresql://user@localhost
 postgresql://user:secret@localhost
 postgresql://other@localhost/otherdb?connect_timeout=10application_name=myapp
+postgresql://node1,node2:5433,node3:4432,node4/mydb?hostorder=randomreadonly=1
 
 Components of the hierarchical part of the URI can also
 be given as parameters.  For example:
@@ -831,7 +832,9 @@ postgresql:///mydb?host=localhostport=5433

 For improved compatibility with JDBC connection URIs,
 instances of parameter ssl=true are translated into
-sslmode=require.
+sslmode=require and
+loadBalanceHosts=true  into
+hostorder=random.

 

@@ -841,6 +844,10 @@ postgresql:///mydb?host=localhostport=5433
 postgresql://[2001:db8::1234]/database
 

+   
+   There can be serveral host specifications, optionally accompanied
+   with port, separated by comma.
+   
 

 The host component is interpreted as described for the parameter PostgreSQL was built). On machines without
 Unix-domain sockets, the default is to connect to localhost.

+   
+   There can be more than one host parameter in
+ 

Re: [HACKERS] Proposal: Implement failover on libpq connect level.

[Victor Wagner]

В Mon, 07 Sep 2015 17:32:48 +0200
"Daniel Verite" <dan...@manitou-mail.org> пишет:

>   Victor Wagner wrote:
> 
> > It would just take a bit more time for client and a bit more load
> > for server - to make sure that this connection is read-write by
> > issuing
> > 
> >show transaction_read_only 
> > 
> > statement before considering connection useful.
> 
> If the purpose of the feature is to wait for a failover to complete,
> shouldn't it check for pg_is_in_recovery() rather than 
>  transaction_read_only ?
> 

Purpose of this feature is to distinguish between master and standby
servers. 

This allows failover system to work with standby servers accepting
client connections, and even to create system where read-only clients 
can be loadbalanced among several hot backup servers, and read-write
clients work with master, but do not need reconfiguration when
failover happens.

pg_is_in_recovery() is really better. But it seems that chapter 25 of
documentation should be improved and this function mentioned in the
section 25.5.1 (Hot Standby / User Overview)



-- 
   Victor Wagner <vi...@wagner.pp.ru>


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

[HACKERS] Discussion summary: Proposal: Implement failover on libpq connect level.

[Victor Wagner]

First of all, I wish to thank all the participants of the discussion
for their useful suggestions and critics.

I see that people are really interested in this feature.

Now, list of syntax and behavior changes against original proposal:

1. There is similar feature in JDBC, so syntax of URL-style connect
string should be compatible with JDBC:

postgresql://host:port,host:port/dbname...

2. In the param=value style connect string, there should be way to
specify different non-standard port for each host in the cluster.

So on, it seems that people came to conclusion, that allowing host:port
syntax as value of the host parameter shouldn't break anything.

So, port should be determined following way:

 If there is :portnumber in the host string, this portnumber takes
precedence over anything else for this particular host. To avoid
problems with file system objects with strange names, colon is
interpreted as port number separator only if value of the host
parameter doesn't begin with slash. If it begins with slash, it is
handled exactly as it handled now.

 If port is not explicitly specified in the host string, then use
parameter port= (for all hosts, which are not accompanied with own port
number), if no port parameter in the connect string, use same chain of
fallbacks up to compiled in standard value, as in use now.

3. Simultaneous connection to all the hosts in cluster shouldn't be
default. By default hosts should be tried in order they are specified
in the connect string. To minimize syntax changes there should be
single parameter 'hostorder' which specifies order of hosts with
possible choices
sequential(default), parallel and random.

4. There should be a way to allow cluster software or system
administrator some time to perform failover procedures and promote one
of standbys to master. So, there should be additional parameter,
failover_timeout, which specifies number of seconds connection attempts
should be retried, if no usable master server found.

5. Parameter readonly (which allows to distinguish between failover and
load balancing of readonly clients) should be implemented as in
original proposal.



-- 
   Victor Wagner <vi...@wagner.pp.ru>


-- 
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] Proposal: Implement failover on libpq connect level.

[Victor Wagner]

On 2015.08.19 at 09:21:50 +, Albe Laurenz wrote:

   Yes, but that will only work reliably if the (read-only) standby does not
   allow connections before it is promoted.
  
  It would just take a bit more time for client and a bit more load for
  server - to make sure that this connection is read-write by
  issuing
  
  show transaction_read_only
  
  statement before considering connection useful.
 
 That's not very comfortable, and a lot of middleware software won't easily
 learn the trick.

It shouldn't be left to middleware. It should be hidden into
PQConnectPoll. This function already handle very complicated state
transition diagram, including authentication, SSL negotiation and so on.
If we just add couple of new states such as CONNECTION_ASK_RW_STATUS
and CONNECTION_RW_STATUS_OK it should be fine.

Application would just call PQConnectPoll repeatedly (either via
PQconnectdb or explicitely when readable/writable condition detected on
the socket integrated into app even loop) until success or
unrecoverable error would be achieved. How many interaction with server
it would take, it is not middleware problem.


  It seems to me that in most cases last host in the connect string would
  be only host which accepts connections, so it wins anyway
 
 I'm not saying that it is particularly wide-spread and useful; it could
 happen through careless editing of connection strings or by using a
 connection service file entry
 (http://www.postgresql.org/docs/current/static/libpq-pgservice.html)
 and overriding the host parameter on the command line.


I don't think that host definition from all possible sources 
(service file, environment, command line) should be collected together.
Although it is essential to be clear when host list is appended and when
- replaced. 

If we implement sequential trial of all hosts, then we can start with
last one, to provide compatibility with existing behavior. In this case
if last host is online, no change occur.

Another idea - is to enable multiple host connection only if special
option (loadbalance or failover) present in the connect string.



  Other idea - allow to specify host-port pair as argument of host
  parameter.
  
host=db1.myorg.com:5432
  
  It is consistent with URL syntax and system administrators are used to
  it. And with long list of hosts there is less chances to made an error
  as host and corresponding port come together.
 
 I don't think that is very attactive as it confuses the distinction between
 host and port.  What would you do with
 
host=db1.myorg.com:2345 port=1234
 
I don't think that it does add any more confusion than simultaneous
existence of host and hostaddr, or ability to specify host and port both
in host part of URL and query parameters

postgresql://user@host:5432/dbname?host=otherhostport=2345

Bot really, you've convinced me that syntax with two or three (host, port
and hostaddr) parallel lists is the best. Although we'll need to allow
empty entries in the lists such as

host=master.db.com,standby1.db.com.standby2.db.com port=,2345, 
hostaddr=,192.168.0.4,192.160.1.8

with evident semantic:
get port, host and hostaddr elements with same number, and if some of
them are empty, behave as it was only host and corresponding parameter
not specified at all.

-- 
Victor Wagner vi...@wagner.pp.ru


-- 
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] Proposal: Implement failover on libpq connect level.

[''Victor Wagner *EXTERN*' *EXTERN*' *EXTERN*]

On 2015.08.19 at 15:35:17 +0100, Simon Riggs wrote:

 
 I think we do need some way of saying that a readonly connection is OK. So

I had such thing in my propsal (boolean parameter readonly). 
But haven't yet checked if it is compatible with jdbc syntax.

 the default would be to connect to each in turn until we find the master.
 It should keep retrying for a period of time since for a short period it is
 possible there is no master. If you specify readonly, then a connection to

It is very important addition  - to specify that if no host is able to
establish read-write session, we should retry and give a chance for
sever administration to promote one of standbys to master. Probably
there should be additional timeout parameter (we have
connection_timeout, and this would be failover_timeout) with some
reasonaable default.



-- 
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] Proposal: Implement failover on libpq connect level.

[Victor Wagner]

On 2015.08.20 at 00:17:35 +0900, Tatsuo Ishii wrote:

  But once connection is established, each client works with one
  server (at least until communication failure occurs and it would call
  PQreset. In this case it has to reprepare statements anyway).
 
 One downside of this is, if one of the standby servers is not
 responding, every time clients will be blocked by the server before
 giving up the connection trial. This could last for hours (for

This shouldn't happen. My proposal was to connect all servers
simultaneously, and then use that connection which would be established
first closing other ones

Even if we wouldn't do so (to properly randomize server load or to be
compatible with jdbc), there is connection_timeout parameter, so 
client wouldn't seat and just wait for hours while system TCP/IP stack
trying to connect nonexistent server.


 example, the network cable is plugged out). I think round robin DNS is
 better because the DNS server will drop the entry corresponding broken

DNS server wouldn't drop anything unless explicitely told so (by
administrator or by some watchdog software which is able to talk
nsupdate protocol). 

And not everyone database owner has control on his own domain.

Moreover, DNS is distributed system with agressive caching. If our
system is not local, DNS records for non-existing server would be cached
by DNS servers of client's internet provider. 



-- 
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] Proposal: Implement failover on libpq connect level.

['Victor Wagner *EXTERN*']

On 2015.08.18 at 08:32:28 +, Albe Laurenz wrote:

 I wonder how useful this is at the present time.
 
 If the primary goes down and the client gets connected to the standby,
 it would have read-only access there.  Most applications wouldn't cope
 well with that.

It is supposed that somebody (either system administrator or some
cluster management software) have noticed failure of master and promoted
one of the standbys to master.

So, clients have only to find out which cluster node serves as master
just now.

Idea is that we don't need any extra administration actions such as IP
migration to do it. Clients have list of alternate servers and discover
which one to work with by trial and error.

I consider in my proposal following situations:

1. Warm standby - doesn't accept client connection at all unless
promoted to master.

2. Hot standby - we have two types of clients - one for which readonly
access is sufficient, and other that need to connect only to master.
In this case intention to write is explicitely stated in the connect
string (readonly=false) and connect procedure would check if node it
tries to connect allowed write.

It seems that most people discussing in this thread think in millisecond
time intervals (failure and immediate reconnect).

I was thinking about much longer time intervals - it would probaly take
seconds to cluster management software to notice server failure and
promote backup server to master, it might be possible for application to
spend minute or so trying to reconnect, but it would take
hours to change connect string on clients - it would require visit of
support enginer to each client terminal, if we are thinking of
distributed OLTP system such as point-of-sale network with thick
clients.


 
  host=main-server host=standby1 host=standby2 port=5432 dbname=database
 
 It seems a bit arbitrary to require that all servers use the same port.

 Maybe parameters like host2, port2, host3, port3 etc. might be better.

I've thought about this approach. But PostgreSQL administration guide
insists that all servers in the cluster should have as identical
configuration as possible to simplify administration. 

Moreover I've seldom have seen configurations where postgresql is
accepting connection on non-default port.

Using host1, host2 etc would have unintended connotations, such is this
is first, main server. I think that client should treat all given
servers as equal and let cluster administration to choose which one
would accept connection.

-- 
Victor Wagner vi...@wagner.pp.ru



-- 
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] Proposal: Implement failover on libpq connect level.

[Victor Wagner]

On 2015.08.19 at 08:28:32 +0530, Amit Kapila wrote:

 On Tue, Aug 18, 2015 at 9:48 AM, Victor Wagner vi...@wagner.pp.ru wrote:
 
 
  Behavoir
  
 
  If PQconnectdb encounters connect string with multiple hosts specified,
  it attempts to establish connection with all these hosts simultaneously,
  and begins to work with server which responds first, unless
  loadbalancing parameter is true.
 
 
 I think here you are mixing the behaviour for load balancing solution and
 failover solution.  It seems to me that for client-side failover solution
 (which is also known as Transparent Application Failover), the connection
 attempt to second server should be done after the first connection is
 broken as that provide more flexibility.

I think that failover procedure should begin before first connection is
ever established.

When client application starts, it has no way of knowing current state
of the server cluster - which of servers is working as master now.

Application uses connect string, placed into its configuration file
long time ago, and changing this configuration might require special
permissions, user of application doesn't have. But user typically know
how to restart application or reboot his terminal. So, for the
spatially distributed networks with  thick clients we can handle only 
initial connections, not connection resets. At least application author
always can implement restoration of connection as closing old
connection and establishing new.

So, when application first establishes connection it have to be prepared
to connect any of alternate hosts.

I don't think that making connections in sequential order provide big
flexibility. But it can greatly increase startup time, because connect
to host which is physically down fails after significant timeout. While
application waits for first connect to fail, it might complete session
initialization with working server several times.

Of course, connecting to servers in sequential order is simpler to
implement, and allows even more mixing of load balancing with failover,
because code would be same.


 Although both ideas (load balancing and failover) seems worth discussing,
 they are separate features and can be worked on separately.  It will be
 easier to sort out the details as well that way.

Really load balancing comes almost for free if we implement connect to
alternate server for failover purposes. I'm not sure that in case of hot
standby, where only readonly transactions can be loadbalanced,
loadbalancing is very useful. And included it in the proposal only
because it is very cheap to implement in this form,

 
 With Regards,
 Amit Kapila.
 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] Proposal: Implement failover on libpq connect level.

[Victor Wagner *EXTERN*]

On 2015.08.19 at 12:29:51 +0530, Amit Kapila wrote:

  It seems that most people discussing in this thread think in millisecond
  time intervals (failure and immediate reconnect).
 
 Why not have this as a separate parameter (*_timeout or something like
 that)?

Because it is not in the software configuration. It is in the people
heads. Or may be in the organizational configuration of the environments
we are talking about.

Each of us imagining some use-case for discussed feature. And these
cases are completely different, and have different typical time
interval.

I haven't explicitely stated my use cases in the proposal. So people
thinking in terms of their use cases, and this is very significant
feedback for me.




-- 
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] Proposal: Implement failover on libpq connect level.

[Victor Wagner]

On 2015.08.19 at 12:42:45 +0900, Tatsuo Ishii wrote:

 I wonder how extended protocol is handled by this proposal. Suppose
 load balacing mode is enabled. PQprepare is executed on standby1. Then
 PQexecPrepared gets called. This may be executed on standby2, which
 will fail because there's no prepared statement created by the former
 PQprepare call.

Here we are discussing load-balancing on the client level, not on the
statement level.

Suppose that we have 100 readonly clients and 3 standby servers + master.
If all clients specify all four servers in the their connect strings,
and connect randomly to them, each server would have approximately 25
clients.

But once connection is established, each client works with one
server (at least until communication failure occurs and it would call
PQreset. In this case it has to reprepare statements anyway).



-- 
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] Proposal: Implement failover on libpq connect level.

[''Victor Wagner *EXTERN*' *EXTERN*']

On 2015.08.19 at 07:15:30 +, Albe Laurenz wrote:

  Idea is that we don't need any extra administration actions such as IP
  migration to do it. Clients have list of alternate servers and discover
  which one to work with by trial and error.
 
 Yes, but that will only work reliably if the (read-only) standby does not
 allow connections before it is promoted.

It would just take a bit more time for client and a bit more load for
server - to make sure that this connection is read-write by
issuing

show transaction_read_only 

statement before considering connection useful.

 
 And as Robert said, there are people out using BDR or other proprietary
 multi-master solutions, so there might well be an audience for this feature.
 
Unfortunately I have no experience with such solutions, so I'd greatly
appreciate feedback from those people.

I've modelled my proposal after another proprietary solution  - Oracle
RAC.


 One problem with that is that this syntax is already allowed, but
 your proposal would silently change the semantics.
 Today, if you have multiple host parameters, the last one wins.
 So with your modification in place, some connect strings that work today
 would start behaving in unexpected ways.

This is serious argument. But what the use case of these connect strings
now? 

It seems to me that in most cases last host in the connect string would
be only host which accepts connections, so it wins anyway

 
 Maybe a better idea would be:
   host=db1.myorg.com,db2.myorg.com port=5432,2345

I've tried not to introduce new delimiters. But this syntax definitely
have some advantages. At least it allows to specify host-port pairs as
two parallel lists.

Other idea - allow to specify host-port pair as argument of host
parameter. 

  host=db1.myorg.com:5432

It is consistent with URL syntax and system administrators are used to
it. And with long list of hosts there is less chances to made an error
as host and corresponding port come together.

But your variant allows to handle hostaddr parameter same way as host
and port.
-- 
Victor Wagner vi...@wagner.pp.ru


-- 
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] Proposal: Implement failover on libpq connect level.

[Victor Wagner]

On 2015.08.19 at 12:55:15 +0530, Amit Kapila wrote:

  I think that failover procedure should begin before first connection is
  ever established.
 
 
 As far as I understand, failover gets initiated once the master server goes
 down or is not accessible due to some reason, so for such cases if you
 have the connection to both the servers then it might not work.

Master server might go down when client is not started yet. 
And when client starts up, it has to find out which server to connect
now.

Consider point-of-sale terminals, bank offices or anything else, which
do not work round the clock. Clerk comes to his workplace in the
morning, switches on terminal and inserts her smartcard to authorize
with server. She doesn't need to know what server name is and where it
is located. Either application finds the server automatically, or
support engineer has to be called to fix things.

Moreover, in some situations restart of application (or even client
terminal) is acceptable price for failover, as long as there is no need
to manually fix the configuration.



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

[HACKERS] Proposal: Implement failover on libpq connect level.

[Victor Wagner]

 and
postgresql authentication). Typically, in the situation where read-only
clients should be load-balanced using this feature, there are much more 
read-only clients, than read-write ones. So some extra load related with 
read-write connection seems to be justified by simplification of client
configuration.

-- 
  Victor Wagner vi...@wagner.pp.ru


-- 
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] [PATCHES] Backend SSL configuration enhancement

[Victor Wagner]

On 2006.09.04 at 15:46:03 -0400, Bruce Momjian wrote:

 Tom Lane wrote:
  Bruce Momjian [EMAIL PROTECTED] writes:
   This has been saved for the 8.3 release:
 http://momjian.postgresql.org/cgi-bin/pgpatches_hold
  
  This version was withdrawn by the author for rework, no?
 
 Right, and the thread in patches_hold shows that.  The reason it is in
 there is so we can ping the author at the start of 8.3 to get an updated
 version.

I've already send version 2 of the patch to patches mailing list.
I think that this letter even got into thread mentioned above.

It's a pity that it's to late for patch to get into 8.2.
It means that during all 8.2 lifecycle we'll have to maintain this patch
separately.


---(end of broadcast)---
TIP 1: if posting/reading through Usenet, please send an appropriate
   subscribe-nomail command to [EMAIL PROTECTED] so that your
   message can get through to the mailing list cleanly

[HACKERS] Re: [PATCHES] Cyrillic to UNICODE conversion

[Victor Wagner]

On Sun, 29 Apr 2001, Tatsuo Ishii wrote:

 From: Tatsuo Ishii [EMAIL PROTECTED]
 Subject: Re: [PATCHES] Cyrillic to UNICODE conversion
 X-Mailer: Mew version 1.94.2 on Emacs 20.7 / Mule 4.1
  [iso-2022-jp] (^[$B0*^[(B)

 Thanks for the fixes. I have committed your patches and they should
 appear in 7.1.1.

 BTW, I have not added cp1251.txt  cp866.txt  koi8-r.txt, since they
 come from Unicode.org and are not permitted to re-distribute.

It is not true for koi8-r.txt. At least one which is included into catdoc
distribution I've made myself from RFC1483, and only afterward it has
appear on unicode.org, and Chernov's KOI8 pages.

 But anyway, if anybody
is able to get them from unicode.org, why bother.
--
Victor Wagner   [EMAIL PROTECTED]
Chief Technical Officer Office:7-(095)-748-53-88
Communiware.Net Home: 7-(095)-135-46-61
http://www.communiware.net  http://www.ice.ru/~vitus


---(end of broadcast)---
TIP 5: Have you checked our extensive FAQ?

http://www.postgresql.org/users-lounge/docs/faq.html