dividing privileges for replication role.
Hi,
I made a patch to divide privileges for replication role.
Currently(9.2), the privilege for replication role is
true / false which means that standby server is able to
connect to another server or not with the replication role.
This management and cascading replication make a strange behavior.
Because cascading replication is able to connect to another standby server,
we can see the cyclic situation.
This behavior has been discussed on Hackers-list(1),
but the conclusion was that's difficult to detect the situation.
(1) /messages/by-id/50D12E8F.8000808@agliodbs.com
And then, I've reported a Bug-list(2) about this.
In this discussion, an idea that controlling
replication-connection with GUC parameter or privileges on
replication role comes up.
I think these can not avoid cyclic situation but will make some help for
DBA.
(2)
/messages/by-id/E1TtVvj-0004B3-2Z@wrigleys.postgresql.org
In this patch, I made below.
a) adding new privileges for replication:"MASTER REPLICATION" and "CASCADE
REPLICATION"
"MASTER REPLICATION": Replication-connection to master server is only
allowed
"CASCADE REPLICATION": Replication-connection to cascade server is only
allowed
("REPLICATION" already implemented means replication-connection to both
servers is allowed)
b) addin above options in createuser command
--master-replication
--cascade-replication
c) dumping pg_authid.rolreplication value in pg_dumpall
is changed by server version like this:
from 9.1
true -> master-replication
false -> noreplication
from 9.2
true -> replication(master & cascade)
false -> noreplication
I've not write any documents and tests for this yet,
but I want any comments whether this change is needed or not.
regards,
---------
NTT Software Corporation
Tomonari Katsumata
Attachments:
divide-replication-rol-privilege.patchtext/plain; charset=US-ASCII; name=divide-replication-rol-privilege.patchDownload
*** a/src/backend/commands/user.c
--- b/src/backend/commands/user.c
***************
*** 250,256 **** CreateRole(CreateRoleStmt *stmt)
if (dcanlogin)
canlogin = intVal(dcanlogin->arg) != 0;
if (disreplication)
! isreplication = intVal(disreplication->arg) != 0;
if (dconnlimit)
{
connlimit = intVal(dconnlimit->arg);
--- 250,256 ----
if (dcanlogin)
canlogin = intVal(dcanlogin->arg) != 0;
if (disreplication)
! isreplication = intVal(disreplication->arg);
if (dconnlimit)
{
connlimit = intVal(dconnlimit->arg);
***************
*** 352,358 **** CreateRole(CreateRoleStmt *stmt)
/* superuser gets catupdate right by default */
new_record[Anum_pg_authid_rolcatupdate - 1] = BoolGetDatum(issuper);
new_record[Anum_pg_authid_rolcanlogin - 1] = BoolGetDatum(canlogin);
! new_record[Anum_pg_authid_rolreplication - 1] = BoolGetDatum(isreplication);
new_record[Anum_pg_authid_rolconnlimit - 1] = Int32GetDatum(connlimit);
if (password)
--- 352,358 ----
/* superuser gets catupdate right by default */
new_record[Anum_pg_authid_rolcatupdate - 1] = BoolGetDatum(issuper);
new_record[Anum_pg_authid_rolcanlogin - 1] = BoolGetDatum(canlogin);
! new_record[Anum_pg_authid_rolreplication - 1] = Int32GetDatum(isreplication);
new_record[Anum_pg_authid_rolconnlimit - 1] = Int32GetDatum(connlimit);
if (password)
***************
*** 732,738 **** AlterRole(AlterRoleStmt *stmt)
if (isreplication >= 0)
{
! new_record[Anum_pg_authid_rolreplication - 1] = BoolGetDatum(isreplication > 0);
new_record_repl[Anum_pg_authid_rolreplication - 1] = true;
}
--- 732,738 ----
if (isreplication >= 0)
{
! new_record[Anum_pg_authid_rolreplication - 1] = Int32GetDatum(isreplication);
new_record_repl[Anum_pg_authid_rolreplication - 1] = true;
}
*** a/src/backend/parser/gram.y
--- b/src/backend/parser/gram.y
***************
*** 555,561 **** static void processCASbits(int cas_bits, int location, const char *constrType,
LEADING LEAKPROOF LEAST LEFT LEVEL LIKE LIMIT LISTEN LOAD LOCAL
LOCALTIME LOCALTIMESTAMP LOCATION LOCK_P
! MAPPING MATCH MAXVALUE MINUTE_P MINVALUE MODE MONTH_P MOVE
NAME_P NAMES NATIONAL NATURAL NCHAR NEXT NO NONE
NOT NOTHING NOTIFY NOTNULL NOWAIT NULL_P NULLIF
--- 555,561 ----
LEADING LEAKPROOF LEAST LEFT LEVEL LIKE LIMIT LISTEN LOAD LOCAL
LOCALTIME LOCALTIMESTAMP LOCATION LOCK_P
! MAPPING MASTER MATCH MAXVALUE MINUTE_P MINVALUE MODE MONTH_P MOVE
NAME_P NAMES NATIONAL NATURAL NCHAR NEXT NO NONE
NOT NOTHING NOTIFY NOTNULL NOWAIT NULL_P NULLIF
***************
*** 571,577 **** static void processCASbits(int cas_bits, int location, const char *constrType,
QUOTE
RANGE READ REAL REASSIGN RECHECK RECURSIVE REF REFERENCES REINDEX
! RELATIVE_P RELEASE RENAME REPEATABLE REPLACE REPLICA
RESET RESTART RESTRICT RETURNING RETURNS REVOKE RIGHT ROLE ROLLBACK
ROW ROWS RULE
--- 571,577 ----
QUOTE
RANGE READ REAL REASSIGN RECHECK RECURSIVE REF REFERENCES REINDEX
! RELATIVE_P RELEASE RENAME REPEATABLE REPLACE REPLICA REPLICATION
RESET RESTART RESTRICT RETURNING RETURNS REVOKE RIGHT ROLE ROLLBACK
ROW ROWS RULE
***************
*** 890,895 **** AlterOptRoleElem:
--- 890,907 ----
{
$$ = makeDefElem("rolemembers", (Node *)$2);
}
+ | REPLICATION
+ {
+ $$ = makeDefElem("isreplication", (Node *)makeInteger(1));
+ }
+ | MASTER REPLICATION
+ {
+ $$ = makeDefElem("isreplication", (Node *)makeInteger(2));
+ }
+ | CASCADE REPLICATION
+ {
+ $$ = makeDefElem("isreplication", (Node *)makeInteger(3));
+ }
| IDENT
{
/*
***************
*** 915,924 **** AlterOptRoleElem:
$$ = makeDefElem("createrole", (Node *)makeInteger(TRUE));
else if (strcmp($1, "nocreaterole") == 0)
$$ = makeDefElem("createrole", (Node *)makeInteger(FALSE));
- else if (strcmp($1, "replication") == 0)
- $$ = makeDefElem("isreplication", (Node *)makeInteger(TRUE));
else if (strcmp($1, "noreplication") == 0)
! $$ = makeDefElem("isreplication", (Node *)makeInteger(FALSE));
else if (strcmp($1, "createdb") == 0)
$$ = makeDefElem("createdb", (Node *)makeInteger(TRUE));
else if (strcmp($1, "nocreatedb") == 0)
--- 927,934 ----
$$ = makeDefElem("createrole", (Node *)makeInteger(TRUE));
else if (strcmp($1, "nocreaterole") == 0)
$$ = makeDefElem("createrole", (Node *)makeInteger(FALSE));
else if (strcmp($1, "noreplication") == 0)
! $$ = makeDefElem("isreplication", (Node *)makeInteger(0));
else if (strcmp($1, "createdb") == 0)
$$ = makeDefElem("createdb", (Node *)makeInteger(TRUE));
else if (strcmp($1, "nocreatedb") == 0)
***************
*** 12561,12566 **** unreserved_keyword:
--- 12571,12577 ----
| LOCATION
| LOCK_P
| MAPPING
+ | MASTER
| MATCH
| MAXVALUE
| MINUTE_P
***************
*** 12613,12618 **** unreserved_keyword:
--- 12624,12630 ----
| REPEATABLE
| REPLACE
| REPLICA
+ | REPLICATION
| RESET
| RESTART
| RESTRICT
*** a/src/backend/utils/init/miscinit.c
--- b/src/backend/utils/init/miscinit.c
***************
*** 392,398 **** SetUserIdAndContext(Oid userid, bool sec_def_context)
/*
* Check if the authenticated user is a replication role
*/
! bool
is_authenticated_user_replication_role(void)
{
bool result = false;
--- 392,398 ----
/*
* Check if the authenticated user is a replication role
*/
! int
is_authenticated_user_replication_role(void)
{
bool result = false;
*** a/src/backend/utils/init/postinit.c
--- b/src/backend/utils/init/postinit.c
***************
*** 724,733 **** InitPostgres(const char *in_dbname, Oid dboid, const char *username,
{
Assert(!bootstrap);
! if (!superuser() && !is_authenticated_user_replication_role())
ereport(FATAL,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
errmsg("must be superuser or replication role to start walsender")));
/* process any options passed in the startup packet */
if (MyProcPort != NULL)
--- 724,758 ----
{
Assert(!bootstrap);
! int replication_privilege = 0;
! if (!superuser() && (replication_privilege = is_authenticated_user_replication_role()) == 0)
ereport(FATAL,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
errmsg("must be superuser or replication role to start walsender")));
+ else
+ {
+ bool isRecovery = RecoveryInProgress();
+ switch (replication_privilege)
+ {
+ case 1:
+ /* nothing to think about privilege */
+ break;
+ case 2:
+ /* master only */
+ if (isRecovery)
+ ereport(FATAL,
+ (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ errmsg("must be superuser or cascade replication role to start walsender")));
+ break;
+ case 3:
+ /* cascade only */
+ if (!isRecovery)
+ ereport(FATAL,
+ (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ errmsg("must be superuser or master replication role to start walsender")));
+ break;
+ }
+ }
/* process any options passed in the startup packet */
if (MyProcPort != NULL)
*** a/src/bin/pg_dump/pg_dumpall.c
--- b/src/bin/pg_dump/pg_dumpall.c
***************
*** 659,670 **** dumpRoles(PGconn *conn)
"rolname = current_user AS is_current_user "
"FROM pg_authid "
"ORDER BY 2");
else if (server_version >= 80200)
printfPQExpBuffer(buf,
"SELECT oid, rolname, rolsuper, rolinherit, "
"rolcreaterole, rolcreatedb, "
"rolcanlogin, rolconnlimit, rolpassword, "
! "rolvaliduntil, false as rolreplication, "
"pg_catalog.shobj_description(oid, 'pg_authid') as rolcomment, "
"rolname = current_user AS is_current_user "
"FROM pg_authid "
--- 659,691 ----
"rolname = current_user AS is_current_user "
"FROM pg_authid "
"ORDER BY 2");
+ else if (server_version >= 90200)
+ printfPQExpBuffer(buf,
+ "SELECT oid, rolname, rolsuper, rolinherit, "
+ "rolcreaterole, rolcreatedb, "
+ "rolcanlogin, rolconnlimit, rolpassword, "
+ "rolvaliduntil, rolreplication::int, "
+ "pg_catalog.shobj_description(oid, 'pg_authid') as rolcomment, "
+ "rolname = current_user AS is_current_user "
+ "FROM pg_authid "
+ "ORDER BY 2");
+ else if (server_version >= 90100)
+ printfPQExpBuffer(buf,
+ "SELECT oid, rolname, rolsuper, rolinherit, "
+ "rolcreaterole, rolcreatedb, "
+ "rolcanlogin, rolconnlimit, rolpassword, "
+ "rolvaliduntil, "
+ "CASE WHEN rolreplication THEN 2 ELSE 0 END, "
+ "pg_catalog.shobj_description(oid, 'pg_authid') as rolcomment, "
+ "rolname = current_user AS is_current_user "
+ "FROM pg_authid "
+ "ORDER BY 2");
else if (server_version >= 80200)
printfPQExpBuffer(buf,
"SELECT oid, rolname, rolsuper, rolinherit, "
"rolcreaterole, rolcreatedb, "
"rolcanlogin, rolconnlimit, rolpassword, "
! "rolvaliduntil, 0 as rolreplication, "
"pg_catalog.shobj_description(oid, 'pg_authid') as rolcomment, "
"rolname = current_user AS is_current_user "
"FROM pg_authid "
***************
*** 674,680 **** dumpRoles(PGconn *conn)
"SELECT oid, rolname, rolsuper, rolinherit, "
"rolcreaterole, rolcreatedb, "
"rolcanlogin, rolconnlimit, rolpassword, "
! "rolvaliduntil, false as rolreplication, "
"null as rolcomment, "
"rolname = current_user AS is_current_user "
"FROM pg_authid "
--- 695,701 ----
"SELECT oid, rolname, rolsuper, rolinherit, "
"rolcreaterole, rolcreatedb, "
"rolcanlogin, rolconnlimit, rolpassword, "
! "rolvaliduntil, 0 as rolreplication, "
"null as rolcomment, "
"rolname = current_user AS is_current_user "
"FROM pg_authid "
***************
*** 690,696 **** dumpRoles(PGconn *conn)
"-1 as rolconnlimit, "
"passwd as rolpassword, "
"valuntil as rolvaliduntil, "
! "false as rolreplication, "
"null as rolcomment, "
"rolname = current_user AS is_current_user "
"FROM pg_shadow "
--- 711,717 ----
"-1 as rolconnlimit, "
"passwd as rolpassword, "
"valuntil as rolvaliduntil, "
! "0 as rolreplication, "
"null as rolcomment, "
"rolname = current_user AS is_current_user "
"FROM pg_shadow "
***************
*** 786,795 **** dumpRoles(PGconn *conn)
else
appendPQExpBuffer(buf, " NOLOGIN");
! if (strcmp(PQgetvalue(res, i, i_rolreplication), "t") == 0)
! appendPQExpBuffer(buf, " REPLICATION");
! else
appendPQExpBuffer(buf, " NOREPLICATION");
if (strcmp(PQgetvalue(res, i, i_rolconnlimit), "-1") != 0)
appendPQExpBuffer(buf, " CONNECTION LIMIT %s",
--- 807,822 ----
else
appendPQExpBuffer(buf, " NOLOGIN");
! if (strcmp(PQgetvalue(res, i, i_rolreplication), "0") == 0)
appendPQExpBuffer(buf, " NOREPLICATION");
+ else{
+ if (strcmp(PQgetvalue(res, i, i_rolreplication), "1") == 0)
+ appendPQExpBuffer(buf, " REPLICATION");
+ else if (strcmp(PQgetvalue(res, i, i_rolreplication), "2") == 0)
+ appendPQExpBuffer(buf, " MASTER REPLICATION");
+ else
+ appendPQExpBuffer(buf, " CASCADE REPLICATION");
+ }
if (strcmp(PQgetvalue(res, i, i_rolconnlimit), "-1") != 0)
appendPQExpBuffer(buf, " CONNECTION LIMIT %s",
*** a/src/bin/scripts/common.h
--- b/src/bin/scripts/common.h
***************
*** 20,25 **** enum trivalue
--- 20,34 ----
TRI_YES
};
+ enum repvalue
+ {
+ REP_DEFAULT,
+ REP_NOREPLICATION,
+ REP_REPLICATION,
+ REP_MASTER,
+ REP_CASCADE
+ };
+
typedef void (*help_handler) (const char *progname);
extern const char *get_user_name(const char *progname);
*** a/src/bin/scripts/createuser.c
--- b/src/bin/scripts/createuser.c
***************
*** 39,45 **** main(int argc, char *argv[])
{"no-login", no_argument, NULL, 'L'},
{"replication", no_argument, NULL, 1},
{"no-replication", no_argument, NULL, 2},
! {"interactive", no_argument, NULL, 3},
/* adduser is obsolete, undocumented spelling of superuser */
{"adduser", no_argument, NULL, 'a'},
{"no-adduser", no_argument, NULL, 'A'},
--- 39,47 ----
{"no-login", no_argument, NULL, 'L'},
{"replication", no_argument, NULL, 1},
{"no-replication", no_argument, NULL, 2},
! {"master-replication", no_argument, NULL, 3},
! {"cascade-replication", no_argument, NULL, 4},
! {"interactive", no_argument, NULL, 5},
/* adduser is obsolete, undocumented spelling of superuser */
{"adduser", no_argument, NULL, 'a'},
{"no-adduser", no_argument, NULL, 'A'},
***************
*** 70,76 **** main(int argc, char *argv[])
createrole = TRI_DEFAULT,
inherit = TRI_DEFAULT,
login = TRI_DEFAULT,
! replication = TRI_DEFAULT,
encrypted = TRI_DEFAULT;
PQExpBufferData sql;
--- 72,78 ----
createrole = TRI_DEFAULT,
inherit = TRI_DEFAULT,
login = TRI_DEFAULT,
! replication = REP_DEFAULT,
encrypted = TRI_DEFAULT;
PQExpBufferData sql;
***************
*** 151,162 **** main(int argc, char *argv[])
encrypted = TRI_NO;
break;
case 1:
! replication = TRI_YES;
break;
case 2:
! replication = TRI_NO;
break;
case 3:
interactive = true;
break;
default:
--- 153,170 ----
encrypted = TRI_NO;
break;
case 1:
! replication = REP_REPLICATION;
break;
case 2:
! replication = REP_NOREPLICATION;
break;
case 3:
+ replication = REP_MASTER;
+ break;
+ case 4:
+ replication = REP_CASCADE;
+ break;
+ case 5:
interactive = true;
break;
default:
***************
*** 296,305 **** main(int argc, char *argv[])
appendPQExpBuffer(&sql, " LOGIN");
if (login == TRI_NO)
appendPQExpBuffer(&sql, " NOLOGIN");
! if (replication == TRI_YES)
appendPQExpBuffer(&sql, " REPLICATION");
! if (replication == TRI_NO)
appendPQExpBuffer(&sql, " NOREPLICATION");
if (conn_limit != NULL)
appendPQExpBuffer(&sql, " CONNECTION LIMIT %s", conn_limit);
appendPQExpBuffer(&sql, ";\n");
--- 304,317 ----
appendPQExpBuffer(&sql, " LOGIN");
if (login == TRI_NO)
appendPQExpBuffer(&sql, " NOLOGIN");
! if (replication == REP_REPLICATION)
appendPQExpBuffer(&sql, " REPLICATION");
! if (replication == REP_NOREPLICATION)
appendPQExpBuffer(&sql, " NOREPLICATION");
+ if (replication == REP_MASTER)
+ appendPQExpBuffer(&sql, " MASTER REPLICATION");
+ if (replication == REP_CASCADE)
+ appendPQExpBuffer(&sql, " CASCADE REPLICATION");
if (conn_limit != NULL)
appendPQExpBuffer(&sql, " CONNECTION LIMIT %s", conn_limit);
appendPQExpBuffer(&sql, ";\n");
***************
*** 348,355 **** help(const char *progname)
printf(_(" -V, --version output version information, then exit\n"));
printf(_(" --interactive prompt for missing role name and attributes rather\n"
" than using defaults\n"));
- printf(_(" --replication role can initiate replication\n"));
printf(_(" --no-replication role cannot initiate replication\n"));
printf(_(" -?, --help show this help, then exit\n"));
printf(_("\nConnection options:\n"));
printf(_(" -h, --host=HOSTNAME database server host or socket directory\n"));
--- 360,369 ----
printf(_(" -V, --version output version information, then exit\n"));
printf(_(" --interactive prompt for missing role name and attributes rather\n"
" than using defaults\n"));
printf(_(" --no-replication role cannot initiate replication\n"));
+ printf(_(" --replication role can initiate replication from master and cascade\n"));
+ printf(_(" --master-replication role can initiate replication from only master\n"));
+ printf(_(" --cascade-replication role can initiate replication from only cascade\n"));
printf(_(" -?, --help show this help, then exit\n"));
printf(_("\nConnection options:\n"));
printf(_(" -h, --host=HOSTNAME database server host or socket directory\n"));
*** a/src/include/catalog/pg_authid.h
--- b/src/include/catalog/pg_authid.h
***************
*** 51,57 **** CATALOG(pg_authid,1260) BKI_SHARED_RELATION BKI_ROWTYPE_OID(2842) BKI_SCHEMA_MAC
bool rolcreatedb; /* allowed to create databases? */
bool rolcatupdate; /* allowed to alter catalogs manually? */
bool rolcanlogin; /* allowed to log in as session user? */
! bool rolreplication; /* role used for streaming replication */
int32 rolconnlimit; /* max connections allowed (-1=no limit) */
/* remaining fields may be null; use heap_getattr to read them! */
--- 51,59 ----
bool rolcreatedb; /* allowed to create databases? */
bool rolcatupdate; /* allowed to alter catalogs manually? */
bool rolcanlogin; /* allowed to log in as session user? */
! int32 rolreplication; /* role used for streaming replication */
! /* 0:noreplication, 1:replication(master & cascade) */
! /* 2:replication(master), 3:replication(cascade) */
int32 rolconnlimit; /* max connections allowed (-1=no limit) */
/* remaining fields may be null; use heap_getattr to read them! */
***************
*** 93,99 **** typedef FormData_pg_authid *Form_pg_authid;
* user choices.
* ----------------
*/
! DATA(insert OID = 10 ( "POSTGRES" t t t t t t t -1 _null_ _null_ ));
#define BOOTSTRAP_SUPERUSERID 10
--- 95,101 ----
* user choices.
* ----------------
*/
! DATA(insert OID = 10 ( "POSTGRES" t t t t t t 1 -1 _null_ _null_ ));
#define BOOTSTRAP_SUPERUSERID 10
*** a/src/include/miscadmin.h
--- b/src/include/miscadmin.h
***************
*** 439,445 **** extern void ValidatePgVersion(const char *path);
extern void process_shared_preload_libraries(void);
extern void process_local_preload_libraries(void);
extern void pg_bindtextdomain(const char *domain);
! extern bool is_authenticated_user_replication_role(void);
/* in access/transam/xlog.c */
extern bool BackupInProgress(void);
--- 439,445 ----
extern void process_shared_preload_libraries(void);
extern void process_local_preload_libraries(void);
extern void pg_bindtextdomain(const char *domain);
! extern int is_authenticated_user_replication_role(void);
/* in access/transam/xlog.c */
extern bool BackupInProgress(void);
*** a/src/include/parser/kwlist.h
--- b/src/include/parser/kwlist.h
***************
*** 231,236 **** PG_KEYWORD("localtimestamp", LOCALTIMESTAMP, RESERVED_KEYWORD)
--- 231,237 ----
PG_KEYWORD("location", LOCATION, UNRESERVED_KEYWORD)
PG_KEYWORD("lock", LOCK_P, UNRESERVED_KEYWORD)
PG_KEYWORD("mapping", MAPPING, UNRESERVED_KEYWORD)
+ PG_KEYWORD("master", MASTER, UNRESERVED_KEYWORD)
PG_KEYWORD("match", MATCH, UNRESERVED_KEYWORD)
PG_KEYWORD("maxvalue", MAXVALUE, UNRESERVED_KEYWORD)
PG_KEYWORD("minute", MINUTE_P, UNRESERVED_KEYWORD)
***************
*** 308,313 **** PG_KEYWORD("rename", RENAME, UNRESERVED_KEYWORD)
--- 309,315 ----
PG_KEYWORD("repeatable", REPEATABLE, UNRESERVED_KEYWORD)
PG_KEYWORD("replace", REPLACE, UNRESERVED_KEYWORD)
PG_KEYWORD("replica", REPLICA, UNRESERVED_KEYWORD)
+ PG_KEYWORD("replication", REPLICATION, UNRESERVED_KEYWORD)
PG_KEYWORD("reset", RESET, UNRESERVED_KEYWORD)
PG_KEYWORD("restart", RESTART, UNRESERVED_KEYWORD)
PG_KEYWORD("restrict", RESTRICT, UNRESERVED_KEYWORD)
On Sat, Jan 19, 2013 at 4:47 AM, Tomonari Katsumata
<t.katsumata1122@gmail.com> wrote:
Hi,
I made a patch to divide privileges for replication role.
Currently(9.2), the privilege for replication role is
true / false which means that standby server is able to
connect to another server or not with the replication role.
Why is it better to do this with a privilege, rather than just using
pg_hba.conf? It doesn't represent an actual "permission level" after
all - it's just an administrative "flag" to say you can't connect.
Which AFAICS can just as easily be handled in pg_hba.conf? I guess I'm
missing something?
--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Katsumata-san,
In this patch, I made below.
a) adding new privileges for replication:"MASTER REPLICATION" and "CASCADE
REPLICATION"
"MASTER REPLICATION": Replication-connection to master server is only
allowed
"CASCADE REPLICATION": Replication-connection to cascade server is only
allowed
("REPLICATION" already implemented means replication-connection to both
servers is allowed)
This seems to me a case of making things more complicated for everyone
in order to satisfy a very narrow use-case. It certainly doesn't seem
to me to do anything about the "accidental cycle" issue. Am I missing
something?
--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Sat, Jan 19, 2013 at 12:47 PM, Tomonari Katsumata <
t.katsumata1122@gmail.com> wrote:
a) adding new privileges for replication:"MASTER REPLICATION" and "CASCADE
REPLICATION""MASTER REPLICATION": Replication-connection to master server is only
allowed
"CASCADE REPLICATION": Replication-connection to cascade server is only
allowed
("REPLICATION" already implemented means replication-connection to both
servers is allowed)
This does not really solve the case you reported because, as reported in
your bug, you could still have each slave connecting to each other using
the privilege CASCADE REPLICATION. It makes even the privilege level more
complicated.
What would be necessary to solve your problem would be to have each standby
being aware that it is connected to a unique master. This is not really an
issue with privileges but more of something like having a standby scanning
its upper cluster node tree and check if there is a master connected. While
checking the cluster node tree, you will also need to be aware if a node
has already been found when you scanned it to be sure that the same node
has not been scanned, what would mean that you are in a cycle.
--
Michael Paquier
http://michael.otacoo.com
On 01/19/2013 11:47 AM, Tomonari Katsumata wrote:
Hi,
I made a patch to divide privileges for replication role.
I've added your patch to the commitfest tracking app for the post-9.3
release; see https://commitfest.postgresql.org/action/patch_view?id=1072 .
If it's convenient for you to keep that entry up to date with any
revised patches you get and any reviews people write that will be rather
helpful. I'll keep an eye on it and update it when I see something
change, but you're paying attention to this one patch so you'll notice
first.
--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
Hi, Magnus, Josh, Michael, Craig
Thank you for comments and registring to CommitFest.
I made a patch to divide privileges for replication role.
Currently(9.2), the privilege for replication role is
true / false which means that standby server is able to
connect to another server or not with the replication role.Why is it better to do this with a privilege, rather than just using
pg_hba.conf? It doesn't represent an actual "permission level" after
all - it's just an administrative "flag" to say you can't connect.
Which AFAICS can just as easily be handled in pg_hba.conf? I guess I'm
missing something?
You are right.
Handling with pg_hba.conf is an easy way.
But I think many users think about switch over, so
the pg_hba.conf is same on master and standby.
it's not convinient that we have to rewrite pg_hba.conf
whenever switch over occurs.
In the other hand, using a privilege, although we have to prepare
each roles before, we don't need to rewrite pg_hba.conf.
So I think it's better with a privilege than using only pg_hba.conf
----
In this patch, I made below.
a) adding new privileges for replication:"MASTER REPLICATION" and
"CASCADE
REPLICATION"
"MASTER REPLICATION": Replication-connection to master server is only
allowed
"CASCADE REPLICATION": Replication-connection to cascade server is
only
allowed
("REPLICATION" already implemented means replication-connection to
both
servers is allowed)
This seems to me a case of making things more complicated for everyone
in order to satisfy a very narrow use-case. It certainly doesn't seem
to me to do anything about the "accidental cycle" issue. Am I missing
something?
I agreed that it is a very narrow use-case and accidental thing.
But I think we should provide a kind of method to avoid it,
because it has been different of before release.
And I don't think it's complicated, because "REPLICATION" and
"NOREPLICATION" do same behavior with before release.
----
a) adding new privileges for replication:"MASTER REPLICATION" and
"CASCADE
REPLICATION"
"MASTER REPLICATION": Replication-connection to master server is only
allowed
"CASCADE REPLICATION": Replication-connection to cascade server is
only
allowed
("REPLICATION" already implemented means replication-connection to
both
servers is allowed)
This does not really solve the case you reported because, as reported in
your bug, you could still have each slave connecting to each other using
the privilege CASCADE REPLICATION. It makes even the privilege level more
complicated.
Yes, the patch can not solve the case at all.
I just added a parameter for users.
It is responsibility of users to connect with a right role.
What would be necessary to solve your problem would be to have each standby
being aware that it is connected to a unique master. This is not really an
issue with privileges but more of something like having a standby scanning
its upper cluster node tree and check if there is a master connected. While
checking the cluster node tree, you will also need to be aware if a node
has already been found when you scanned it to be sure that the same node
has not been scanned, what would mean that you are in a cycle.
I think this is very complicated.
At least, now I can't solve it...
If someday we can detect it, this kind of switch will be needed.
Because some users may need the cyclic situation.
----
I'm not insisting to use replication-role, but
I want something to control this behavior.
regards,
------------
NTT Software Corporation
Tomonari Katsumata
Tomonari Katsumata <t.katsumata1122@gmail.com> writes:
Why is it better to do this with a privilege, rather than just using
pg_hba.conf?
You are right.
Handling with pg_hba.conf is an easy way.
But I think many users think about switch over, so
the pg_hba.conf is same on master and standby.
it's not convinient that we have to rewrite pg_hba.conf
whenever switch over occurs.
In the other hand, using a privilege, although we have to prepare
each roles before, we don't need to rewrite pg_hba.conf.
That sounds good, but if the behavior is controlled by a privilege
(ie, it's stored in system catalogs) then it's impossible to have
different settings on different slave servers --- or indeed to change
the settings locally on a slave at all. You can only change settings
on the master and let the change replicate to all the slaves. Quite
aside from whether you want to manage things like that, what happens if
your master has crashed and you find you need to change the settings on
the way to getting a slave to take over?
The crash-recovery worry is one of the main reasons that things like
pg_hba.conf aren't stored in system catalogs already. It's not always
convenient to need a running server before you can change the settings.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Hi, Tom
Thank you for comments.
Tomonari Katsumata <t.katsumata1122@gmail.com> writes:
Why is it better to do this with a privilege, rather than just using
pg_hba.conf?You are right.
Handling with pg_hba.conf is an easy way.But I think many users think about switch over, so
the pg_hba.conf is same on master and standby.
it's not convinient that we have to rewrite pg_hba.conf
whenever switch over occurs.In the other hand, using a privilege, although we have to prepare
each roles before, we don't need to rewrite pg_hba.conf.That sounds good, but if the behavior is controlled by a privilege
(ie, it's stored in system catalogs) then it's impossible to have
different settings on different slave servers --- or indeed to change
the settings locally on a slave at all. You can only change settings
on the master and let the change replicate to all the slaves.
Yes, I'm thinking changing settings on the master and the settings are
propagating to all slaves.
Quite aside from whether you want to manage things like that, what
happens if
your master has crashed and you find you need to change the settings on
the way to getting a slave to take over?The crash-recovery worry is one of the main reasons that things like
pg_hba.conf aren't stored in system catalogs already. It's not always
convenient to need a running server before you can change the settings.
I understand that the approach in my patch(using pribileges for controlling
its behavior) does not match the policy.
But I'm still thinking I should put a something to controle
the behavior.
Then, how about to add a new option "standby_mode" to Database Connection
Control Functions like application_name.
ex:
primary_conninfo = 'port=5432 standby_mode=master-cascade'
primary_conninfo = 'port=5432 standby_mode=master-only'
primary_conninfo = 'port=5432 standby_mode=cascade-only'
I think it will be able to do same control with privilege controlling.
And it won't be contrary to the policy(no data is stored in system
catalogs).
Because it is corner-case, I should not do anything about this?
(Am I concerning too much?)
regards,
--------
NTT Software Corporation
Tomonari Katsumata
On Wed, Jan 23, 2013 at 5:46 PM, Tomonari Katsumata <
t.katsumata1122@gmail.com> wrote:
ex:
primary_conninfo = 'port=5432 standby_mode=master-cascade'
primary_conninfo = 'port=5432 standby_mode=master-only'
primary_conninfo = 'port=5432 standby_mode=cascade-only'I think it will be able to do same control with privilege controlling.
And it won't be contrary to the policy(no data is stored in system
catalogs).Because it is corner-case, I should not do anything about this?
(Am I concerning too much?)
Just curious, but... What is your primary goal with this patch?
Solving the cyclic problem?
--
Michael Paquier
http://michael.otacoo.com
Hi, Michael
2013/1/23 Michael Paquier <michael.paquier@gmail.com>
On Wed, Jan 23, 2013 at 5:46 PM, Tomonari Katsumata <
t.katsumata1122@gmail.com> wrote:ex:
primary_conninfo = 'port=5432 standby_mode=master-cascade'
primary_conninfo = 'port=5432 standby_mode=master-only'
primary_conninfo = 'port=5432 standby_mode=cascade-only'I think it will be able to do same control with privilege controlling.
And it won't be contrary to the policy(no data is stored in system
catalogs).Because it is corner-case, I should not do anything about this?
(Am I concerning too much?)Just curious, but... What is your primary goal with this patch?
Solving the cyclic problem?
No, I'm not thinking about solving the cyclic problem directly.
It is too difficult for me.
And the goal of my patch is adding some selections to avoid it for users.
--------
NTT Software Corporation
Tomonari Katsumata