max_sync_workers_per_subscription is missing in postgresql.conf

Started by Masahiko Sawadaalmost 9 years ago6 messages
#1Masahiko Sawada
sawada.mshk@gmail.com
1 attachment(s)

Hi all,

Attached a patch for $subject.

I added this parameter into "Asynchronous Behavior" section of
"RESOURCE" section. But GUC parameter for subscriber now is written in
this section, in spite of there is "REPLICATION" section. I think that
we can coordinate these parameters to not confuse user. For example in
documentation, these parameters are described in "19.6.4. Subscribers"
section of "19.6. Replication" section. Thought?

Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

Attachments:

Add_max_sync_workers_per_subscription_into_postgresql_conf.patchapplication/octet-stream; name=Add_max_sync_workers_per_subscription_into_postgresql_conf.patchDownload
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index 512be0a..9107c20 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -165,6 +165,7 @@
 #max_parallel_workers = 8	    # maximum number of max_worker_processes that
 					# can be used in parallel queries
 #max_logical_replication_workers = 4	# taken from max_worker_processes
+#max_sync_workers_per_subscription = 2	# taken from max_worker_processes
 #old_snapshot_threshold = -1		# 1min-60d; -1 disables; 0 is immediate
 					# (change requires restart)
 #backend_flush_after = 0		# measured in pages, 0 disables
