Comments to Synchronous replication patch v3
in [PgSql]
|
From: ITAGAKI Takahiro on 24 Nov 2008 21:42 "Fujii Masao" <masao.fujii(a)gmail.com> wrote: > On Fri, Nov 14, 2008 at 7:15 PM, Fujii Masao <masao.fujii(a)gmail.com> wrote: > > Attached is the latest version of Synch Rep patch. Sorry for my late posting. > > I fixed some bugs in v1patch and integrated walreceiver into core. I have some comments about v3 patch. [1] Should walsender database be real or not? [2] User-configurable replication_timeout is dangerous [3] How to configure wal_buffers and wal_sender_delay? [4] XLogArchivingActive on replication mode [5] Usage of XLog Send-Flush-Wait [6] Bit-OR or Logical-OR ---- [1] Should walsender database be real or not? Index: bin/initdb/initdb.c + "CREATE DATABASE walsender;\n", You create walsender as a *real* database, but is it really needed? Can we make it a virtual database only for walreceiver? And backend process crashes when I login the walsender database: $ psql walsender psql: server closed the connection unexpectedly This probably means the server terminated abnormally before or while processing the request. Even if we need to have the database in real, I'd like to use another name for it. The name 'walsender' seems to be an internal module name but it should be a feature name (ex. 'replication'). ---- [2] User-configurable replication_timeout is dangerous Index: backend/utils/misc/guc.c + {"replication_timeout", PGC_USERSET, WAL_REPLICATION, You export replication_timeout as a PGC_USERSET variable, but it is dangerous. It allows non-superusers to kill servers easily by setting it too low value. Walsender dies with FATAL on timeout. BTW, how about adding an option to choose FATAL or ERROR on timeout? I think FATAL is too strong for some cases. ---- [3] How to configure wal_buffers and wal_sender_delay? The parameter wal_buffers might be more important in synch rep, but we don't have a mean to know whether wal_buffers is enough or not. Should we have a new system view 'pg_stat_xlog' to find shortage of wal_buffers and wal_sender_delay? ---- [4] XLogArchivingActive on replication mode XLogArchivingActive is not modified in the patch, but is it safe? For example, we might need to disable COPY-without-wal optimization when replication is enabled. ---- [5] Usage of XLog Send-Flush-Wait There are many following sequences: RequestXLogSend(WriteRqstPtr, true); XLogFlush(WriteRqstPtr); WaitXLogSend(WriteRqstPtr, false, true); It introduces additional complexities to use XLogFlush. Is it possible to push RequestXLogSend and WaitXLogSend into XLogFlush? It might require a new flag argument in XLogFlush. ---- [6] Bit-OR or Logical-OR WaitXLogSend(XactLastRecEnd, false, forceSyncCommit | haveNonTemp); should be WaitXLogSend(XactLastRecEnd, false, forceSyncCommit || haveNonTemp); | is bit OR and || is logical OR. Regards, --- ITAGAKI Takahiro NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: "Fujii Masao" on 25 Nov 2008 04:03 On Tue, Nov 25, 2008 at 11:42 AM, ITAGAKI Takahiro <itagaki.takahiro(a)oss.ntt.co.jp> wrote: > "Fujii Masao" <masao.fujii(a)gmail.com> wrote: >> On Fri, Nov 14, 2008 at 7:15 PM, Fujii Masao <masao.fujii(a)gmail.com> wrote: >> > Attached is the latest version of Synch Rep patch. Sorry for my late posting. >> > I fixed some bugs in v1patch and integrated walreceiver into core. > > I have some comments about v3 patch. Hi, Itagaki-san. Thanks for taking time to review the patch. > > [1] Should walsender database be real or not? > [2] User-configurable replication_timeout is dangerous > [3] How to configure wal_buffers and wal_sender_delay? > [4] XLogArchivingActive on replication mode > [5] Usage of XLog Send-Flush-Wait > [6] Bit-OR or Logical-OR > > ---- > [1] Should walsender database be real or not? > Index: bin/initdb/initdb.c > + "CREATE DATABASE walsender;\n", > > You create walsender as a *real* database, but is it really needed? > Can we make it a virtual database only for walreceiver? I think that the real database is better, because it's simpler and the user can re-create it easily even if it is lost accidentally. Of course, if we introduce the new API to handle a virtual database, we can virtualize it. Is it worth introducing it? > > And backend process crashes when I login the walsender database: > $ psql walsender > psql: server closed the connection unexpectedly > This probably means the server terminated abnormally > before or while processing the request. Obviously undesirable behavior :( I will fix it. > > Even if we need to have the database in real, I'd like to use another > name for it. The name 'walsender' seems to be an internal module name > but it should be a feature name (ex. 'replication'). Agreed. The name 'replication' is more suitable, I also think. Any other ideas? > > ---- > [2] User-configurable replication_timeout is dangerous > Index: backend/utils/misc/guc.c > + {"replication_timeout", PGC_USERSET, WAL_REPLICATION, > > You export replication_timeout as a PGC_USERSET variable, but it is > dangerous. It allows non-superusers to kill servers easily by setting it > too low value. Walsender dies with FATAL on timeout. > > BTW, how about adding an option to choose FATAL or ERROR on timeout? > I think FATAL is too strong for some cases. OK, I will add the new GUC option for specifying the behavior when the timeout occurs. I think the valid values are ERROR, FATAL and PANIC. * ERROR only cancels that the backend waits for replication. The backends (including the canceled one) and walsender continue processing. But, synchronous replication might be broken. * FATAL terminates walsender. The backends continue processing without replication. * PANIC terminates all processes. In previous discussion, someone wanted this behavior. Or, should I define replication_timeout as PGC_SUSET? > > ---- > [3] How to configure wal_buffers and wal_sender_delay? > The parameter wal_buffers might be more important in synch rep, > but we don't have a mean to know whether wal_buffers is enough or not. > Should we have a new system view 'pg_stat_xlog' to find shortage > of wal_buffers and wal_sender_delay? Couting the number of times which the buffer page is replaced in AdvanceXLInsertBuffer might be help to configure them. Of course, this kind of feature is very useful. But, is it essential for Synch Rep? If not, I'd like to postpone it to 8.5. > > ---- > [4] XLogArchivingActive on replication mode > XLogArchivingActive is not modified in the patch, but is it safe? > For example, we might need to disable COPY-without-wal optimization > when replication is enabled. You are right! I will fix it. > > ---- > [5] Usage of XLog Send-Flush-Wait > There are many following sequences: > RequestXLogSend(WriteRqstPtr, true); > XLogFlush(WriteRqstPtr); > WaitXLogSend(WriteRqstPtr, false, true); > > It introduces additional complexities to use XLogFlush. > Is it possible to push RequestXLogSend and WaitXLogSend into XLogFlush? > It might require a new flag argument in XLogFlush. OK, I will push them into XLogFlush. > > ---- > [6] Bit-OR or Logical-OR > WaitXLogSend(XactLastRecEnd, false, forceSyncCommit | haveNonTemp); > should be > WaitXLogSend(XactLastRecEnd, false, forceSyncCommit || haveNonTemp); > | is bit OR and || is logical OR. Oops! I will fix it. Regards, -- Fujii Masao NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: "Dickson S. Guedes" on 25 Nov 2008 06:38 Fujii Masao escreveu: > (...) >> Even if we need to have the database in real, I'd like to use another >> name for it. The name 'walsender' seems to be an internal module name >> but it should be a feature name (ex. 'replication'). >> > > Agreed. The name 'replication' is more suitable, I also think. > Any other ideas? > 'walsender' should be a schema in the 'replication' database. Other modules, in replication feature, could be placed there too. []s -- Dickson S. Guedes Administrador de Banco de Dados Confesol - Projeto Colmeia Florianopolis, SC, Brasil (48) 3322-1185, ramal: 26 -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Alvaro Herrera on 25 Nov 2008 08:57 Dickson S. Guedes escribi�: > Fujii Masao escreveu: >> (...) >>> Even if we need to have the database in real, I'd like to use another >>> name for it. The name 'walsender' seems to be an internal module name >>> but it should be a feature name (ex. 'replication'). >>> >> >> Agreed. The name 'replication' is more suitable, I also think. >> Any other ideas? > > 'walsender' should be a schema in the 'replication' database. Other > modules, in replication feature, could be placed there too. Hmm, what is this database there for? -- Alvaro Herrera http://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: "Fujii Masao" on 25 Nov 2008 10:17 On Tue, Nov 25, 2008 at 10:57 PM, Alvaro Herrera <alvherre(a)commandprompt.com> wrote: > Dickson S. Guedes escribió: >> Fujii Masao escreveu: >>> (...) >>>> Even if we need to have the database in real, I'd like to use another >>>> name for it. The name 'walsender' seems to be an internal module name >>>> but it should be a feature name (ex. 'replication'). >>>> >>> >>> Agreed. The name 'replication' is more suitable, I also think. >>> Any other ideas? >> >> 'walsender' should be a schema in the 'replication' database. Other >> modules, in replication feature, could be placed there too. > > Hmm, what is this database there for? It's for authentication for replication. This was discussed before. Please see the following thread and feel free to comment. http://archives.postgresql.org/pgsql-hackers/2008-11/msg00187.php Regards, -- Fujii Masao NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
|
Next
|
Last
Pages: 1 2 3 Prev: ERROR: incompatible library Next: combined indexes with Gist - planner issues? |