Skip site navigation (1) Skip section navigation (2)

Re: Broken hint bits (freeze)

From: Bruce Momjian <bruce(at)momjian(dot)us>
To: Sergey Burladyan <eshkinkot(at)gmail(dot)com>
Cc: Vladimir Borodin <root(at)simply(dot)name>,Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>,Dmitriy Sarafannikov <dsarafannikov(at)yandex(dot)ru>,pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Broken hint bits (freeze)
Date: 2017-06-20 17:18:44
Message-ID: 20170620171844.GC24975@momjian.us (view raw, whole thread or download thread mbox)
Thread:
Lists: pgsql-hackers
Sorry, this email from June 16 didn't make it to the lists for some odd
reason so I am reposting it now.  I will apply a patch based on this
email shortly.

What is really odd is that I replied to this email already but the
original wasn't posted.  I think it was something about my email reader.

---------------------------------------------------------------------------

On Fri, Jun 16, 2017 at 10:57:33PM +0300, Sergey Burladyan wrote:
> Bruce Momjian <bruce(at)momjian(dot)us> writes:
> > On Fri, Jun 16, 2017 at 04:33:16AM +0300, Sergey Burladyan wrote:
> > > Bruce Momjian <bruce(at)momjian(dot)us> writes:
> > The way pg_upgrade uses rsync, the standby never needs to replay the WAL
> > when it starts up because we already copied the changed system tables
> > and hard linked the user data files.
> 
> Oh, it is my fail, I was not run test script completely for current git
> master. In git master it work as expected. But not in previous versions.
> I used this test script and got this result:
> 9.2 -> master: wal_level setting:                    replica
> 9.2 -> 9.6: wal_level setting:                    minimal
> 9.2 -> 9.5: wal_level setting:                    minimal
> 9.2 -> 9.4: Current wal_level setting:            minimal

Wow, thank you again for your excellent research.

> >From git master pg_upgrade is restart new master again after
> pg_resetwal -o, as you said.
> 
> It is from src/bin/pg_upgrade/check.c:176
> void
> issue_warnings(void)
> {
>     /* Create dummy large object permissions for old < PG 9.0? */
>     if (GET_MAJOR_VERSION(old_cluster.major_version) <= 804)
>     {
>         start_postmaster(&new_cluster, true);
>         new_9_0_populate_pg_largeobject_metadata(&new_cluster, false);
>         stop_postmaster(false);
>     }
> 
>     /* Reindex hash indexes for old < 10.0 */
>     if (GET_MAJOR_VERSION(old_cluster.major_version) <= 906)
>     {
>         start_postmaster(&new_cluster, true);
>         old_9_6_invalidate_hash_indexes(&new_cluster, false);
>         stop_postmaster(false);
>     }
> }

Yes, that is _exactly_ the right place to look.  Only in PG 10 do we
restart the new cluster to invalidate hash indexes.  In previous
releases we didn't do the restart.

That didn't matter with the old rsync instructions, but now that we have
removed the start/stop before rsync step, the final WAL status of
pg_upgrade matters.

I suggest applying the attached patch 

-- 
  Bruce Momjian  <bruce(at)momjian(dot)us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+                      Ancient Roman grave inscription +

--2oS5YaxWCcQjTEyO
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="wal.diff"

diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.c
new file mode 100644
index 8b9e81e..b79e54a
*** a/src/bin/pg_upgrade/check.c
--- b/src/bin/pg_upgrade/check.c
*************** report_clusters_compatible(void)
*** 174,196 ****
  
  
  void
! issue_warnings(void)
  {
  	/* Create dummy large object permissions for old < PG 9.0? */
  	if (GET_MAJOR_VERSION(old_cluster.major_version) <= 804)
- 	{
- 		start_postmaster(&new_cluster, true);
  		new_9_0_populate_pg_largeobject_metadata(&new_cluster, false);
- 		stop_postmaster(false);
- 	}
  
  	/* Reindex hash indexes for old < 10.0 */
  	if (GET_MAJOR_VERSION(old_cluster.major_version) <= 906)
- 	{
- 		start_postmaster(&new_cluster, true);
  		old_9_6_invalidate_hash_indexes(&new_cluster, false);
! 		stop_postmaster(false);
! 	}
  }
  
  
