Thread: Re: [HACKERS] read-only database
Tom Lane wrote: > I'd view this as a postmaster state that propagates to backends. > Probably you'd enable it by means of a postmaster option, and the > only way to get out of it is to shut down and restart the postmaster > without the option. I've created a patch to make a postmaster read-only. (attached patch can be applied to 8.0.1) Read-only state can be enabled/disabled by the postmaster option, or the postgresql.conf option. If you start the postmaster with "-r" options, the cluster will go to read-only. % pg_ctl -o "-i -r" -D $PGDATA start Or if you set "readonly_cluster = true" in the postgresql.conf, the cluster will also become read-only. Any comments? -- NAGAYASU Satoshi <nagayasus@nttdata.co.jp> OpenSource Development Center, NTT DATA Corp. http://www.nttdata.co.jp diff -ru postgresql-8.0.1.orig/src/backend/executor/execMain.c postgresql-8.0.1/src/backend/executor/execMain.c --- postgresql-8.0.1.orig/src/backend/executor/execMain.c 2005-01-15 02:53:33.000000000 +0900 +++ postgresql-8.0.1/src/backend/executor/execMain.c 2005-03-21 13:12:22.000000000 +0900 @@ -43,6 +43,7 @@ #include "optimizer/clauses.h" #include "optimizer/var.h" #include "parser/parsetree.h" +#include "postmaster/postmaster.h" #include "utils/acl.h" #include "utils/guc.h" #include "utils/lsyscache.h" @@ -127,7 +128,7 @@ * If the transaction is read-only, we need to check if any writes are * planned to non-temporary tables. */ - if (XactReadOnly && !explainOnly) + if ( (XactReadOnly || ReadOnlyCluster) && !explainOnly) ExecCheckXactReadOnly(queryDesc->parsetree); /* diff -ru postgresql-8.0.1.orig/src/backend/postmaster/postmaster.c postgresql-8.0.1/src/backend/postmaster/postmaster.c --- postgresql-8.0.1.orig/src/backend/postmaster/postmaster.c 2005-01-13 01:38:17.000000000 +0900 +++ postgresql-8.0.1/src/backend/postmaster/postmaster.c 2005-03-21 13:21:17.000000000 +0900 @@ -236,6 +236,8 @@ extern int optreset; #endif +bool ReadOnlyCluster = false; + /* * postmaster.c - function prototypes */ @@ -440,7 +442,7 @@ opterr = 1; - while ((opt = getopt(argc, argv, "A:a:B:b:c:D:d:Fh:ik:lm:MN:no:p:Ss-:")) != -1) + while ((opt = getopt(argc, argv, "A:a:B:b:c:D:d:Fh:ik:lm:MN:no:p:rSs-:")) != -1) { switch (opt) { @@ -515,6 +517,9 @@ case 'p': SetConfigOption("port", optarg, PGC_POSTMASTER, PGC_S_ARGV); break; + case 'r': + SetConfigOption("readonly_cluster", "true", PGC_POSTMASTER, PGC_S_ARGV); + break; case 'S': /* diff -ru postgresql-8.0.1.orig/src/backend/tcop/utility.c postgresql-8.0.1/src/backend/tcop/utility.c --- postgresql-8.0.1.orig/src/backend/tcop/utility.c 2005-01-25 02:46:29.000000000 +0900 +++ postgresql-8.0.1/src/backend/tcop/utility.c 2005-03-21 13:13:45.000000000 +0900 @@ -47,6 +47,7 @@ #include "parser/parse_expr.h" #include "parser/parse_type.h" #include "postmaster/bgwriter.h" +#include "postmaster/postmaster.h" #include "rewrite/rewriteDefine.h" #include "rewrite/rewriteRemove.h" #include "storage/fd.h" @@ -265,7 +266,7 @@ static void check_xact_readonly(Node *parsetree) { - if (!XactReadOnly) + if (!XactReadOnly && !ReadOnlyCluster) return; /* diff -ru postgresql-8.0.1.orig/src/backend/utils/misc/guc.c postgresql-8.0.1/src/backend/utils/misc/guc.c --- postgresql-8.0.1.orig/src/backend/utils/misc/guc.c 2005-01-01 14:43:08.000000000 +0900 +++ postgresql-8.0.1/src/backend/utils/misc/guc.c 2005-03-21 13:06:42.000000000 +0900 @@ -851,6 +851,15 @@ #endif }, + { + {"readonly_cluster", PGC_POSTMASTER, UNGROUPED, + gettext_noop("Enables the postmaster read-only."), + NULL + }, + &ReadOnlyCluster, + false, NULL, NULL + }, + /* End-of-list marker */ { {NULL, 0, 0, NULL, NULL}, NULL, false, NULL, NULL diff -ru postgresql-8.0.1.orig/src/include/postmaster/postmaster.h postgresql-8.0.1/src/include/postmaster/postmaster.h --- postgresql-8.0.1.orig/src/include/postmaster/postmaster.h 2005-01-01 07:03:39.000000000 +0900 +++ postgresql-8.0.1/src/include/postmaster/postmaster.h 2005-03-21 13:03:16.000000000 +0900 @@ -34,6 +34,7 @@ extern HANDLE PostmasterHandle; #endif +extern bool ReadOnlyCluster; extern int PostmasterMain(int argc, char *argv[]); extern void ClosePostmasterPorts(bool am_syslogger);
Satoshi Nagayasu wrote:
>
> Tom Lane wrote:
> > I'd view this as a postmaster state that propagates to backends.
> > Probably you'd enable it by means of a postmaster option, and the
> > only way to get out of it is to shut down and restart the postmaster
> > without the option.
>
> I've created a patch to make a postmaster read-only.
> (attached patch can be applied to 8.0.1)
>
> Read-only state can be enabled/disabled by the postmaster option,
> or the postgresql.conf option.
>
> If you start the postmaster with "-r" options,
> the cluster will go to read-only.
>
> % pg_ctl -o "-i -r" -D $PGDATA start
>
> Or if you set "readonly_cluster = true" in the postgresql.conf,
> the cluster will also become read-only.
Nice idea. I have attached a new patch which has a few adjustments.
First, we are moving away from using postmaster flags, and instead
encouraging people to use postgresql.conf, so I removed the -r flag but
added an entry in postgresql.conf for this. I can see why it might be
nice to have it as a postmaster flag, but at a certain point the number
of flags gets too confusing so postgresql.conf is better. Second, I
changed it so it can be modified by a sighup to the postmaster, which
can't be done with a postmaster flag.
Also, I renamed it to server_read_only because that seems more
consistent than readonly_cluster.
Also, I added documentation for this postgresql.conf variable.
With this change, I see we now have three read_only options:
transaction_read_only
default_transaction_read_only
server_read_only
The first one is not documented (should it be?) and I assume allows you
to query and change the READ ONLY status of a single transaction, while
default_transaction_read_only affects all new transactions for the
session, and server_read_only is for all transactions on the server.
It seems server_read_only is the same as default_transaction_read_only
except it can't be changed. It seems more like a secure version of
default_transaction_read_only rather than something new.
If we set default_transaction_read_only to true in postgresql.conf,
could we just prevent that from being changed by a session. As I
remember we have abandoned the idea of trying to limit session changes
to postgresql.conf values so maybe this is the way we have to go.
--
Bruce Momjian | http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073
Index: doc/src/sgml/runtime.sgml
===================================================================
RCS file: /cvsroot/pgsql/doc/src/sgml/runtime.sgml,v
retrieving revision 1.315
diff -c -c -r1.315 runtime.sgml
*** doc/src/sgml/runtime.sgml 23 Apr 2005 03:27:40 -0000 1.315
--- doc/src/sgml/runtime.sgml 6 May 2005 22:48:41 -0000
***************
*** 3224,3229 ****
--- 3224,3247 ----
</listitem>
</varlistentry>
+ <varlistentry id="guc-server-read-only" xreflabel="server_read_only">
+ <indexterm>
+ <primary>force read-only transaction</primary>
+ </indexterm>
+ <indexterm>
+ <primary><varname>server_read_only</> configuration parameter</primary>
+ </indexterm>
+
+ <term><varname>server_read_only</varname> (<type>boolean</type>)</term>
+ <listitem>
+ <para>
+ This parameter behaves just like <varname>default_read_only</>
+ except it can only be set from <filename>postgresql.conf</> and is server-wide.
+ The default is false (read/write).
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry id="guc-statement-timeout" xreflabel="statement_timeout">
<term><varname>statement_timeout</varname> (<type>integer</type>)</term>
<indexterm>
Index: src/backend/executor/execMain.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/executor/execMain.c,v
retrieving revision 1.248
diff -c -c -r1.248 execMain.c
*** src/backend/executor/execMain.c 6 May 2005 17:24:53 -0000 1.248
--- src/backend/executor/execMain.c 6 May 2005 22:48:42 -0000
***************
*** 130,136 ****
* If the transaction is read-only, we need to check if any writes are
* planned to non-temporary tables.
*/
! if (XactReadOnly && !explainOnly)
ExecCheckXactReadOnly(queryDesc->parsetree);
/*
--- 130,136 ----
* If the transaction is read-only, we need to check if any writes are
* planned to non-temporary tables.
*/
! if ( (XactReadOnly || ServerReadOnly) && !explainOnly)
ExecCheckXactReadOnly(queryDesc->parsetree);
/*
Index: src/backend/tcop/utility.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/tcop/utility.c,v
retrieving revision 1.236
diff -c -c -r1.236 utility.c
*** src/backend/tcop/utility.c 28 Apr 2005 21:47:15 -0000 1.236
--- src/backend/tcop/utility.c 6 May 2005 22:48:45 -0000
***************
*** 265,271 ****
static void
check_xact_readonly(Node *parsetree)
{
! if (!XactReadOnly)
return;
/*
--- 265,271 ----
static void
check_xact_readonly(Node *parsetree)
{
! if (!XactReadOnly && !ServerReadOnly)
return;
/*
Index: src/backend/utils/misc/guc.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/utils/misc/guc.c,v
retrieving revision 1.261
diff -c -c -r1.261 guc.c
*** src/backend/utils/misc/guc.c 1 May 2005 18:56:19 -0000 1.261
--- src/backend/utils/misc/guc.c 6 May 2005 22:48:48 -0000
***************
*** 142,147 ****
--- 142,149 ----
bool default_with_oids = false;
+ bool ServerReadOnly = false;
+
int log_min_error_statement = PANIC;
int log_min_messages = NOTICE;
int client_min_messages = NOTICE;
***************
*** 794,799 ****
--- 796,809 ----
false, assign_transaction_read_only, NULL
},
{
+ {"server_read_only", PGC_SIGHUP, CLIENT_CONN_STATEMENT,
+ gettext_noop("Forces all transactions to be read-only."),
+ NULL
+ },
+ &ServerReadOnly,
+ false, NULL, NULL
+ },
+ {
{"add_missing_from", PGC_USERSET, COMPAT_OPTIONS_PREVIOUS,
gettext_noop("Automatically adds missing table references to FROM clauses."),
NULL
Index: src/backend/utils/misc/postgresql.conf.sample
===================================================================
RCS file: /cvsroot/pgsql/src/backend/utils/misc/postgresql.conf.sample,v
retrieving revision 1.140
diff -c -c -r1.140 postgresql.conf.sample
*** src/backend/utils/misc/postgresql.conf.sample 21 Apr 2005 19:18:13 -0000 1.140
--- src/backend/utils/misc/postgresql.conf.sample 6 May 2005 22:48:48 -0000
***************
*** 278,283 ****
--- 278,284 ----
#check_function_bodies = true
#default_transaction_isolation = 'read committed'
#default_transaction_read_only = false
+ #server_read_only = false
#statement_timeout = 0 # 0 is disabled, in milliseconds
# - Locale and Formatting -
Index: src/include/postmaster/postmaster.h
===================================================================
RCS file: /cvsroot/pgsql/src/include/postmaster/postmaster.h,v
retrieving revision 1.9
diff -c -c -r1.9 postmaster.h
*** src/include/postmaster/postmaster.h 31 Dec 2004 22:03:39 -0000 1.9
--- src/include/postmaster/postmaster.h 6 May 2005 22:48:49 -0000
***************
*** 34,40 ****
extern HANDLE PostmasterHandle;
#endif
-
extern int PostmasterMain(int argc, char *argv[]);
extern void ClosePostmasterPorts(bool am_syslogger);
--- 34,39 ----
Index: src/include/utils/guc.h
===================================================================
RCS file: /cvsroot/pgsql/src/include/utils/guc.h,v
retrieving revision 1.60
diff -c -c -r1.60 guc.h
*** src/include/utils/guc.h 25 Mar 2005 16:17:28 -0000 1.60
--- src/include/utils/guc.h 6 May 2005 22:48:49 -0000
***************
*** 121,126 ****
--- 121,128 ----
extern bool default_with_oids;
+ extern bool ServerReadOnly;
+
extern int log_min_error_statement;
extern int log_min_messages;
extern int client_min_messages;
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> It seems server_read_only is the same as default_transaction_read_only
> except it can't be changed.
I thought the TODO item was for a low-level read-only option, suitable
for trying to look at a corrupted database or run off a read-only volume.
This is very far from being that --- it allows temp table creation/use,
and it still eats transaction IDs so it is certainly not read-only to
xlog or clog.
I am not sure I see any use case for this implementation: it is
read-only enough to get in your way, without being read-only enough
to derive any real benefit.
regards, tom lane
Tom Lane wrote: > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > It seems server_read_only is the same as default_transaction_read_only > > except it can't be changed. > > I thought the TODO item was for a low-level read-only option, suitable > for trying to look at a corrupted database or run off a read-only volume. > This is very far from being that --- it allows temp table creation/use, > and it still eats transaction IDs so it is certainly not read-only to > xlog or clog. > > I am not sure I see any use case for this implementation: it is > read-only enough to get in your way, without being read-only enough > to derive any real benefit. I am not sure I see the use case either but I developed it so everyone could look at it and decide if it is useful. When true, it is basically a unchangable default_transaction_read_only. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073
I think the read-only has two meanings for the user. First is the internal state. XID, OID or something like that. In these cases, the internal state mustn't be changed. Some users will need the read-only for internal state. Second is read-only for the user data contents. In some cases, the user want to make the user data as read-only. For this purpose, the user doesn't care XID or OID, I guess. So, we can implement them in different way. I think both are necessary. Bruce Momjian wrote: > Tom Lane wrote: > >>Bruce Momjian <pgman@candle.pha.pa.us> writes: >> >>>It seems server_read_only is the same as default_transaction_read_only >>>except it can't be changed. >> >>I thought the TODO item was for a low-level read-only option, suitable >>for trying to look at a corrupted database or run off a read-only volume. >>This is very far from being that --- it allows temp table creation/use, >>and it still eats transaction IDs so it is certainly not read-only to >>xlog or clog. >> >>I am not sure I see any use case for this implementation: it is >>read-only enough to get in your way, without being read-only enough >>to derive any real benefit. > > > I am not sure I see the use case either but I developed it so everyone > could look at it and decide if it is useful. When true, it is basically > a unchangable default_transaction_read_only. > -- NAGAYASU Satoshi <nagayasus@nttdata.co.jp> OpenSource Development Center, NTT DATA Corp. http://www.nttdata.co.jp/
Satoshi Nagayasu <nagayasus@nttdata.co.jp> writes:
> I think the read-only has two meanings for the user.
> First is the internal state. XID, OID or something like that.
> In these cases, the internal state mustn't be changed.
> Some users will need the read-only for internal state.
> Second is read-only for the user data contents.
> In some cases, the user want to make the user data as read-only.
> For this purpose, the user doesn't care XID or OID, I guess.
> So, we can implement them in different way.
> I think both are necessary.
Indeed, but we already have a implementation of the second form, in
a reasonably spec-compliant fashion. The TODO item concerns the first
form, which is something that the current system cannot do at all.
regards, tom lane
On Mon, May 09, 2005 at 09:02:07AM +0900, Satoshi Nagayasu wrote: > I think the read-only has two meanings for the user. > > First is the internal state. XID, OID or something like that. > In these cases, the internal state mustn't be changed. > Some users will need the read-only for internal state. > > Second is read-only for the user data contents. > In some cases, the user want to make the user data as read-only. > For this purpose, the user doesn't care XID or OID, I guess. > > So, we can implement them in different way. > I think both are necessary. But the second is only a subset of the first, no? So why not just implement the first? Put another way, why do you think the second is necessary? -- Alvaro Herrera (<alvherre[@]dcc.uchile.cl>) Thou shalt check the array bounds of all strings (indeed, all arrays), for surely where thou typest "foo" someone someday shall type "supercalifragilisticexpialidocious" (5th Commandment for C programmers)
> But the second is only a subset of the first, no? So why not just > implement the first? Put another way, why do you think the second is > necessary? Because there is "default_transaction_read_only" option and implementation. My implementation is an extension of the existing option. I wanted to make the postmaster read-only, and found "default_transaction_read_only" option, but it can be overwritten. -- NAGAYASU Satoshi <nagayasus@nttdata.co.jp> OpenSource Development Center, NTT DATA Corp. http://www.nttdata.co.jp/
Satoshi Nagayasu wrote: > I wanted to make the postmaster read-only, and found > "default_transaction_read_only" option, but it can be overwritten. I mean it can be overridden by the user. I don't want that. -- NAGAYASU Satoshi <nagayasus@nttdata.co.jp> OpenSource Development Center, NTT DATA Corp. http://www.nttdata.co.jp/
Satoshi Nagayasu wrote: > > Satoshi Nagayasu wrote: > > I wanted to make the postmaster read-only, and found > > "default_transaction_read_only" option, but it can be overwritten. > > I mean it can be overridden by the user. I don't want that. I understand, but we haven't gotten enough requests from people for a new option that can't be overridden. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073
On Mon, May 09, 2005 at 10:13:22PM -0400, Bruce Momjian wrote: > Satoshi Nagayasu wrote: > > > > Satoshi Nagayasu wrote: > > > I wanted to make the postmaster read-only, and found > > > "default_transaction_read_only" option, but it can be overwritten. > > > > I mean it can be overridden by the user. I don't want that. > > I understand, but we haven't gotten enough requests from people for a > new option that can't be overridden. The ability to have PGDATA in read-only media (like CDs) has been requested a lot of times, hasn't it? -- Alvaro Herrera (<alvherre[@]dcc.uchile.cl>) "Porque francamente, si para saber manejarse a uno mismo hubiera que rendir examen... ¿Quién es el machito que tendría carnet?" (Mafalda)
Alvaro Herrera wrote: > On Mon, May 09, 2005 at 10:13:22PM -0400, Bruce Momjian wrote: > > Satoshi Nagayasu wrote: > > > > > > Satoshi Nagayasu wrote: > > > > I wanted to make the postmaster read-only, and found > > > > "default_transaction_read_only" option, but it can be overwritten. > > > > > > I mean it can be overridden by the user. I don't want that. > > > > I understand, but we haven't gotten enough requests from people for a > > new option that can't be overridden. > > The ability to have PGDATA in read-only media (like CDs) has been > requested a lot of times, hasn't it? Right. I am saying the idea of having a GUC that acts like "default_transaction_read_only" but can't be changed isn't something that has been requested frequently. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073
Alvaro Herrera <alvherre@dcc.uchile.cl> writes:
> The ability to have PGDATA in read-only media (like CDs) has been
> requested a lot of times, hasn't it?
It's come up a few times ... more than an un-overridable read-only mode
anyway. Also, I should think that those who want a secure read-only
mode want it enforced selectively --- for instance, assuredly read-only
for some users but not others. I can hardly see any use case for the
patch as proposed; it seems to have all the disadvantages of a low-level
read-only mode (eg, not selective) without any of the advantages.
regards, tom lane
Tom Lane wrote: > Alvaro Herrera <alvherre@dcc.uchile.cl> writes: > > The ability to have PGDATA in read-only media (like CDs) has been > > requested a lot of times, hasn't it? > > It's come up a few times ... more than an un-overridable read-only mode > anyway. Also, I should think that those who want a secure read-only > mode want it enforced selectively --- for instance, assuredly read-only > for some users but not others. I can hardly see any use case for the > patch as proposed; it seems to have all the disadvantages of a low-level > read-only mode (eg, not selective) without any of the advantages. Having removed our security for not allowing override of things like log_statement, it seems we need a more general capability for controlling how something can be set that no one can change. One nify trick would be to use '=' in postgresql.conf for things that can be over-ridden by the user, and ':=' for values that can not be changed. I do think we need that functionality for a variety of purposes. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Having removed our security for not allowing override of things like
> log_statement, it seems we need a more general capability for
> controlling how something can be set that no one can change.
The initial implementation was definitely pretty broken, but I agree
we should try again.
I think that transaction_read_only and default_transaction_read_only
are a special case: they embody our implementation of SQL-spec-mandated
features (SET TRANSACTION READ ONLY and friends), and so any messing
about with them has to surmount the objection that it'll be breaking
spec-mandated behavior. But the other things we wanted this for in
the past, such as logging control, were outside the scope of the spec
AFAIR.
regards, tom lane
Bruce Momjian wrote: >>It's come up a few times ... more than an un-overridable read-only mode >>anyway. Also, I should think that those who want a secure read-only >>mode want it enforced selectively --- for instance, assuredly read-only >>for some users but not others. I can hardly see any use case for the >>patch as proposed; it seems to have all the disadvantages of a low-level >>read-only mode (eg, not selective) without any of the advantages. Our company has some PostgreSQL replication systems for our customers. I need to switch the database state between read-only and writable for recovering or maintenance. As I mentioned before, I wanted to the read-only database mode. It is the per-database state. http://archives.postgresql.org/pgsql-hackers/2005-03/msg00540.php However, if it is not provided, we have to find alternative way to get our purpose. So I'm still looking for how to make the (user) database as read-only. -- NAGAYASU Satoshi <nagayasus@nttdata.co.jp> OpenSource Development Center, NTT DATA Corp. http://www.nttdata.co.jp/
> As I mentioned before, I wanted to the read-only database mode. > It is the per-database state. > > http://archives.postgresql.org/pgsql-hackers/2005-03/msg00540.php > > However, if it is not provided, we have to find alternative way > to get our purpose. > > So I'm still looking for how to make the (user) database as read-only. > Mammoth PostgreSQL Replicator could do this. If you set a database to a slave and tell it to be a slave for all tables it would be read only. Sincerely, Joshua D. Drake Command Prompt, Inc.
On E, 2005-05-09 at 20:21 -0700, Joshua D. Drake wrote: > > As I mentioned before, I wanted to the read-only database mode. > > It is the per-database state. > > > > http://archives.postgresql.org/pgsql-hackers/2005-03/msg00540.php > > > > However, if it is not provided, we have to find alternative way > > to get our purpose. > > > > So I'm still looking for how to make the (user) database as read-only. > > > > Mammoth PostgreSQL Replicator could do this. If you set a database to a > slave and tell it to be a slave for all tables it would be read only. Would it still not have the same issues as Slony (one can create new tables) this patch (one can create temp tables) ? -- Hannu Krosing <hannu@tm.ee>