| From: | shveta malik <shveta(dot)malik(at)gmail(dot)com> |
|---|---|
| To: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Ashutosh Sharma <ashu(dot)coek88(at)gmail(dot)com> |
| Cc: | "Zhijie Hou (Fujitsu)" <houzj(dot)fnst(at)fujitsu(dot)com>, Ajin Cherian <itsajin(at)gmail(dot)com>, SATYANARAYANA NARLAPURAM <satyanarlapuram(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, shveta malik <shveta(dot)malik(at)gmail(dot)com> |
| Subject: | Re: synchronized_standby_slots behavior inconsistent with quorum-based synchronous replication |
| Date: | 2026-06-10 06:45:52 |
| Message-ID: | CAJpy0uBR-8-d__LgfqqUbFvLBvzL6bY_kjUmBWAVsLLu1QZEPA@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Please find a few comments on June8 version fo patches:
1)
patch001:
SYNC_REP_DEFAULT: do we need to give one-line comment for this
somewhere as unlike PRIORITY and QUORUM, it is not self-explanatory.
patch002:
2)
It is better to avoid mentioning it as 'synchronized standby slots'.
We can make it as 'synchronized_standby_slots' in below comments:
+ /* Parse the synchronized standby slots configuration */
+ * Report problem states for synchronized standby slots that prevented the
3)
For these error-messages too, we need to mention GUC name to give
better clarity.
+ GUC_check_errmsg("number of synchronized standby slots (%d) must not
exceed the number of unique listed slots (%d)",
+ syncrep_parse_result->num_sync,
+ syncrep_parse_result->nmembers);
How about:
GUC_check_errcode(ERRCODE_INVALID_PARAMETER_VALUE);
GUC_check_errmsg("invalid value for parameter \"%s\: synchronization
requirement (%d) exceeds the number of unique listed slots (%d)",
"synchronized_standby_slots",
syncrep_parse_result->num_sync,
syncrep_parse_result->nmembers);
Or
GUC_check_errmsg("invalid value for parameter \"%s\: required number
of synchronized standby slots (%d) exceeds the number of unique listed
slots (%d)",
"synchronized_standby_slots",
syncrep_parse_result->num_sync,
syncrep_parse_result->nmembers);
3)
+ GUC_check_errmsg("number of synchronized standby slots (%d) must be
greater than zero",
+ syncrep_parse_result->num_sync)
How about:
GUC_check_errcode(ERRCODE_INVALID_PARAMETER_VALUE);
GUC_check_errmsg("invalid value for parameter \"%s\: required number
of synchronized standby slots (%d) must be greater than zero",
"synchronized_standby_slots",
syncrep_parse_result->num_sync);
---
Or, better yet, we can split the messages and details for both, example:
GUC_check_errcode(ERRCODE_INVALID_PARAMETER_VALUE);
GUC_check_errmsg("invalid value for parameter \"%s\",
"synchronized_standby_slots");
GUC_check_errdetail("The required number of synchronized standby slots
(%d) exceeds the number of unique listed slots (%d)",
syncrep_parse_result->num_sync,
syncrep_parse_result->nmembers);)
4)
+ReportUnavailableSyncStandbySlots(SyncStandbySlotsStateInfo * slot_states
We can get rid of space before slot_states. I think pgindent might
have put it in my patch. Sorry for the trouble.
5)
003:
- wait_for_all = (required == synchronized_standby_slots_config->nslotnames);
+ wait_for_all =
+ (synchronized_standby_slots_config->syncrep_method == SYNC_REP_DEFAULT);
This change can be moved to 002, right?
thanks
Shveta
| From | Date | Subject | |
|---|---|---|---|
| Next Message | shveta malik | 2026-06-10 06:51:04 | Re: DOCS - Add missing EXCEPT parameter description to ALTER PUBLICATION |
| Previous Message | Nisha Moond | 2026-06-10 06:45:02 | Re: DOCS - Add missing EXCEPT parameter description to ALTER PUBLICATION |