date:20150421

Re: [HACKERS] Streaming replication and WAL archive interactions

2015-04-21 Thread Heikki Linnakangas


On 04/22/2015 03:30 AM, Michael Paquier wrote:

This is going to change a behavior that people are used to for a
couple of releases. I would not mind having this patch do
"archive_mode = on during recovery" => archive only segments generated
by this node + the last partial segment on the old timeline at
promotion, without renaming to preserve backward compatible behavior.
If master and standby point to separate archive locations, this way
the operator can sort out later the one he would want to use. If they
point to the same location, archive_command scripts already do
internally such renaming, at least that's what I suspect an
experienced user would do, now it is true that not many people are
experienced in this area I see mistakes regarding such things on a
weekly basis... This .partial segment renaming is something that we
should let the archive_command manage with its internal logic.


Currently, the archive command doesn't know if the segment it's 
archiving is partial or not, so you can't put any logic there to manage 
it. But if we archive it with the .partial suffix, then you can put 
logic in the restore_command to check for .partial files, if you really 
want to.


I feel that the best approach is to archive the last, partial segment, 
but with the .partial suffix. I don't see any plausible real-world setup 
where the current behaviour would be better. I don't really see much 
need to archive the partial segment at all, but there's also no harm in 
doing it, as long as it's clearly marked with the .partial suffix.


BTW, pg_receivexlog also uses a ".partial" file, while it's streaming 
WAL from the server. The .partial suffix is removed when the segment is 
complete. So there's some precedence to this. pg_receivexlog adds just 
".partial" to the filename, it doesn't add any information of what 
portion of the file is valid like I suggested here, though. Perhaps we 
should follow pg_receivexlog's example at promotion too, for consistency.


- Heikki



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

Re: [HACKERS] Add pg_settings.pending_restart column

2015-04-21 Thread Michael Paquier

On Thu, Mar 5, 2015 at 12:04 PM, Peter Eisentraut  wrote:
> On 2/17/15 10:45 AM, Robert Haas wrote:
>> You don't really need the "else" here, and in parallel cases:
>>
>>  if (*conf->variable != newval)
>>  {
>> +record->status |= GUC_PENDING_RESTART;
>>  ereport(elevel,
>>  (errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
>>   errmsg("parameter \"%s\" cannot be
>> changed without restarting the server",
>>  name)));
>>  return 0;
>>  }
>> +else
>> +record->status &= ~GUC_PENDING_RESTART;
>>  return -1;
>>
>> The if-statement ends with "return 0" so there is no reason for the "else".
>
> I kind of liked the symmetry of if/else, but I can change it.

This feature looks useful to me. I had a quick look and it is working
as intended: issuing SIGHUP to reload parameters updates the
pending_restart status correctly.

One additional comment on top of what has already been mentioned is
that this lacks parenthesis IMO:
- values[16] = conf->status & GUC_PENDING_RESTART ? "t" : "f";
+ values[16] = (conf->status & GUC_PENDING_RESTART) ? "t" : "f";
Also, documentation was not correctly formatted.

Changes with ALTER SYSTEM (and include files) get recognized as well.
For example:
=# \! echo max_prepared_transactions = 100 >> $PGDATA/postgresql.conf
=# select pg_reload_conf();
 pg_reload_conf

 t
(1 row)
=# select name from pg_settings where pending_restart;
   name
---
 max_prepared_transactions
(1 row)
=# alter system set max_connections = 1000;
ALTER SYSTEM
=# select pg_reload_conf();
 pg_reload_conf

 t
(1 row)
=# select name from pg_settings where pending_restart;
   name
---
 max_connections
 max_prepared_transactions
(2 rows)

Attached is a rebased patch with previous comments addressed as I was
looking at it.
Switching this patch as "Ready for committer".
Regards,
-- 
Michael
diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index d0b78f2..53d3f4f 100644
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -8822,6 +8822,14 @@ SELECT * FROM pg_locks pl LEFT JOIN pg_prepared_xacts ppx
   or when examined by a non-superuser)
   
  