#2Petr Jelinek
petr.jelinek@2ndquadrant.com
In reply to: Masahiko Sawada (#1)
1 attachment(s)
Re: max_sync_workers_per_subscription is missing in postgresql.conf

On 10/04/17 07:16, Masahiko Sawada wrote:

Hi all,

Attached a patch for $subject.

I added this parameter into "Asynchronous Behavior" section of
"RESOURCE" section. But GUC parameter for subscriber now is written in
this section, in spite of there is "REPLICATION" section. I think that
we can coordinate these parameters to not confuse user. For example in
documentation, these parameters are described in "19.6.4. Subscribers"
section of "19.6. Replication" section. Thought?

Good catch, but it's actually taken from max_logical_replication_workers
so the patch should look more like attached IMHO.

--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Attachments:

Add_max_sync_workers_per_subscription_into_postgresql_conf.patchapplication/x-patch; name=Add_max_sync_workers_per_subscription_into_postgresql_conf.patchDownload
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index 512be0a..9107c20 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -165,6 +165,7 @@
 #max_parallel_workers = 8	    # maximum number of max_worker_processes that
 					# can be used in parallel queries
 #max_logical_replication_workers = 4	# taken from max_worker_processes
+#max_sync_workers_per_subscription = 2	# taken from max_logical_replication_workers
 #old_snapshot_threshold = -1		# 1min-60d; -1 disables; 0 is immediate
 					# (change requires restart)
 #backend_flush_after = 0		# measured in pages, 0 disables
#3Masahiko Sawada
sawada.mshk@gmail.com
In reply to: Petr Jelinek (#2)
Re: max_sync_workers_per_subscription is missing in postgresql.conf

On Mon, Apr 10, 2017 at 9:32 PM, Petr Jelinek
<petr.jelinek@2ndquadrant.com> wrote:

On 10/04/17 07:16, Masahiko Sawada wrote:

Hi all,

Attached a patch for $subject.

I added this parameter into "Asynchronous Behavior" section of
"RESOURCE" section. But GUC parameter for subscriber now is written in
this section, in spite of there is "REPLICATION" section. I think that
we can coordinate these parameters to not confuse user. For example in
documentation, these parameters are described in "19.6.4. Subscribers"
section of "19.6. Replication" section. Thought?

Good catch, but it's actually taken from max_logical_replication_workers
so the patch should look more like attached IMHO.

Thank you for the patch. The patch looks good to me.

Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

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

#4Fujii Masao
masao.fujii@gmail.com
In reply to: Masahiko Sawada (#3)
1 attachment(s)
Re: max_sync_workers_per_subscription is missing in postgresql.conf

On Mon, Apr 10, 2017 at 9:39 PM, Masahiko Sawada <sawada.mshk@gmail.com> wrote:

On Mon, Apr 10, 2017 at 9:32 PM, Petr Jelinek
<petr.jelinek@2ndquadrant.com> wrote:

On 10/04/17 07:16, Masahiko Sawada wrote:

Hi all,

Attached a patch for $subject.

I added this parameter into "Asynchronous Behavior" section of
"RESOURCE" section. But GUC parameter for subscriber now is written in
this section, in spite of there is "REPLICATION" section. I think that
we can coordinate these parameters to not confuse user. For example in
documentation, these parameters are described in "19.6.4. Subscribers"
section of "19.6. Replication" section. Thought?

Yes, I think that we should not only add the parameter into
postgresql.conf.sample
but also

- add REPLICATION_SUBSCRIBERS into config_group
- mark max_logical_replication_workers and max_sync_workers_per_subscription
as REPLICATION_SUBSCRIBERS parameters, in guc.c
- move those parameters into "Subscribers" section in postgresql.conf.sample

The attached patch does these.

Regards,

--
Fujii Masao

Attachments:

Add_max_sync_workers_per_subscription_into_postgresql_conf_v3.patchapplication/octet-stream; name=Add_max_sync_workers_per_subscription_into_postgresql_conf_v3.patchDownload
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 19d258d..b2bc835 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -610,6 +610,8 @@ const char *const config_group_names[] =
 	gettext_noop("Replication / Master Server"),
 	/* REPLICATION_STANDBY */
 	gettext_noop("Replication / Standby Servers"),
+	/* REPLICATION_SUBSCRIBERS */
+	gettext_noop("Replication / Subscribers"),
 	/* QUERY_TUNING */
 	gettext_noop("Query Tuning"),
 	/* QUERY_TUNING_METHOD */
@@ -2511,7 +2513,7 @@ static struct config_int ConfigureNamesInt[] =
 	{
 		{"max_logical_replication_workers",
 			PGC_POSTMASTER,
-			RESOURCES_ASYNCHRONOUS,
+			REPLICATION_SUBSCRIBERS,
 			gettext_noop("Maximum number of logical replication worker processes."),
 			NULL,
 		},
@@ -2523,7 +2525,7 @@ static struct config_int ConfigureNamesInt[] =
 	{
 		{"max_sync_workers_per_subscription",
 			PGC_SIGHUP,
-			RESOURCES_ASYNCHRONOUS,
+			REPLICATION_SUBSCRIBERS,
 			gettext_noop("Maximum number of table synchronization workers per subscription."),
 			NULL,
 		},
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index 512be0a..1435d92 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -164,7 +164,6 @@
 #max_parallel_workers_per_gather = 2	# taken from max_parallel_workers
 #max_parallel_workers = 8	    # maximum number of max_worker_processes that
 					# can be used in parallel queries
-#max_logical_replication_workers = 4	# taken from max_worker_processes
 #old_snapshot_threshold = -1		# 1min-60d; -1 disables; 0 is immediate
 					# (change requires restart)
 #backend_flush_after = 0		# measured in pages, 0 disables
@@ -273,6 +272,13 @@
 #wal_retrieve_retry_interval = 5s	# time to wait before retrying to
 					# retrieve WAL after a failed attempt
 
+# - Subscribers -
+
+# These settings are ignored on a publisher.
+
+#max_logical_replication_workers = 4	# taken from max_worker_processes
+#max_sync_workers_per_subscription = 2	# taken from max_logical_replication_workers
+
 
 #------------------------------------------------------------------------------
 # QUERY TUNING
diff --git a/src/include/utils/guc_tables.h b/src/include/utils/guc_tables.h
index 2da9115..91c6000 100644
--- a/src/include/utils/guc_tables.h
+++ b/src/include/utils/guc_tables.h
@@ -72,6 +72,7 @@ enum config_group
 	REPLICATION_SENDING,
 	REPLICATION_MASTER,
 	REPLICATION_STANDBY,
+	REPLICATION_SUBSCRIBERS,
 	QUERY_TUNING,
 	QUERY_TUNING_METHOD,
 	QUERY_TUNING_COST,
#5Masahiko Sawada
sawada.mshk@gmail.com
In reply to: Fujii Masao (#4)
Re: max_sync_workers_per_subscription is missing in postgresql.conf

On Tue, Apr 11, 2017 at 1:39 AM, Fujii Masao <masao.fujii@gmail.com> wrote:

On Mon, Apr 10, 2017 at 9:39 PM, Masahiko Sawada <sawada.mshk@gmail.com> wrote:

On Mon, Apr 10, 2017 at 9:32 PM, Petr Jelinek
<petr.jelinek@2ndquadrant.com> wrote:

On 10/04/17 07:16, Masahiko Sawada wrote:

Hi all,

Attached a patch for $subject.

I added this parameter into "Asynchronous Behavior" section of
"RESOURCE" section. But GUC parameter for subscriber now is written in
this section, in spite of there is "REPLICATION" section. I think that
we can coordinate these parameters to not confuse user. For example in
documentation, these parameters are described in "19.6.4. Subscribers"
section of "19.6. Replication" section. Thought?

Yes, I think that we should not only add the parameter into
postgresql.conf.sample
but also

- add REPLICATION_SUBSCRIBERS into config_group
- mark max_logical_replication_workers and max_sync_workers_per_subscription
as REPLICATION_SUBSCRIBERS parameters, in guc.c
- move those parameters into "Subscribers" section in postgresql.conf.sample

+1

The attached patch does these.

The patch looks good to me.

Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

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

#6Fujii Masao
masao.fujii@gmail.com
In reply to: Masahiko Sawada (#5)
Re: max_sync_workers_per_subscription is missing in postgresql.conf

On Tue, Apr 11, 2017 at 4:50 PM, Masahiko Sawada <sawada.mshk@gmail.com> wrote:

On Tue, Apr 11, 2017 at 1:39 AM, Fujii Masao <masao.fujii@gmail.com> wrote:

On Mon, Apr 10, 2017 at 9:39 PM, Masahiko Sawada <sawada.mshk@gmail.com> wrote:

On Mon, Apr 10, 2017 at 9:32 PM, Petr Jelinek
<petr.jelinek@2ndquadrant.com> wrote:

On 10/04/17 07:16, Masahiko Sawada wrote:

Hi all,

Attached a patch for $subject.

I added this parameter into "Asynchronous Behavior" section of
"RESOURCE" section. But GUC parameter for subscriber now is written in
this section, in spite of there is "REPLICATION" section. I think that
we can coordinate these parameters to not confuse user. For example in
documentation, these parameters are described in "19.6.4. Subscribers"
section of "19.6. Replication" section. Thought?

Yes, I think that we should not only add the parameter into
postgresql.conf.sample
but also

- add REPLICATION_SUBSCRIBERS into config_group
- mark max_logical_replication_workers and max_sync_workers_per_subscription
as REPLICATION_SUBSCRIBERS parameters, in guc.c
- move those parameters into "Subscribers" section in postgresql.conf.sample

+1

The attached patch does these.

The patch looks good to me.

Pushed. Thanks!

Regards,

--
Fujii Masao

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