--- 174,198 ----
  
  
  void
! issue_warnings_and_set_wal_level(void)
  {
+ 	/*
+ 	 * We unconditionally start/stop the new server because pg_resetwal -o
+ 	 * set wal_level to 'minimum'.  If the user is upgrading standby
+ 	 * servers using the rsync instructions, they will need pg_upgrade
+ 	 * to write its final WAL record showing wal_level as 'replica'.
+ 	 */
+ 	start_postmaster(&new_cluster, true);
+ 
  	/* Create dummy large object permissions for old < PG 9.0? */
  	if (GET_MAJOR_VERSION(old_cluster.major_version) <= 804)
  		new_9_0_populate_pg_largeobject_metadata(&new_cluster, false);
  
  	/* Reindex hash indexes for old < 10.0 */
  	if (GET_MAJOR_VERSION(old_cluster.major_version) <= 906)
  		old_9_6_invalidate_hash_indexes(&new_cluster, false);
! 
! 	stop_postmaster(false);
  }
  
  
diff --git a/src/bin/pg_upgrade/pg_upgrade.c b/src/bin/pg_upgrade/pg_upgrade.c
new file mode 100644
index ca1aa5c..2a9c397
*** a/src/bin/pg_upgrade/pg_upgrade.c
--- b/src/bin/pg_upgrade/pg_upgrade.c
*************** main(int argc, char **argv)
*** 162,168 ****
  	create_script_for_cluster_analyze(&analyze_script_file_name);
  	create_script_for_old_cluster_deletion(&deletion_script_file_name);
  
! 	issue_warnings();
  
  	pg_log(PG_REPORT, "\nUpgrade Complete\n");
  	pg_log(PG_REPORT, "----------------\n");
--- 162,168 ----
  	create_script_for_cluster_analyze(&analyze_script_file_name);
  	create_script_for_old_cluster_deletion(&deletion_script_file_name);
  
! 	issue_warnings_and_set_wal_level();
  
  	pg_log(PG_REPORT, "\nUpgrade Complete\n");
  	pg_log(PG_REPORT, "----------------\n");
diff --git a/src/bin/pg_upgrade/pg_upgrade.h b/src/bin/pg_upgrade/pg_upgrade.h
new file mode 100644
index 8fbf8ac..e3a577a
*** a/src/bin/pg_upgrade/pg_upgrade.h
--- b/src/bin/pg_upgrade/pg_upgrade.h
*************** void		output_check_banner(bool live_chec
*** 332,338 ****
  void		check_and_dump_old_cluster(bool live_check);
  void		check_new_cluster(void);
  void		report_clusters_compatible(void);
! void		issue_warnings(void);
  void output_completion_banner(char *analyze_script_file_name,
  						 char *deletion_script_file_name);
  void		check_cluster_versions(void);
--- 332,338 ----
  void		check_and_dump_old_cluster(bool live_check);
  void		check_new_cluster(void);
  void		report_clusters_compatible(void);
! void		issue_warnings_and_set_wal_level(void);
  void output_completion_banner(char *analyze_script_file_name,
  						 char *deletion_script_file_name);
  void		check_cluster_versions(void);

--2oS5YaxWCcQjTEyO--


-- 
  Bruce Momjian  <bruce(at)momjian(dot)us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+                      Ancient Roman grave inscription +


In response to

pgsql-hackers by date

Next:From: Tom LaneDate: 2017-06-20 17:43:53
Subject: Re: PATCH: Don't downcase filepath/filename while loading libraries
Previous:From: Peter EisentrautDate: 2017-06-20 16:43:08
Subject: Re: Something is rotten in publication drop

Privacy Policy | About PostgreSQL
Copyright © 1996-2017 The PostgreSQL Global Development Group