+ 
+  pending_restart
+  boolean
+  true if the value has been changed in the
+  configuration file but needs a restart; or false
+  otherwise.
+  
+ 
 

   
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index f43aff2..e09b021 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -5897,12 +5897,14 @@ set_config_option(const char *name, const char *value,
 {
 	if (*conf->variable != newval)
 	{
+		record->status |= GUC_PENDING_RESTART;
 		ereport(elevel,
 (errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
  errmsg("parameter \"%s\" cannot be changed without restarting the server",
 		name)));
 		return 0;
 	}
+	record->status &= ~GUC_PENDING_RESTART;
 	return -1;
 }
 
@@ -5985,12 +5987,14 @@ set_config_option(const char *name, const char *value,
 {
 	if (*conf->variable != newval)
 	{
+		record->status |= GUC_PENDING_RESTART;
 		ereport(elevel,
 (errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
  errmsg("parameter \"%s\" cannot be changed without restarting the server",
 		name)));
 		return 0;
 	}
+	record->status &= ~GUC_PENDING_RESTART;
 	return -1;
 }
 
@@ -6073,12 +6077,14 @@ set_config_option(const char *name, const char *value,
 {
 	if (*conf->variable != newval)
 	{
+		record->status |= GUC_PENDING_RESTART;
 		ereport(elevel,
 (errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
  errmsg("parameter \"%s\" cannot be changed without restarting the server",
 		name)));
 		return 0;
 	}
+	record->status &= ~GUC_PENDING_RESTART;
 	return -1;
 }
 
@@ -6179,12 +6185,14 @@ set_config_option(const char *name, const char *value,
 	if (*conf->variable == NULL || newval == NULL ||
 		strcmp(*conf->variable, newval) != 0)
 	{
+		record->status |= GUC_PENDING_RESTART;
 		ereport(elevel,
 (errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
  errmsg("parameter \"%s\" cannot be changed without restarting the server",
 		name)));
 		return 0;
 	}
+	record->status &= ~GUC_PENDING_RESTART;
 	return -1;
 }
 
@@ -6272,12 +6280,14 @@ set_config_option(const char *name, const char *value,
 {
 	if (*conf->variable != newval)
 	{
+		record->status |= GUC_PENDING_RESTART;
 		ereport(elevel,
 (errcode(ERRCODE_CANT_CHANGE_RUNT

Re: [HACKERS] Streaming replication and WAL archive interactions

2015-04-21 Thread Heikki Linnakangas


On 04/22/2015 12:42 AM, Robert Haas wrote:

On Tue, Apr 21, 2015 at 6:55 AM, Heikki Linnakangas  wrote:

On 04/21/2015 12:04 PM, Michael Paquier wrote:

On Tue, Apr 21, 2015 at 4:38 PM, Heikki Linnakangas 
wrote:

Note that even though we don't archive the partial last segment on the
previous timeline, the same WAL is copied to the first segment on the new
timeline. So the WAL isn't lost.


But if the failed master has archived those segments safely, we may need
them, no? I am not sure we can ignore a user who would want to do a PITR
with recovery_target_timeline pointing to the one of the failed master.


I think it would be acceptable. If you want to maintain an up-to-the-second
archive, you can use pg_receivexlog. Mind you, if the standby wasn't
promoted, the partial segment would not be present in the archive anyway.
And you can copy the WAL segment manually from 000200XX to
pg_xlog/000100XX before starting PITR.

Another thought is that we could archive the partial file, but with a
different name to avoid confusing it with the full segment. For example, we
could archive a partial 00010012 segment as
"00020012.0128.partial", where 0128 indicates how
far that file is valid (this naming is similar to how the backup history
files are named). Recovery wouldn't automatically pick up those files, but
the DBA could easily copy the partial file into pg_xlog with the full
segment's name, if he wants to do PITR to that piece of WAL.


So, suppose you A replicating to B (via an archive) replicating to C
(via a separate archive); A dies, B is promoted.  It sounds to me like
today this will work and with your proposed change it will require
manual intervention.


No. If there is no streaming replication involved, no partial files will 
be archived, with or without this patch. There is no change to that 
scenario.


Note that it's a bit complicated to set up that scenario today. 
Archiving is never enabled in recovery mode, so you'll need to use a 
custom cron job or something to maintain the archive that C uses. The 
files will not automatically flow from B to the second archive. With the 
patch we're discussing, however, it would be easy: just set 
archive_mode='always' in B.


- Heikki


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

[HACKERS] Authenticating from SSL certificates

2015-04-21 Thread kee...@thebrocks.net

Hello,

I'm looking into connection to postgres using authentication from client
certificates. [1]

The documentation states that the common name (aka CN) is read from the
certificate and used as the user's login (aka auth_user).
The problem is the common name is typically the user's full name. A field
like email address would contain a more computer friendly identifier.

So my feature request is to allow the postgres admin to specify the field
in the ssl client certificate to be used to read the auth_user.

I started to dig into the code and have some thoughts, but wanted to get
any advice before I started writing up some code.

Add a "user" option to pg_hba.conf:
# TYPE DATABASE USER ADDRESS METHOD
hostssl all all all cert map=usermap user=CN

1. Documentation seems straight forward [1]
2. The configuration value would be added in parse_hba_line and this value
is accessible via port->hba.
3. The certificate can be parsed from port->peer with something like
X509_NAME_field_to_text [2].
4. The user requested field would then be passed as auth_user
into check_usermap [3].

The current code parses the ssl common name and populates peer_cn pretty
early on. [4]
That suggests to me that most of the ssl parsing wants to be done up front.
Then again, peer_cn is not used anywhere else so it may be fine to just
delete this field from the structure.

An alternative is to populate peer_cn with the user requested field. [4]
The configuration option would be in postgresql.conf and would reside in a
global variable (similar to ssl_cert_file).

Any pointers would be great.
I could find a little history in the archives, but couldn't determine if
any decisions or conclusions had been made.

Thanks,
Keenan

[1]: http://www.postgresql.org/docs/9.4/static/auth-methods.html#AUTH-CERT
[2]:
https://github.com/postgres/postgres/blob/b0a738f428ca4e52695c0f019c1560c64cc59aef/contrib/sslinfo/sslinfo.c#L171-L192
[3]:
https://github.com/postgres/postgres/blob/b0a738f428ca4e52695c0f019c1560c64cc59aef/src/backend/libpq/auth.c#L2153
[4]:
https://github.com/postgres/postgres/blob/b0a738f428ca4e52695c0f019c1560c64cc59aef/src/backend/libpq/be-secure-openssl.c#L428-L445

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-04-21 Thread Kouhei Kaigai

Hanada-san,

> I reviewed the Custom/Foreign join API patch again after writing a patch of 
> join
> push-down support for postgres_fdw.
>
Thanks for your dedicated jobs, my comments are inline below.

> Here, please let me summarize the changes in the patch as the result of my 
> review.
> 
> * Add set_join_pathlist_hook_type in add_paths_to_joinrel
> This hook is intended to provide a chance to add one or more CustomPaths for 
> an
> actual join combination.  If the join is reversible, the hook is called for 
> both
> A * B and B * A.  This is different from FDW API but it seems fine because 
> FDWs
> should have chances to process the join in more abstract level than CSPs.
> 
> Parameters are same as hash_inner_and_outer, so they would be enough for 
> hash-like
> or nestloop-like methods.  I’m not sure whether mergeclause_list is necessary
> as a parameter or not.  It’s information for merge join which is generated 
> when
> enable_mergejoin is on and the join is not FULL OUTER.  Does some CSP need it
> for processing a join in its own way?  Then it must be in parameter list 
> because
> select_mergejoin_clauses is static so it’s not accessible from external 
> modules.
>
I think, a preferable way is to reproduce the mergeclause_list by extension 
itself,
rather than pass it as a hook argument, because it is uncertain whether CSP 
should
follow "enable_mergejoin" parameter even if it implements a logic like 
merge-join.
Of course, it needs to expose select_mergejoin_clauses. It seems to me a 
straight-
forward way.

> The timing of the hooking, after considering all built-in path types, seems 
> fine
> because some of CSPs might want to use built-in paths as a template or a 
> source.
> 
> One concern is in the document of the hook function.  "Implementing Custom 
> Paths”
> says:
> 
> > A custom scan provider will be also able to add paths by setting the 
> > following
> hook, to replace built-in join paths by custom-scan that performs as if a scan
> on preliminary joined relations, which us called after the core code has 
> generated
> what it believes to be the complete and correct set of access paths for the 
> join.
> 
> I think “replace” would mis-lead readers that CSP can remove or edit existing
> built-in paths listed in RelOptInfo#pathlist or linked from cheapest_foo.  
> IIUC
> CSP can just add paths for the join relation, and planner choose it if it’s 
> the
> cheapest.
>
I adjusted the documentation stuff as follows:

   A custom scan provider will be also able to add paths by setting the
   following hook, to add CustomPath nodes that perform as
   if built-in join logic doing. It is typically expected to take two
   input relations then generate a joined output stream, or just scans
   preliminaty joined relations like materialized-view. This hook is
   called next to the consideration of core join logics, then planner
   will choose the best path to run the relations join in the built-in
   and custom ones.

Probably, it can introduce what this hook works correctly.
v12 patch updated only this portion.

> * Add new FDW API GetForeignJoinPaths in make_join_rel
> This FDW API is intended to provide a chance to add ForeignPaths for a join 
> relation.
> This is called only once for a join relation, so FDW should consider reversed
> combination if it’s meaningful in their own mechanisms.
> 
> Note that this is called only when the join relation was *NOT* found in the
> PlannerInfo, to avoid redundant calls.
>
Yep, it is designed according to the discussion upthreads.
It can produce N-way remote join paths even if intermediate join relation is
more expensive than local join + two foreign scan.

> Parameters seems enough for postgres_fdw to process N-way join on remote side
> with pushing down join conditions and remote filters.
>
You ensured it clearly.

> * Treat scanrelid == 0 as pseudo scan
> A foreign/custom join is represented by a scan against a pseudo relation, i.e.
> result of a join.  Usually Scan has valid scanrelid, oid of a relation being
> scanned, and many functions assume that it’s always valid.  The patch adds 
> another
> code paths for scanrelid == 0 as custom/foreign join scans.
>
Right,

> * Pseudo scan target list support
> CustomScan and ForeignScan have csp_ps_tlist and fdw_ps_tlist respectively, 
> for
> column reference tracking.  A scan generated for custom/foreign join would 
> have
> column from multiple relations in its target list, i.e. output columns.  
> Ordinary
> scans have all valid columns of the relation as output, so references to them
> can be resolved easily, but we need an additional mechanism to determine where
> a reference in a target list of custom/foreign scan come from.  This is very
> similar to what IndexOnlyScan does, so we reuse INDEX_VAR as mark of an 
> indirect
> reference to another relation’s var.
>
Right, FDW/CSP driver is responsible to set *_ps_tlist to inform the core 
planner
which columns of relations are referenced, and which attr

Re: [HACKERS] Idea: closing the loop for "pg_ctl reload"

2015-04-21 Thread Jan de Visser

On April 21, 2015 09:34:51 PM Jan de Visser wrote:
> On April 21, 2015 09:01:14 PM Jan de Visser wrote:
> > On April 21, 2015 07:32:05 PM Payal Singh wrote:
... snip ...
> 
> Urgh. It appears you are right. Will fix.
> 
> jan

Attached a new attempt. This was one from the category 'I have no idea how 
that ever worked", but whatever. For reference, this is how it looks for me 
(magic man-behind-the-curtain postgresql.conf editing omitted):

jan@wolverine:~/Projects/postgresql$ initdb -D data
... Bla bla bla ...
jan@wolverine:~/Projects/postgresql$ pg_ctl -D data -l logfile start
server starting
jan@wolverine:~/Projects/postgresql$ tail -5 logfile
LOG:  database system was shut down at 2015-04-21 22:03:33 EDT
LOG:  database system is ready to accept connections
LOG:  autovacuum launcher started
jan@wolverine:~/Projects/postgresql$ pg_ctl -D data reload
server signaled
jan@wolverine:~/Projects/postgresql$ tail -5 logfile
LOG:  database system was shut down at 2015-04-21 22:03:33 EDT
LOG:  database system is ready to accept connections
LOG:  autovacuum launcher started
LOG:  received SIGHUP, reloading configuration files
jan@wolverine:~/Projects/postgresql$ pg_ctl -D data reload
server signaled
pg_ctl: Reload of server with PID 14656 FAILED
Consult the server log for details.
jan@wolverine:~/Projects/postgresql$ tail -5 logfile
LOG:  autovacuum launcher started
LOG:  received SIGHUP, reloading configuration files
LOG:  received SIGHUP, reloading configuration files
LOG:  syntax error in file "/home/jan/Projects/postgresql/data/postgresql.conf" 
line 1, near end of line
LOG:  configuration file "/home/jan/Projects/postgresql/data/postgresql.conf" 
contains errors; no changes were applied
jan@wolverine:~/Projects/postgresql$ diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index a9f20ac..a7819d2 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -1222,6 +1222,15 @@ PostmasterMain(int argc, char *argv[])
 #endif
 
 	/*
+	 * Update postmaster.pid with startup time as the last reload time:
+	 */
+	{
+		char last_reload_info[32];
+		snprintf(last_reload_info, 32, "%ld %d", (long) MyStartTime, 1);
+		AddToDataDirLockFile(LOCK_FILE_LINE_LAST_RELOAD, last_reload_info);
+	}
+
+	/*
 	 * Remember postmaster startup time
 	 */
 	PgStartTime = GetCurrentTimestamp();
@@ -2341,6 +2350,8 @@ static void
 SIGHUP_handler(SIGNAL_ARGS)
 {
 	int			save_errno = errno;
+	boolreload_success;
+	charlast_reload_info[32];
 
 	PG_SETMASK(&BlockSig);
 
@@ -2348,7 +2359,16 @@ SIGHUP_handler(SIGNAL_ARGS)
 	{
 		ereport(LOG,
 (errmsg("received SIGHUP, reloading configuration files")));
-		ProcessConfigFile(PGC_SIGHUP);
+		reload_success = ProcessConfigFile(PGC_SIGHUP);
+
+		/*
+		 * Write the current time and the result of the reload to the
+		 * postmaster.pid file.
+		 */
+		snprintf(last_reload_info, 32, "%ld %d",
+(long) time(NULL), reload_success);
+		AddToDataDirLockFile(LOCK_FILE_LINE_LAST_RELOAD, last_reload_info);
+
 		SignalChildren(SIGHUP);
 		SignalUnconnectedWorkers(SIGHUP);
 		if (StartupPID != 0)
diff --git a/src/backend/utils/misc/guc-file.l b/src/backend/utils/misc/guc-file.l
index c5e0fac..3162cd5 100644
--- a/src/backend/utils/misc/guc-file.l
+++ b/src/backend/utils/misc/guc-file.l
@@ -109,7 +109,7 @@ STRING			\'([^'\\\n]|\\.|\'\')*\'
  * All options mentioned in the configuration file are set to new values.
  * If an error occurs, no values will be changed.
  */
-void
+bool
 ProcessConfigFile(GucContext context)
 {
 	bool		error = false;
@@ -202,7 +202,7 @@ ProcessConfigFile(GucContext context)
 		 * the config file.
 		 */
 		if (head == NULL)
-			return;
+			return false;
 	}
 
 	/*
@@ -430,6 +430,7 @@ ProcessConfigFile(GucContext context)
 	 * freed here.
 	 */
 	FreeConfigVariables(head);
+	return !error;
 }
 
 /*
diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c
index 80d7bc7..0ffe97b 100644
--- a/src/bin/pg_ctl/pg_ctl.c
+++ b/src/bin/pg_ctl/pg_ctl.c
@@ -73,6 +73,20 @@ typedef enum
 	RUN_AS_SERVICE_COMMAND
 } CtlCommand;
 
+typedef struct
+{
+	pgpid_tpid;
+	char  *datadir;
+	time_t startup_ts;
+	intport;
+	char  *socketdir;
+	char  *listenaddr;
+	unsigned long  shmkey;
+	intshmid;
+	time_t reload_ts;
+	bool   reload_ok;
+} PIDFileContents;
+
 #define DEFAULT_WAIT	60
 
 static bool do_wait = false;
@@ -153,6 +167,8 @@ static int	CreateRestrictedProcess(char *cmd, PROCESS_INFORMATION *processInfo,
 static pgpid_t get_pgpid(bool is_status_request);
 static char **readfile(const char *path);
 static void free_readfile(char **optlines);
+static PIDFileContents * get_pidfile_contents(const char *path);
+static void free_pidfile_contents(PIDFileContents *contents);
 static int	start_postmaster(void);
 static void read_post_opts(void);
 
@@ -415,6 +431,78 @@ free_readfile(char **optlines)
 }
 
 /*
+ * Read and parse the contents of

Re: [HACKERS] Idea: closing the loop for "pg_ctl reload"

2015-04-21 Thread Jan de Visser

On April 21, 2015 09:01:14 PM Jan de Visser wrote:
> On April 21, 2015 07:32:05 PM Payal Singh wrote:
> > I'm trying to review this patch and applied
> > http://www.postgresql.org/message-id/attachment/37123/Let_pg_ctl_check_the
> > _r esult_of_a_postmaster_config_reload.patch to postgres. gmake check
> > passed but while starting postgres I see:
> > 
> > [postgres@vagrant-centos65 data]$ LOG:  incomplete data in
> > "postmaster.pid": found only 5 newlines while trying to add line 8
> > LOG:  redirecting log output to logging collector process
> > HINT:  Future log output will appear in directory "pg_log".
> > 
> > 
> > Also, a simple syntax error test gave no warning at all on doing a reload,
> > but just as before there was an error message in the logs:
> > 
> > [postgres@vagrant-centos65 data]$ /usr/local/pgsql/bin/pg_ctl -D
> > /usr/local/pgsql/data reload
> > server signaled
> > [postgres@vagrant-centos65 data]$ cd pg_log
> > [postgres@vagrant-centos65 pg_log]$ ls
> > postgresql-2015-04-21_232328.log  postgresql-2015-04-21_232858.log
> > [postgres@vagrant-centos65 pg_log]$ grep error
> > postgresql-2015-04-21_232858.log
> > LOG:  syntax error in file "/usr/local/pgsql/data/postgresql.conf" line
> > 211, near token "/"
> > LOG:  configuration file "/usr/local/pgsql/data/postgresql.conf" contains
> > errors; no changes were applied
> > 
> > I'm guessing since this is a patch submitted to the commitfest after the
> > current one, am I too early to start reviewing it?
> > 
> > Payal
> 
> But, but, but...  it worked for me... :-)
> 
> I'll have a look. I'll apply my patch to a clean tree and see if any bits
> have rotted in the last month or so.
> 
> One thing to note is that you won't get the actual error; just a message
> that reloading failed.
> 
> jan

Urgh. It appears you are right. Will fix.

jan


-- 
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] Idea: closing the loop for "pg_ctl reload"

2015-04-21 Thread Jan de Visser

(Please don't top post)

On April 21, 2015 07:32:05 PM Payal Singh wrote:
> I'm trying to review this patch and applied
> http://www.postgresql.org/message-id/attachment/37123/Let_pg_ctl_check_the_r
> esult_of_a_postmaster_config_reload.patch to postgres. gmake check passed
> but while starting postgres I see:
> 
> [postgres@vagrant-centos65 data]$ LOG:  incomplete data in
> "postmaster.pid": found only 5 newlines while trying to add line 8
> LOG:  redirecting log output to logging collector process
> HINT:  Future log output will appear in directory "pg_log".
> 
> 
> Also, a simple syntax error test gave no warning at all on doing a reload,
> but just as before there was an error message in the logs:
> 
> [postgres@vagrant-centos65 data]$ /usr/local/pgsql/bin/pg_ctl -D
> /usr/local/pgsql/data reload
> server signaled
> [postgres@vagrant-centos65 data]$ cd pg_log
> [postgres@vagrant-centos65 pg_log]$ ls
> postgresql-2015-04-21_232328.log  postgresql-2015-04-21_232858.log
> [postgres@vagrant-centos65 pg_log]$ grep error
> postgresql-2015-04-21_232858.log
> LOG:  syntax error in file "/usr/local/pgsql/data/postgresql.conf" line
> 211, near token "/"
> LOG:  configuration file "/usr/local/pgsql/data/postgresql.conf" contains
> errors; no changes were applied
> 
> I'm guessing since this is a patch submitted to the commitfest after the
> current one, am I too early to start reviewing it?
> 
> Payal

But, but, but...  it worked for me... :-)

I'll have a look. I'll apply my patch to a clean tree and see if any bits have 
rotted in the last month or so. 

One thing to note is that you won't get the actual error; just a message that 
reloading failed.

jan



-- 
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] Streaming replication and WAL archive interactions

2015-04-21 Thread Michael Paquier

On Wed, Apr 22, 2015 at 6:42 AM, Robert Haas  wrote:
>
> On Tue, Apr 21, 2015 at 6:55 AM, Heikki Linnakangas  wrote:
> > On 04/21/2015 12:04 PM, Michael Paquier wrote:
> >> On Tue, Apr 21, 2015 at 4:38 PM, Heikki Linnakangas 
> >> wrote:
> >>> Note that even though we don't archive the partial last segment on the
> >>> previous timeline, the same WAL is copied to the first segment on the new
> >>> timeline. So the WAL isn't lost.
> >>
> >> But if the failed master has archived those segments safely, we may need
> >> them, no? I am not sure we can ignore a user who would want to do a PITR
> >> with recovery_target_timeline pointing to the one of the failed master.
> >
> > I think it would be acceptable. If you want to maintain an up-to-the-second
> > archive, you can use pg_receivexlog. Mind you, if the standby wasn't
> > promoted, the partial segment would not be present in the archive anyway.
> > And you can copy the WAL segment manually from 000200XX to
> > pg_xlog/000100XX before starting PITR.
> >
> > Another thought is that we could archive the partial file, but with a
> > different name to avoid confusing it with the full segment. For example, we
> > could archive a partial 00010012 segment as
> > "00020012.0128.partial", where 0128 indicates how
> > far that file is valid (this naming is similar to how the backup history
> > files are named). Recovery wouldn't automatically pick up those files, but
> > the DBA could easily copy the partial file into pg_xlog with the full
> > segment's name, if he wants to do PITR to that piece of WAL.
>
> So, suppose you A replicating to B (via an archive) replicating to C
> (via a separate archive); A dies, B is promoted.  It sounds to me like
> today this will work and with your proposed change it will require
> manual intervention.  I don't think that's OK.


This is going to change a behavior that people are used to for a
couple of releases. I would not mind having this patch do
"archive_mode = on during recovery" => archive only segments generated
by this node + the last partial segment on the old timeline at
promotion, without renaming to preserve backward compatible behavior.
If master and standby point to separate archive locations, this way
the operator can sort out later the one he would want to use. If they
point to the same location, archive_command scripts already do
internally such renaming, at least that's what I suspect an
experienced user would do, now it is true that not many people are
experienced in this area I see mistakes regarding such things on a
weekly basis... This .partial segment renaming is something that we
should let the archive_command manage with its internal logic.
Regards,
-- 
Michael


-- 
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] Fix broken Install.bat when target directory contains a space

2015-04-21 Thread Michael Paquier

On Wed, Apr 22, 2015 at 12:40 AM, Asif Naeem  wrote:

> Thank you Michael, latest patch looks good to me. I have changed its
> status to ready for committer.
>

Thanks!
-- 
Michael

Re: [HACKERS] Idea: closing the loop for "pg_ctl reload"

2015-04-21 Thread Payal Singh

I'm trying to review this patch and applied
http://www.postgresql.org/message-id/attachment/37123/Let_pg_ctl_check_the_result_of_a_postmaster_config_reload.patch
to postgres. gmake check passed but while starting postgres I see:

[postgres@vagrant-centos65 data]$ LOG:  incomplete data in
"postmaster.pid": found only 5 newlines while trying to add line 8
LOG:  redirecting log output to logging collector process
HINT:  Future log output will appear in directory "pg_log".

Also, a simple syntax error test gave no warning at all on doing a reload,
but just as before there was an error message in the logs:

[postgres@vagrant-centos65 data]$ /usr/local/pgsql/bin/pg_ctl -D
/usr/local/pgsql/data reload
server signaled
[postgres@vagrant-centos65 data]$ cd pg_log
[postgres@vagrant-centos65 pg_log]$ ls
postgresql-2015-04-21_232328.log  postgresql-2015-04-21_232858.log
[postgres@vagrant-centos65 pg_log]$ grep error
postgresql-2015-04-21_232858.log
LOG:  syntax error in file "/usr/local/pgsql/data/postgresql.conf" line
211, near token "/"
LOG:  configuration file "/usr/local/pgsql/data/postgresql.conf" contains
errors; no changes were applied

I'm guessing since this is a patch submitted to the commitfest after the
current one, am I too early to start reviewing it?

Payal

Payal Singh,
Database Administrator,
OmniTI Computer Consulting Inc.
Phone: 240.646.0770 x 253

On Thu, Mar 5, 2015 at 4:06 PM, Jim Nasby  wrote:

> On 3/4/15 7:13 PM, Jan de Visser wrote:
>
>> On March 4, 2015 11:08:09 PM Andres Freund wrote:
>>
>>> Let's get the basic feature (notification of failed reloads) done
>>> first. That will be required with or without including the error
>>> message.  Then we can get more fancy later, if somebody really wants to
>>> invest the time.
>>>
>>
>> Except for also fixing pg_reload_conf() to pay attention to what happens
>> with
>> postmaster.pid, the patch I submitted does exactly what you want I think.
>>
>> And I don't mind spending time on stuff like this. I'm not smart enough
>> to deal
>> with actual, you know, database technology.
>>
>
> Yeah, lets at least get this wrapped and we can see about improving it.
>
> I like the idea of doing a here-doc or similar in the .pid, though I think
> it'd be sufficient to just prefix all the continuation lines with a tab. An
> uglier option would be just striping the newlines out.
> --
> Jim Nasby, Data Architect, Blue Treble Consulting
> Data in Trouble? Get it in Treble! http://BlueTreble.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] Freeze avoidance of very large table.

2015-04-21 Thread Jim Nasby


On 4/21/15 3:21 PM, Robert Haas wrote:

It's possible that we could use this infrastructure to freeze more
aggressively in other circumstances.  For example, perhaps VACUUM
should freeze any page it intends to mark all-visible.  That's not a
guaranteed win, because it might increase WAL volume: setting a page
all-visible does not emit an FPI for that page, but freezing any tuple
on it would, if the page hasn't otherwise been modified since the last
checkpoint.  Even if that were no issue, the freezing itself must be
WAL-logged.  But if we could somehow get to a place where all-visible
=> frozen, then autovacuum would never need to visit all-visible
pages, a huge win.


I don't know how bad the extra WAL traffic would be; we'd obviously need 
to incur it eventually, so it's a question of how common it is for a 
page to go all-visible but then go not-all-visible again before 
freezing. It would presumably be far more traffic than some form of a 
FrozenMap though...



We could also attack the problem from the other end.  Instead of
trying to set the bits on the individual tuples, we could decide that
whenever a page is marked all-visible, we regard it as frozen
regardless of the bits set or not set on the individual tuples.
Anybody who wants to modify the page must freeze any unfrozen tuples
"for real" before clearing the visibility map bit.  This would have
the same end result as the previous idea: all-visible would
essentially imply frozen, and autovacuum could ignore those pages
categorically.


Pushing what's currently background work onto foreground processes 
doesn't seem like a good idea...



I'm not saying those ideas don't have problems, because they do.  But
I think they are worth further exploring.  The main reason I gave up
on that is because Heikki was working on the XID-to-LSN mapping stuff.
That seemed like a better approach than either of the above, so as
long as Heikki was working on that, there wasn't much reason to pursue
more lowbrow approaches.  Clearly, though, we need to do something
about this.  Freezing is a big problem for lots of users.


Did XID-LSN die? I see at the bottom of the thread it was returned with 
feedback; I guess Heikki just hasn't had time and there's no major 
blockers? From what I remember this is probably a better solution, but 
if it's not going to make it into 9.6 then we should probably at least 
look further into a FM.



All that having been said, I don't think adding a new fork is a good
approach.  We already have problems pretty commonly where our
customers complain about running out of inodes.  Adding another fork
for every table would exacerbate that problem considerably.


Andres idea of adding this to the VM may work well to handle that. It 
would double the size of the VM, but it would still be a ratio of 
32,000-1 compared to heap size, or 2MB for a 64GB table.

--
Jim Nasby, Data Architect, Blue Treble Consulting
Data in Trouble? Get it in Treble! http://BlueTreble.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] GSoC 2015 proposal: Support for microvacuum for GiST

2015-04-21 Thread Ilia Ivanicki

>
> GSoC should be treated as a full-time job, that's how much time you're
>> expected to dedicate to it. Having bachelor's degree exams in June would be
>> a serious problem. You'll need to discuss with the potential mentors on how
>> to make up for that time.
>>
>
My bachelor's diploma is almost done and I will have enough time for GSoC
work.


> Other than that, the schedule seems fairly relaxed. In fact, this project
> seems a bit too small for a GSoC project. I'd suggest coming up with some
> additional GiST-related work that you could do, in addition to the
> microvacuum thing. Otherwise I think there's a risk that you finish the
> patch in May, and have nothing to do for the rest of the summer.


I want to take additional work-item for gsoc 2015.

I don't known, which item of todo is completed, but I compose list of
items:

1) add support for microvacuum for GIN index in common with Anastasiya
Lubennikova (she will be realize function amgettuple in GIN), if it's a
possible feature.

2) bug with Index on inet changes query result (
http://www.postgresql.org/message-id/flat/201010112055.o9bktzf7011...@wwwmaster.postgresql.org#201010112055.o9bktzf7011...@wwwmaster.postgresql.org
)

3) Teach GIN cost estimation about "fast scans"(I know very little about
GIN, but discussion in mailing list was interesting for me)

4) pg_restore unusable for expensive matviews (
http://www.postgresql.org/message-id/flat/20140820021530.2534.43...@wrigleys.postgresql.org#20140820021530.2534.43...@wrigleys.postgresql.org
)

5) may be community can suggest me such thing with GiST or microvacuum for
GiST will be usefull for all.

Best wishes,
Ivanitskiy Ilya.

[HACKERS] Buffer management improvement wiki page

2015-04-21 Thread Jim Nasby

There's been far more ideas and testing done around improving shared 
buffer management than I can remember, and I suspect I'm not alone in 
that regard. So I've created a wiki page as a place to pull this 
information together. I'll try and keep highlights/important links 
posted there, but help would be welcome. I think that in particular any 
test data would be very useful to post there.


https://wiki.postgresql.org/wiki/Shared_Buffer_Improvements
--
Jim Nasby, Data Architect, Blue Treble Consulting
Data in Trouble? Get it in Treble! http://BlueTreble.com


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

73 matches

Mail list logo