Thread: Proof of concept: standalone backend with full FE/BE protocol
Attached is a proof-of-concept patch for talking to a standalone backend using libpq and a pair of pipes. It works, to the extent that psql and pg_dump can run without any postmaster present: $ psql regression psql: could not connect to server: No such file or directory Is the server running locally and accepting connections on Unix domain socket "/tmp/.s.PGSQL.5432"? $ psql "standalone_datadir = $PGDATA dbname = regression" psql (9.3devel) Type "help" for help. regression=> \d List of relations Schema | Name | Type | Owner --------+-----------------------------+----------+------- public | a | table | tgl public | a_star | table | tgl public | abstime_tbl | table | tgl public | aggtest | table | tgl public | array_index_op_test | table | tgl ... but there's quite a bit of work to do yet before this could be a committable patch. Some notes: 1. As you can see above, the feature is triggered by specifying the new connection option "standalone_datadir", whose value must be the location of the data directory. I also invented an option "standalone_backend", which can be set to specify which postgres executable to launch. If the latter isn't specified, libpq defaults to trying the installation PGBINDIR that was selected by configure. (I don't think it can use the relative path tricks we use in pg_ctl and elsewhere, since there's no good reason to assume that it's running in a Postgres-supplied program.) I'm not particularly wedded to these names or the syntax, but I think this is the basic functionality we'd need. 2. As far as the backend is concerned, use of FE/BE protocol rather than traditional standalone mode is triggered by writing "--child" instead of "--single" as the first argument on the postgres command line. (I'm not wedded to that name either ... anybody have a better idea?) 3. The bulk of the changes have to do with the fact that we need to keep track of two file descriptors not one. This was a bit tedious, but fairly straightforward --- though I was surprised to find that send() and recv() don't work on pipes, at least not on Linux. You have to use read() and write() instead. 4. As coded, the backend assumes the incoming pipe is on its FD 0 and the outgoing pipe is on its FD 1. This made the command line simple but I'm having second thoughts about it: if anything inside the backend tries to read stdin or write stdout, unpleasant things will happen. It might be better to not reassign the pipe FD numbers. In that case we'd have to pass them on the command line, so that the syntax would be something like "postgres --child 4,5 -D pgdata ...". 5. The fork/exec code is pretty primitive with respect to error handling. I didn't put much time into it since I'm afraid we may need to refactor it entirely before a Windows equivalent can be written. (And I need somebody to write/test the Windows equivalent - any volunteers?) 6. I didn't bother with passing anything except -D and the database name to the standalone backend. Probably we'd like to be able to pass other command-line switches too. Again, it's not clear that it's worth doing much here until we have equivalent Windows code available. 7. I haven't tried to make pg_upgrade use this yet. 8. PQcancel needs some work - it can't do what it does now, but it could do kill(conn->postgres_pid, SIGINT) instead. At least in Unix. I have no idea what we'd do in Windows. This doesn't matter for pg_upgrade of course, but it'd be important for manual use of this mode. Although the immediate use of this would be for pg_upgrade, I think that people would soon drop the traditional --single mode and do anything they need to do in standalone mode using this method, since psql is so vastly more user-friendly than a --single backend. In the longer run, this could provide a substitute for the "embedded database" mode that we keep saying we're not going to implement. That is, applications could fire up a standalone backend as a child process and not need a postmaster anywhere, which would be a lot more convenient for an app that wants a private database and doesn't want to involve its users in managing a Postgres server. However, there are some additional things we'd need to think about before advertising it as a fit solution for that. Notably, while the lack of any background processes is just what you want for pg_upgrade and disaster recovery, an ordinary application is probably going to want to rely on autovacuum; and we need bgwriter and other background processes for best performance. So I'm speculating about having a postmaster process that isn't listening on any ports, but is managing background processes in addition to a single child backend. That's for another day though. Comments? Anyone want to have a go at fixing this for Windows? regards, tom lane diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c index c765454..6e65a9d 100644 *** a/src/backend/libpq/auth.c --- b/src/backend/libpq/auth.c *************** ClientAuthentication(Port *port) *** 313,318 **** --- 313,321 ---- { int status = STATUS_ERROR; + /* Authentication is not supported over pipes */ + Assert(port->rsock == port->wsock); + /* * Get the authentication method to use for this frontend/database * combination. Note: we do not parse the file at this point; this has *************** pg_krb5_recvauth(Port *port) *** 820,826 **** return ret; retval = krb5_recvauth(pg_krb5_context, &auth_context, ! (krb5_pointer) &port->sock, pg_krb_srvnam, pg_krb5_server, 0, pg_krb5_keytab, &ticket); if (retval) { --- 823,829 ---- return ret; retval = krb5_recvauth(pg_krb5_context, &auth_context, ! (krb5_pointer) &port->rsock, pg_krb_srvnam, pg_krb5_server, 0, pg_krb5_keytab, &ticket); if (retval) { *************** auth_peer(hbaPort *port) *** 1768,1774 **** struct passwd *pass; errno = 0; ! if (getpeereid(port->sock, &uid, &gid) != 0) { /* Provide special error message if getpeereid is a stub */ if (errno == ENOSYS) --- 1771,1777 ---- struct passwd *pass; errno = 0; ! if (getpeereid(port->rsock, &uid, &gid) != 0) { /* Provide special error message if getpeereid is a stub */ if (errno == ENOSYS) diff --git a/src/backend/libpq/be-secure.c b/src/backend/libpq/be-secure.c index 0d66dab..0bde205 100644 *** a/src/backend/libpq/be-secure.c --- b/src/backend/libpq/be-secure.c *************** rloop: *** 301,307 **** { prepare_for_client_read(); ! n = recv(port->sock, ptr, len, 0); client_read_ended(); } --- 301,307 ---- { prepare_for_client_read(); ! n = read(port->rsock, ptr, len); client_read_ended(); } *************** wloop: *** 393,399 **** } else #endif ! n = send(port->sock, ptr, len, 0); return n; } --- 393,399 ---- } else #endif ! n = write(port->wsock, ptr, len); return n; } *************** open_server_SSL(Port *port) *** 876,881 **** --- 876,882 ---- Assert(!port->ssl); Assert(!port->peer); + Assert(port->rsock == port->wsock); if (!(port->ssl = SSL_new(SSL_context))) { *************** open_server_SSL(Port *port) *** 886,892 **** close_SSL(port); return -1; } ! if (!my_SSL_set_fd(port->ssl, port->sock)) { ereport(COMMERROR, (errcode(ERRCODE_PROTOCOL_VIOLATION), --- 887,893 ---- close_SSL(port); return -1; } ! if (!my_SSL_set_fd(port->ssl, port->rsock)) { ereport(COMMERROR, (errcode(ERRCODE_PROTOCOL_VIOLATION), diff --git a/src/backend/libpq/pqcomm.c b/src/backend/libpq/pqcomm.c index 5e86987..fa4d322 100644 *** a/src/backend/libpq/pqcomm.c --- b/src/backend/libpq/pqcomm.c *************** pq_close(int code, Datum arg) *** 215,224 **** * transport layer reports connection closure, and you can be sure the * backend has exited. * ! * We do set sock to PGINVALID_SOCKET to prevent any further I/O, * though. */ ! MyProcPort->sock = PGINVALID_SOCKET; } } --- 215,225 ---- * transport layer reports connection closure, and you can be sure the * backend has exited. * ! * We do set the socket to PGINVALID_SOCKET to prevent any further I/O, * though. */ ! MyProcPort->rsock = PGINVALID_SOCKET; ! MyProcPort->wsock = PGINVALID_SOCKET; } } *************** Setup_AF_UNIX(char *sock_path) *** 609,615 **** /* * StreamConnection -- create a new connection with client using ! * server port. Set port->sock to the FD of the new connection. * * ASSUME: that this doesn't need to be non-blocking because * the Postmaster uses select() to tell when the server master --- 610,616 ---- /* * StreamConnection -- create a new connection with client using ! * server port. Set port->rsock/wsock to the FD of the new connection. * * ASSUME: that this doesn't need to be non-blocking because * the Postmaster uses select() to tell when the server master *************** StreamConnection(pgsocket server_fd, Por *** 622,630 **** { /* accept connection and fill in the client (remote) address */ port->raddr.salen = sizeof(port->raddr.addr); ! if ((port->sock = accept(server_fd, ! (struct sockaddr *) & port->raddr.addr, ! &port->raddr.salen)) < 0) { ereport(LOG, (errcode_for_socket_access(), --- 623,631 ---- { /* accept connection and fill in the client (remote) address */ port->raddr.salen = sizeof(port->raddr.addr); ! if ((port->rsock = accept(server_fd, ! (struct sockaddr *) & port->raddr.addr, ! &port->raddr.salen)) < 0) { ereport(LOG, (errcode_for_socket_access(), *************** StreamConnection(pgsocket server_fd, Por *** 641,646 **** --- 642,650 ---- return STATUS_ERROR; } + /* It's a bidirectional socket, so wsock and rsock are equal */ + port->wsock = port->rsock; + #ifdef SCO_ACCEPT_BUG /* *************** StreamConnection(pgsocket server_fd, Por *** 653,659 **** /* fill in the server (local) address */ port->laddr.salen = sizeof(port->laddr.addr); ! if (getsockname(port->sock, (struct sockaddr *) & port->laddr.addr, &port->laddr.salen) < 0) { --- 657,663 ---- /* fill in the server (local) address */ port->laddr.salen = sizeof(port->laddr.addr); ! if (getsockname(port->rsock, (struct sockaddr *) & port->laddr.addr, &port->laddr.salen) < 0) { *************** StreamConnection(pgsocket server_fd, Por *** 668,674 **** #ifdef TCP_NODELAY on = 1; ! if (setsockopt(port->sock, IPPROTO_TCP, TCP_NODELAY, (char *) &on, sizeof(on)) < 0) { elog(LOG, "setsockopt(TCP_NODELAY) failed: %m"); --- 672,678 ---- #ifdef TCP_NODELAY on = 1; ! if (setsockopt(port->rsock, IPPROTO_TCP, TCP_NODELAY, (char *) &on, sizeof(on)) < 0) { elog(LOG, "setsockopt(TCP_NODELAY) failed: %m"); *************** StreamConnection(pgsocket server_fd, Por *** 676,682 **** } #endif on = 1; ! if (setsockopt(port->sock, SOL_SOCKET, SO_KEEPALIVE, (char *) &on, sizeof(on)) < 0) { elog(LOG, "setsockopt(SO_KEEPALIVE) failed: %m"); --- 680,686 ---- } #endif on = 1; ! if (setsockopt(port->rsock, SOL_SOCKET, SO_KEEPALIVE, (char *) &on, sizeof(on)) < 0) { elog(LOG, "setsockopt(SO_KEEPALIVE) failed: %m"); *************** StreamConnection(pgsocket server_fd, Por *** 690,696 **** * http://support.microsoft.com/kb/823764/EN-US/ */ on = PQ_SEND_BUFFER_SIZE * 4; ! if (setsockopt(port->sock, SOL_SOCKET, SO_SNDBUF, (char *) &on, sizeof(on)) < 0) { elog(LOG, "setsockopt(SO_SNDBUF) failed: %m"); --- 694,700 ---- * http://support.microsoft.com/kb/823764/EN-US/ */ on = PQ_SEND_BUFFER_SIZE * 4; ! if (setsockopt(port->rsock, SOL_SOCKET, SO_SNDBUF, (char *) &on, sizeof(on)) < 0) { elog(LOG, "setsockopt(SO_SNDBUF) failed: %m"); *************** TouchSocketFiles(void) *** 779,784 **** --- 783,793 ---- * * Sets the socket non-blocking if nonblocking is TRUE, or sets it * blocking otherwise. + * + * When using a pair of pipes for communication, this affects the read side + * only; when using a bidirectional socket, it affects both read and write. + * We currently have no need for nonblocking write in the pipe case, so it's + * not worth rethinking this API (yet). * -------------------------------- */ static void *************** pq_set_nonblocking(bool nonblocking) *** 798,810 **** */ if (nonblocking) { ! if (!pg_set_noblock(MyProcPort->sock)) ereport(COMMERROR, (errmsg("could not set socket to non-blocking mode: %m"))); } else { ! if (!pg_set_block(MyProcPort->sock)) ereport(COMMERROR, (errmsg("could not set socket to blocking mode: %m"))); } --- 807,819 ---- */ if (nonblocking) { ! if (!pg_set_noblock(MyProcPort->rsock)) ereport(COMMERROR, (errmsg("could not set socket to non-blocking mode: %m"))); } else { ! if (!pg_set_block(MyProcPort->rsock)) ereport(COMMERROR, (errmsg("could not set socket to blocking mode: %m"))); } *************** pq_setkeepaliveswin32(Port *port, int id *** 1481,1487 **** ka.keepalivetime = idle * 1000; ka.keepaliveinterval = interval * 1000; ! if (WSAIoctl(port->sock, SIO_KEEPALIVE_VALS, (LPVOID) &ka, sizeof(ka), --- 1490,1496 ---- ka.keepalivetime = idle * 1000; ka.keepaliveinterval = interval * 1000; ! if (WSAIoctl(port->rsock, SIO_KEEPALIVE_VALS, (LPVOID) &ka, sizeof(ka), *************** pq_getkeepalivesidle(Port *port) *** 1520,1526 **** ACCEPT_TYPE_ARG3 size = sizeof(port->default_keepalives_idle); #ifdef TCP_KEEPIDLE ! if (getsockopt(port->sock, IPPROTO_TCP, TCP_KEEPIDLE, (char *) &port->default_keepalives_idle, &size) < 0) { --- 1529,1535 ---- ACCEPT_TYPE_ARG3 size = sizeof(port->default_keepalives_idle); #ifdef TCP_KEEPIDLE ! if (getsockopt(port->rsock, IPPROTO_TCP, TCP_KEEPIDLE, (char *) &port->default_keepalives_idle, &size) < 0) { *************** pq_getkeepalivesidle(Port *port) *** 1528,1534 **** port->default_keepalives_idle = -1; /* don't know */ } #else ! if (getsockopt(port->sock, IPPROTO_TCP, TCP_KEEPALIVE, (char *) &port->default_keepalives_idle, &size) < 0) { --- 1537,1543 ---- port->default_keepalives_idle = -1; /* don't know */ } #else ! if (getsockopt(port->rsock, IPPROTO_TCP, TCP_KEEPALIVE, (char *) &port->default_keepalives_idle, &size) < 0) { *************** pq_setkeepalivesidle(int idle, Port *por *** 1574,1587 **** idle = port->default_keepalives_idle; #ifdef TCP_KEEPIDLE ! if (setsockopt(port->sock, IPPROTO_TCP, TCP_KEEPIDLE, (char *) &idle, sizeof(idle)) < 0) { elog(LOG, "setsockopt(TCP_KEEPIDLE) failed: %m"); return STATUS_ERROR; } #else ! if (setsockopt(port->sock, IPPROTO_TCP, TCP_KEEPALIVE, (char *) &idle, sizeof(idle)) < 0) { elog(LOG, "setsockopt(TCP_KEEPALIVE) failed: %m"); --- 1583,1596 ---- idle = port->default_keepalives_idle; #ifdef TCP_KEEPIDLE ! if (setsockopt(port->rsock, IPPROTO_TCP, TCP_KEEPIDLE, (char *) &idle, sizeof(idle)) < 0) { elog(LOG, "setsockopt(TCP_KEEPIDLE) failed: %m"); return STATUS_ERROR; } #else ! if (setsockopt(port->rsock, IPPROTO_TCP, TCP_KEEPALIVE, (char *) &idle, sizeof(idle)) < 0) { elog(LOG, "setsockopt(TCP_KEEPALIVE) failed: %m"); *************** pq_getkeepalivesinterval(Port *port) *** 1618,1624 **** #ifndef WIN32 ACCEPT_TYPE_ARG3 size = sizeof(port->default_keepalives_interval); ! if (getsockopt(port->sock, IPPROTO_TCP, TCP_KEEPINTVL, (char *) &port->default_keepalives_interval, &size) < 0) { --- 1627,1633 ---- #ifndef WIN32 ACCEPT_TYPE_ARG3 size = sizeof(port->default_keepalives_interval); ! if (getsockopt(port->rsock, IPPROTO_TCP, TCP_KEEPINTVL, (char *) &port->default_keepalives_interval, &size) < 0) { *************** pq_setkeepalivesinterval(int interval, P *** 1662,1668 **** if (interval == 0) interval = port->default_keepalives_interval; ! if (setsockopt(port->sock, IPPROTO_TCP, TCP_KEEPINTVL, (char *) &interval, sizeof(interval)) < 0) { elog(LOG, "setsockopt(TCP_KEEPINTVL) failed: %m"); --- 1671,1677 ---- if (interval == 0) interval = port->default_keepalives_interval; ! if (setsockopt(port->rsock, IPPROTO_TCP, TCP_KEEPINTVL, (char *) &interval, sizeof(interval)) < 0) { elog(LOG, "setsockopt(TCP_KEEPINTVL) failed: %m"); *************** pq_getkeepalivescount(Port *port) *** 1698,1704 **** { ACCEPT_TYPE_ARG3 size = sizeof(port->default_keepalives_count); ! if (getsockopt(port->sock, IPPROTO_TCP, TCP_KEEPCNT, (char *) &port->default_keepalives_count, &size) < 0) { --- 1707,1713 ---- { ACCEPT_TYPE_ARG3 size = sizeof(port->default_keepalives_count); ! if (getsockopt(port->rsock, IPPROTO_TCP, TCP_KEEPCNT, (char *) &port->default_keepalives_count, &size) < 0) { *************** pq_setkeepalivescount(int count, Port *p *** 1737,1743 **** if (count == 0) count = port->default_keepalives_count; ! if (setsockopt(port->sock, IPPROTO_TCP, TCP_KEEPCNT, (char *) &count, sizeof(count)) < 0) { elog(LOG, "setsockopt(TCP_KEEPCNT) failed: %m"); --- 1746,1752 ---- if (count == 0) count = port->default_keepalives_count; ! if (setsockopt(port->rsock, IPPROTO_TCP, TCP_KEEPCNT, (char *) &count, sizeof(count)) < 0) { elog(LOG, "setsockopt(TCP_KEEPCNT) failed: %m"); diff --git a/src/backend/main/main.c b/src/backend/main/main.c index 33c5a0a..a301923 100644 *** a/src/backend/main/main.c --- b/src/backend/main/main.c *************** main(int argc, char *argv[]) *** 191,196 **** --- 191,198 ---- AuxiliaryProcessMain(argc, argv); /* does not return */ else if (argc > 1 && strcmp(argv[1], "--describe-config") == 0) GucInfoMain(); /* does not return */ + else if (argc > 1 && strcmp(argv[1], "--child") == 0) + ChildPostgresMain(argc, argv, get_current_username(progname)); /* does not return */ else if (argc > 1 && strcmp(argv[1], "--single") == 0) PostgresMain(argc, argv, get_current_username(progname)); /* does not return */ else diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c index 73520a6..2aac9c8 100644 *** a/src/backend/postmaster/postmaster.c --- b/src/backend/postmaster/postmaster.c *************** ServerLoop(void) *** 1430,1436 **** * We no longer need the open socket or port structure * in this process */ ! StreamClose(port->sock); ConnFree(port); } } --- 1430,1437 ---- * We no longer need the open socket or port structure * in this process */ ! StreamClose(port->rsock); ! /* wsock is same socket, do not close separately */ ConnFree(port); } } *************** ProcessStartupPacket(Port *port, bool SS *** 1630,1636 **** #endif retry1: ! if (send(port->sock, &SSLok, 1, 0) != 1) { if (errno == EINTR) goto retry1; /* if interrupted, just retry */ --- 1631,1637 ---- #endif retry1: ! if (send(port->wsock, &SSLok, 1, 0) != 1) { if (errno == EINTR) goto retry1; /* if interrupted, just retry */ *************** ConnCreate(int serverFd) *** 1984,1991 **** if (StreamConnection(serverFd, port) != STATUS_OK) { ! if (port->sock >= 0) ! StreamClose(port->sock); ConnFree(port); return NULL; } --- 1985,1993 ---- if (StreamConnection(serverFd, port) != STATUS_OK) { ! if (port->rsock >= 0) ! StreamClose(port->rsock); ! /* wsock must be same socket, do not close separately */ ConnFree(port); return NULL; } *************** BackendStartup(Port *port) *** 3363,3369 **** /* in parent, successful fork */ ereport(DEBUG2, (errmsg_internal("forked new backend, pid=%d socket=%d", ! (int) pid, (int) port->sock))); /* * Everything's been successful, it's safe to add this backend to our list --- 3365,3371 ---- /* in parent, successful fork */ ereport(DEBUG2, (errmsg_internal("forked new backend, pid=%d socket=%d", ! (int) pid, (int) port->rsock))); /* * Everything's been successful, it's safe to add this backend to our list *************** report_fork_failure_to_client(Port *port *** 3401,3413 **** strerror(errnum)); /* Set port to non-blocking. Don't do send() if this fails */ ! if (!pg_set_noblock(port->sock)) return; /* We'll retry after EINTR, but ignore all other failures */ do { ! rc = send(port->sock, buffer, strlen(buffer) + 1, 0); } while (rc < 0 && errno == EINTR); } --- 3403,3415 ---- strerror(errnum)); /* Set port to non-blocking. Don't do send() if this fails */ ! if (!pg_set_noblock(port->wsock)) return; /* We'll retry after EINTR, but ignore all other failures */ do { ! rc = send(port->wsock, buffer, strlen(buffer) + 1, 0); } while (rc < 0 && errno == EINTR); } *************** ExitPostmaster(int status) *** 4268,4273 **** --- 4270,4335 ---- proc_exit(status); } + + /* + * ChildPostgresMain - start a new-style standalone postgres process + * + * This may not belong here, but it does share a lot of code with ConnCreate + * and BackendInitialize. Basically what it has to do is set up a + * MyProcPort structure and then hand off control to PostgresMain. + * Beware that not very much stuff is initialized yet. + * + * In the future it might be interesting to support a "standalone + * multiprocess" mode in which we have a postmaster process that doesn't + * listen for connections, but does supervise autovacuum, bgwriter, etc + * auxiliary processes. So that's another reason why postmaster.c might be + * the right place for this. + */ + void + ChildPostgresMain(int argc, char *argv[], const char *username) + { + Port *port; + + if (!(port = (Port *) calloc(1, sizeof(Port)))) + ereport(FATAL, + (errcode(ERRCODE_OUT_OF_MEMORY), + errmsg("out of memory"))); + + /* + * GSSAPI specific state struct must exist even though we won't use it + */ + #if defined(ENABLE_GSS) || defined(ENABLE_SSPI) + port->gss = (pg_gssinfo *) calloc(1, sizeof(pg_gssinfo)); + if (!port->gss) + ereport(FATAL, + (errcode(ERRCODE_OUT_OF_MEMORY), + errmsg("out of memory"))); + #endif + + /* The pipe file descriptors are presumed to be 0,1 */ + port->rsock = 0; + port->wsock = 1; + + /* Default assumption about protocol to use */ + FrontendProtocol = port->proto = PG_PROTOCOL_LATEST; + + /* save process start time */ + port->SessionStartTime = GetCurrentTimestamp(); + MyStartTime = timestamptz_to_time_t(port->SessionStartTime); + + /* set these to empty in case they are needed */ + port->remote_host = ""; + port->remote_port = ""; + + MyProcPort = port; + + /* And pass off control to PostgresMain */ + PostgresMain(argc, argv, username); + + abort(); /* not reached */ + } + + /* * sigusr1_handler - handle signal conditions from child processes */ *************** save_backend_variables(BackendParameters *** 4795,4801 **** #endif { memcpy(¶m->port, port, sizeof(Port)); ! if (!write_inheritable_socket(¶m->portsocket, port->sock, childPid)) return false; strlcpy(param->DataDir, DataDir, MAXPGPATH); --- 4857,4863 ---- #endif { memcpy(¶m->port, port, sizeof(Port)); ! if (!write_inheritable_socket(¶m->portsocket, port->rsock, childPid)) return false; strlcpy(param->DataDir, DataDir, MAXPGPATH); *************** static void *** 5022,5028 **** restore_backend_variables(BackendParameters *param, Port *port) { memcpy(port, ¶m->port, sizeof(Port)); ! read_inheritable_socket(&port->sock, ¶m->portsocket); SetDataDir(param->DataDir); --- 5084,5091 ---- restore_backend_variables(BackendParameters *param, Port *port) { memcpy(port, ¶m->port, sizeof(Port)); ! read_inheritable_socket(&port->rsock, ¶m->portsocket); ! port->wsock = port->rsock; SetDataDir(param->DataDir); diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c index 38f7a3f..8cda8a3 100644 *** a/src/backend/replication/walsender.c --- b/src/backend/replication/walsender.c *************** WalSndLoop(void) *** 855,861 **** /* Sleep until something happens or replication timeout */ WaitLatchOrSocket(&MyWalSnd->latch, wakeEvents, ! MyProcPort->sock, sleeptime); /* * Check for replication timeout. Note we ignore the corner case --- 855,861 ---- /* Sleep until something happens or replication timeout */ WaitLatchOrSocket(&MyWalSnd->latch, wakeEvents, ! MyProcPort->rsock, sleeptime); /* * Check for replication timeout. Note we ignore the corner case diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c index f1248a8..d589f71 100644 *** a/src/backend/tcop/postgres.c --- b/src/backend/tcop/postgres.c *************** process_postgres_switches(int argc, char *** 3257,3264 **** { gucsource = PGC_S_ARGV; /* switches came from command line */ ! /* Ignore the initial --single argument, if present */ ! if (argc > 1 && strcmp(argv[1], "--single") == 0) { argv++; argc--; --- 3257,3266 ---- { gucsource = PGC_S_ARGV; /* switches came from command line */ ! /* Ignore the initial --single or --child argument, if present */ ! if (argc > 1 && ! (strcmp(argv[1], "--single") == 0 || ! strcmp(argv[1], "--child") == 0)) { argv++; argc--; *************** PostgresMain(int argc, char *argv[], con *** 3522,3532 **** * standalone). */ if (!IsUnderPostmaster) - { MyProcPid = getpid(); MyStartTime = time(NULL); - } /* * Fire up essential subsystems: error and memory management --- 3524,3533 ---- * standalone). */ if (!IsUnderPostmaster) MyProcPid = getpid(); + if (MyProcPort == NULL) MyStartTime = time(NULL); /* * Fire up essential subsystems: error and memory management *************** PostgresMain(int argc, char *argv[], con *** 3538,3543 **** --- 3539,3554 ---- SetProcessingMode(InitProcessing); + /* + * In child-postgres mode, we can now initialize libpq and then enable + * reporting of ereport errors to the client. + */ + if (!IsUnderPostmaster && MyProcPort != NULL) + { + pq_init(); /* initialize libpq to talk to client */ + whereToSendOutput = DestRemote; /* now safe to ereport to client */ + } + /* Compute paths, if we didn't inherit them from postmaster */ if (my_exec_path[0] == '\0') { diff --git a/src/include/libpq/libpq-be.h b/src/include/libpq/libpq-be.h index 4d92c18..3427e71 100644 *** a/src/include/libpq/libpq-be.h --- b/src/include/libpq/libpq-be.h *************** typedef struct *** 99,110 **** * still available when a backend is running (see MyProcPort). The data * it points to must also be malloc'd, or else palloc'd in TopMemoryContext, * so that it survives into PostgresMain execution! */ typedef struct Port { ! pgsocket sock; /* File descriptor */ ! bool noblock; /* is the socket in non-blocking mode? */ ProtocolVersion proto; /* FE/BE protocol version */ SockAddr laddr; /* local addr (postmaster) */ SockAddr raddr; /* remote addr (client) */ --- 99,118 ---- * still available when a backend is running (see MyProcPort). The data * it points to must also be malloc'd, or else palloc'd in TopMemoryContext, * so that it survives into PostgresMain execution! + * + * When using a socket to communicate with the frontend, rsock and wsock are + * equal. When using a pair of pipes, they are not. Note that SSL, GSS, + * etc are not supported in the latter case, since those libraries expect to + * work with bidirectional file descriptors. By convention, we use rsock + * when performing an operation that expects a socket; it doesn't really + * matter, but let's be consistent. */ typedef struct Port { ! pgsocket rsock; /* File descriptor for reading */ ! pgsocket wsock; /* File descriptor for writing */ ! bool noblock; /* is the read socket in non-blocking mode? */ ProtocolVersion proto; /* FE/BE protocol version */ SockAddr laddr; /* local addr (postmaster) */ SockAddr raddr; /* remote addr (client) */ *************** typedef struct Port *** 157,168 **** int keepalives_interval; int keepalives_count; - #if defined(ENABLE_GSS) || defined(ENABLE_SSPI) - /* * If GSSAPI is supported, store GSSAPI information. Otherwise, store a * NULL pointer to make sure offsets in the struct remain the same. */ pg_gssinfo *gss; #else void *gss; --- 165,175 ---- int keepalives_interval; int keepalives_count; /* * If GSSAPI is supported, store GSSAPI information. Otherwise, store a * NULL pointer to make sure offsets in the struct remain the same. */ + #if defined(ENABLE_GSS) || defined(ENABLE_SSPI) pg_gssinfo *gss; #else void *gss; diff --git a/src/include/postmaster/postmaster.h b/src/include/postmaster/postmaster.h index 0fe7ec2..6d4245c 100644 *** a/src/include/postmaster/postmaster.h --- b/src/include/postmaster/postmaster.h *************** extern int postmaster_alive_fds[2]; *** 47,52 **** --- 47,55 ---- extern const char *progname; extern void PostmasterMain(int argc, char *argv[]) __attribute__((noreturn)); + + extern void ChildPostgresMain(int argc, char *argv[], const char *username) __attribute__((noreturn)); + extern void ClosePostmasterPorts(bool am_syslogger); extern int MaxLivePostmasterChildren(void); diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c index 3dcd0c3..c226eb8 100644 *** a/src/interfaces/libpq/fe-connect.c --- b/src/interfaces/libpq/fe-connect.c *************** *** 17,22 **** --- 17,23 ---- #include <sys/types.h> #include <sys/stat.h> + #include <sys/wait.h> #include <fcntl.h> #include <ctype.h> #include <time.h> *************** static const PQconninfoOption PQconninfo *** 259,264 **** --- 260,271 ---- {"replication", NULL, NULL, NULL, "Replication", "D", 5}, + {"standalone_datadir", NULL, NULL, NULL, + "Standalone-Data-Directory", "D", 64}, + + {"standalone_backend", NULL, NULL, NULL, + "Standalone-Backend", "D", 64}, + /* Terminating entry --- MUST BE LAST */ {NULL, NULL, NULL, NULL, NULL, NULL, 0} *************** pgthreadlock_t pg_g_threadlock = default *** 345,350 **** --- 352,392 ---- /* + * pqDropConnection + * + * Close any physical connection to the server, and reset associated + * state inside the connection object. We don't release state that + * would be needed to reconnect, though. + */ + void + pqDropConnection(PGconn *conn) + { + /* Drop any SSL state */ + pqsecure_close(conn); + /* Close the file descriptor(s) proper */ + if (conn->rsock >= 0) + closesocket(conn->rsock); + if (conn->rsock != conn->wsock && conn->wsock >= 0) + closesocket(conn->wsock); + conn->rsock = conn->wsock = -1; + /* If we forked a child postgres process, wait for it to exit */ + if (conn->postgres_pid > 0) + { + while (waitpid(conn->postgres_pid, NULL, 0) < 0) + { + /* If interrupted by signal, keep waiting */ + if (errno != EINTR) + break; + } + } + conn->postgres_pid = -1; + /* Discard any unread/unsent data */ + conn->inStart = conn->inCursor = conn->inEnd = 0; + conn->outCount = 0; + } + + + /* * Connecting to a Database * * There are now six different ways a user of this API can connect to the *************** fillPGconn(PGconn *conn, PQconninfoOptio *** 621,626 **** --- 663,672 ---- conn->pghost = tmp ? strdup(tmp) : NULL; tmp = conninfo_getval(connOptions, "port"); conn->pgport = tmp ? strdup(tmp) : NULL; + tmp = conninfo_getval(connOptions, "standalone_datadir"); + conn->standalone_datadir = tmp ? strdup(tmp) : NULL; + tmp = conninfo_getval(connOptions, "standalone_backend"); + conn->standalone_backend = tmp ? strdup(tmp) : NULL; tmp = conninfo_getval(connOptions, "tty"); conn->pgtty = tmp ? strdup(tmp) : NULL; tmp = conninfo_getval(connOptions, "options"); *************** connectNoDelay(PGconn *conn) *** 1003,1009 **** #ifdef TCP_NODELAY int on = 1; ! if (setsockopt(conn->sock, IPPROTO_TCP, TCP_NODELAY, (char *) &on, sizeof(on)) < 0) { --- 1049,1055 ---- #ifdef TCP_NODELAY int on = 1; ! if (setsockopt(conn->rsock, IPPROTO_TCP, TCP_NODELAY, (char *) &on, sizeof(on)) < 0) { *************** setKeepalivesIdle(PGconn *conn) *** 1149,1155 **** idle = 0; #ifdef TCP_KEEPIDLE ! if (setsockopt(conn->sock, IPPROTO_TCP, TCP_KEEPIDLE, (char *) &idle, sizeof(idle)) < 0) { char sebuf[256]; --- 1195,1201 ---- idle = 0; #ifdef TCP_KEEPIDLE ! if (setsockopt(conn->rsock, IPPROTO_TCP, TCP_KEEPIDLE, (char *) &idle, sizeof(idle)) < 0) { char sebuf[256]; *************** setKeepalivesIdle(PGconn *conn) *** 1162,1168 **** #else #ifdef TCP_KEEPALIVE /* Darwin uses TCP_KEEPALIVE rather than TCP_KEEPIDLE */ ! if (setsockopt(conn->sock, IPPROTO_TCP, TCP_KEEPALIVE, (char *) &idle, sizeof(idle)) < 0) { char sebuf[256]; --- 1208,1214 ---- #else #ifdef TCP_KEEPALIVE /* Darwin uses TCP_KEEPALIVE rather than TCP_KEEPIDLE */ ! if (setsockopt(conn->rsock, IPPROTO_TCP, TCP_KEEPALIVE, (char *) &idle, sizeof(idle)) < 0) { char sebuf[256]; *************** setKeepalivesInterval(PGconn *conn) *** 1194,1200 **** interval = 0; #ifdef TCP_KEEPINTVL ! if (setsockopt(conn->sock, IPPROTO_TCP, TCP_KEEPINTVL, (char *) &interval, sizeof(interval)) < 0) { char sebuf[256]; --- 1240,1246 ---- interval = 0; #ifdef TCP_KEEPINTVL ! if (setsockopt(conn->rsock, IPPROTO_TCP, TCP_KEEPINTVL, (char *) &interval, sizeof(interval)) < 0) { char sebuf[256]; *************** setKeepalivesCount(PGconn *conn) *** 1226,1232 **** count = 0; #ifdef TCP_KEEPCNT ! if (setsockopt(conn->sock, IPPROTO_TCP, TCP_KEEPCNT, (char *) &count, sizeof(count)) < 0) { char sebuf[256]; --- 1272,1278 ---- count = 0; #ifdef TCP_KEEPCNT ! if (setsockopt(conn->rsock, IPPROTO_TCP, TCP_KEEPCNT, (char *) &count, sizeof(count)) < 0) { char sebuf[256]; *************** setKeepalivesWin32(PGconn *conn) *** 1268,1274 **** ka.keepalivetime = idle * 1000; ka.keepaliveinterval = interval * 1000; ! if (WSAIoctl(conn->sock, SIO_KEEPALIVE_VALS, (LPVOID) &ka, sizeof(ka), --- 1314,1320 ---- ka.keepalivetime = idle * 1000; ka.keepaliveinterval = interval * 1000; ! if (WSAIoctl(conn->rsock, SIO_KEEPALIVE_VALS, (LPVOID) &ka, sizeof(ka), *************** setKeepalivesWin32(PGconn *conn) *** 1289,1294 **** --- 1335,1397 ---- #endif /* SIO_KEEPALIVE_VALS */ #endif /* WIN32 */ + /* + * Fork and exec a command, connecting up pipes to its stdin and stdout. + */ + static pid_t + pg_popen2(char **argv, int *infp, int *outfp) + { + pid_t pid; + int p_stdin[2]; + int p_stdout[2]; + + if (pipe(p_stdin) != 0) + return -1; + if (pipe(p_stdout) != 0) + { + close(p_stdin[0]); + close(p_stdin[1]); + return -1; + } + + pid = fork(); + + if (pid < 0) + { + /* fork failed */ + close(p_stdin[0]); + close(p_stdin[1]); + close(p_stdout[0]); + close(p_stdout[1]); + return pid; + } + else if (pid == 0) + { + /* successful, in child process */ + close(p_stdin[1]); + close(p_stdout[0]); + dup2(p_stdin[0], 0); + close(p_stdin[0]); + dup2(p_stdout[1], 1); + close(p_stdout[1]); + + execv(argv[0], argv); + perror(argv[0]); + exit(1); + } + else + { + /* successful, in parent process */ + close(p_stdin[0]); + close(p_stdout[1]); + *infp = p_stdout[0]; + *outfp = p_stdin[1]; + } + + return pid; + } + + /* ---------- * connectDBStart - * Begin the process of making a connection to the backend. *************** connectDBStart(PGconn *conn) *** 1317,1322 **** --- 1420,1474 ---- conn->outCount = 0; /* + * If the standalone_datadir option was specified, ignore any host or + * port specifications and just try to fork a standalone backend. + */ + if (conn->standalone_datadir && conn->standalone_datadir[0]) + { + char *be_argv[10]; + + /* + * We set up the backend's command line in execv(3) style, so that + * we don't need to cope with shell quoting rules. + */ + if (conn->standalone_backend && conn->standalone_backend[0]) + be_argv[0] = conn->standalone_backend; + else /* assume we should use hard-wired path */ + be_argv[0] = PGBINDIR "/postgres"; + + be_argv[1] = "--child"; + be_argv[2] = "-D"; + be_argv[3] = conn->standalone_datadir; + be_argv[4] = (conn->dbName && conn->dbName[0]) ? conn->dbName : NULL; + be_argv[5] = NULL; + + conn->postgres_pid = pg_popen2(be_argv, &conn->rsock, &conn->wsock); + if (conn->postgres_pid < 0) + { + char sebuf[256]; + + appendPQExpBuffer(&conn->errorMessage, + libpq_gettext("could not fork standalone backend: %s\n"), + pqStrerror(errno, sebuf, sizeof(sebuf))); + goto connect_errReturn; + } + + /* + * Go directly to CONNECTION_AUTH_OK state, since the standalone + * backend is not going to issue any authentication challenge to us. + * We're just waiting for startup to conclude. + */ + #ifdef USE_SSL + conn->allow_ssl_try = false; + #endif + conn->pversion = PG_PROTOCOL(3, 0); + conn->status = CONNECTION_AUTH_OK; + conn->asyncStatus = PGASYNC_BUSY; + + return 1; + } + + /* * Determine the parameters to pass to pg_getaddrinfo_all. */ *************** connectDBStart(PGconn *conn) *** 1416,1427 **** return 1; connect_errReturn: ! if (conn->sock >= 0) ! { ! pqsecure_close(conn); ! closesocket(conn->sock); ! conn->sock = -1; ! } conn->status = CONNECTION_BAD; return 0; } --- 1568,1574 ---- return 1; connect_errReturn: ! pqDropConnection(conn); conn->status = CONNECTION_BAD; return 0; } *************** keep_going: /* We will come back to *** 1617,1624 **** conn->raddr.salen = addr_cur->ai_addrlen; /* Open a socket */ ! conn->sock = socket(addr_cur->ai_family, SOCK_STREAM, 0); ! if (conn->sock < 0) { /* * ignore socket() failure if we have more addresses --- 1764,1771 ---- conn->raddr.salen = addr_cur->ai_addrlen; /* Open a socket */ ! conn->rsock = socket(addr_cur->ai_family, SOCK_STREAM, 0); ! if (conn->rsock < 0) { /* * ignore socket() failure if we have more addresses *************** keep_going: /* We will come back to *** 1635,1640 **** --- 1782,1790 ---- break; } + /* Socket is bidirectional */ + conn->wsock = conn->rsock; + /* * Select socket options: no delay of outgoing data for * TCP sockets, nonblock mode, close-on-exec. Fail if any *************** keep_going: /* We will come back to *** 1644,1674 **** { if (!connectNoDelay(conn)) { ! closesocket(conn->sock); ! conn->sock = -1; conn->addr_cur = addr_cur->ai_next; continue; } } ! if (!pg_set_noblock(conn->sock)) { appendPQExpBuffer(&conn->errorMessage, libpq_gettext("could not set socket to non-blocking mode: %s\n"), SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf))); ! closesocket(conn->sock); ! conn->sock = -1; conn->addr_cur = addr_cur->ai_next; continue; } #ifdef F_SETFD ! if (fcntl(conn->sock, F_SETFD, FD_CLOEXEC) == -1) { appendPQExpBuffer(&conn->errorMessage, libpq_gettext("could not set socket to close-on-exec mode: %s\n"), SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf))); ! closesocket(conn->sock); ! conn->sock = -1; conn->addr_cur = addr_cur->ai_next; continue; } --- 1794,1821 ---- { if (!connectNoDelay(conn)) { ! pqDropConnection(conn); conn->addr_cur = addr_cur->ai_next; continue; } } ! if (!pg_set_noblock(conn->rsock)) { appendPQExpBuffer(&conn->errorMessage, libpq_gettext("could not set socket to non-blocking mode: %s\n"), SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf))); ! pqDropConnection(conn); conn->addr_cur = addr_cur->ai_next; continue; } #ifdef F_SETFD ! if (fcntl(conn->rsock, F_SETFD, FD_CLOEXEC) == -1) { appendPQExpBuffer(&conn->errorMessage, libpq_gettext("could not set socket to close-on-exec mode: %s\n"), SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf))); ! pqDropConnection(conn); conn->addr_cur = addr_cur->ai_next; continue; } *************** keep_going: /* We will come back to *** 1693,1699 **** /* Do nothing */ } #ifndef WIN32 ! else if (setsockopt(conn->sock, SOL_SOCKET, SO_KEEPALIVE, (char *) &on, sizeof(on)) < 0) { --- 1840,1846 ---- /* Do nothing */ } #ifndef WIN32 ! else if (setsockopt(conn->rsock, SOL_SOCKET, SO_KEEPALIVE, (char *) &on, sizeof(on)) < 0) { *************** keep_going: /* We will come back to *** 1715,1722 **** if (err) { ! closesocket(conn->sock); ! conn->sock = -1; conn->addr_cur = addr_cur->ai_next; continue; } --- 1862,1868 ---- if (err) { ! pqDropConnection(conn); conn->addr_cur = addr_cur->ai_next; continue; } *************** keep_going: /* We will come back to *** 1754,1760 **** #ifdef SO_NOSIGPIPE optval = 1; ! if (setsockopt(conn->sock, SOL_SOCKET, SO_NOSIGPIPE, (char *) &optval, sizeof(optval)) == 0) { conn->sigpipe_so = true; --- 1900,1906 ---- #ifdef SO_NOSIGPIPE optval = 1; ! if (setsockopt(conn->rsock, SOL_SOCKET, SO_NOSIGPIPE, (char *) &optval, sizeof(optval)) == 0) { conn->sigpipe_so = true; *************** keep_going: /* We will come back to *** 1766,1772 **** * Start/make connection. This should not block, since we * are in nonblock mode. If it does, well, too bad. */ ! if (connect(conn->sock, addr_cur->ai_addr, addr_cur->ai_addrlen) < 0) { if (SOCK_ERRNO == EINPROGRESS || --- 1912,1918 ---- * Start/make connection. This should not block, since we * are in nonblock mode. If it does, well, too bad. */ ! if (connect(conn->rsock, addr_cur->ai_addr, addr_cur->ai_addrlen) < 0) { if (SOCK_ERRNO == EINPROGRESS || *************** keep_going: /* We will come back to *** 1802,1812 **** * failure and keep going if there are more addresses. */ connectFailureMessage(conn, SOCK_ERRNO); ! if (conn->sock >= 0) ! { ! closesocket(conn->sock); ! conn->sock = -1; ! } /* * Try the next address, if any. --- 1948,1954 ---- * failure and keep going if there are more addresses. */ connectFailureMessage(conn, SOCK_ERRNO); ! pqDropConnection(conn); /* * Try the next address, if any. *************** keep_going: /* We will come back to *** 1835,1841 **** * state waiting for us on the socket. */ ! if (getsockopt(conn->sock, SOL_SOCKET, SO_ERROR, (char *) &optval, &optlen) == -1) { appendPQExpBuffer(&conn->errorMessage, --- 1977,1983 ---- * state waiting for us on the socket. */ ! if (getsockopt(conn->rsock, SOL_SOCKET, SO_ERROR, (char *) &optval, &optlen) == -1) { appendPQExpBuffer(&conn->errorMessage, *************** keep_going: /* We will come back to *** 1851,1856 **** --- 1993,1999 ---- * error message. */ connectFailureMessage(conn, optval); + pqDropConnection(conn); /* * If more addresses remain, keep trying, just as in the *************** keep_going: /* We will come back to *** 1858,1868 **** */ if (conn->addr_cur->ai_next != NULL) { - if (conn->sock >= 0) - { - closesocket(conn->sock); - conn->sock = -1; - } conn->addr_cur = conn->addr_cur->ai_next; conn->status = CONNECTION_NEEDED; goto keep_going; --- 2001,2006 ---- *************** keep_going: /* We will come back to *** 1872,1878 **** /* Fill in the client address */ conn->laddr.salen = sizeof(conn->laddr.addr); ! if (getsockname(conn->sock, (struct sockaddr *) & conn->laddr.addr, &conn->laddr.salen) < 0) { --- 2010,2016 ---- /* Fill in the client address */ conn->laddr.salen = sizeof(conn->laddr.addr); ! if (getsockname(conn->rsock, (struct sockaddr *) & conn->laddr.addr, &conn->laddr.salen) < 0) { *************** keep_going: /* We will come back to *** 1910,1916 **** gid_t gid; errno = 0; ! if (getpeereid(conn->sock, &uid, &gid) != 0) { /* * Provide special error message if getpeereid is a --- 2048,2054 ---- gid_t gid; errno = 0; ! if (getpeereid(conn->rsock, &uid, &gid) != 0) { /* * Provide special error message if getpeereid is a *************** keep_going: /* We will come back to *** 2137,2148 **** /* only retry once */ conn->allow_ssl_try = false; /* Must drop the old connection */ ! closesocket(conn->sock); ! conn->sock = -1; conn->status = CONNECTION_NEEDED; - /* Discard any unread/unsent data */ - conn->inStart = conn->inCursor = conn->inEnd = 0; - conn->outCount = 0; goto keep_going; } } --- 2275,2282 ---- /* only retry once */ conn->allow_ssl_try = false; /* Must drop the old connection */ ! pqDropConnection(conn); conn->status = CONNECTION_NEEDED; goto keep_going; } } *************** keep_going: /* We will come back to *** 2252,2264 **** { conn->pversion = PG_PROTOCOL(2, 0); /* Must drop the old connection */ ! pqsecure_close(conn); ! closesocket(conn->sock); ! conn->sock = -1; conn->status = CONNECTION_NEEDED; - /* Discard any unread/unsent data */ - conn->inStart = conn->inCursor = conn->inEnd = 0; - conn->outCount = 0; goto keep_going; } --- 2386,2393 ---- { conn->pversion = PG_PROTOCOL(2, 0); /* Must drop the old connection */ ! pqDropConnection(conn); conn->status = CONNECTION_NEEDED; goto keep_going; } *************** keep_going: /* We will come back to *** 2323,2334 **** /* only retry once */ conn->wait_ssl_try = false; /* Must drop the old connection */ ! closesocket(conn->sock); ! conn->sock = -1; conn->status = CONNECTION_NEEDED; - /* Discard any unread/unsent data */ - conn->inStart = conn->inCursor = conn->inEnd = 0; - conn->outCount = 0; goto keep_going; } --- 2452,2459 ---- /* only retry once */ conn->wait_ssl_try = false; /* Must drop the old connection */ ! pqDropConnection(conn); conn->status = CONNECTION_NEEDED; goto keep_going; } *************** keep_going: /* We will come back to *** 2343,2355 **** /* only retry once */ conn->allow_ssl_try = false; /* Must drop the old connection */ ! pqsecure_close(conn); ! closesocket(conn->sock); ! conn->sock = -1; conn->status = CONNECTION_NEEDED; - /* Discard any unread/unsent data */ - conn->inStart = conn->inCursor = conn->inEnd = 0; - conn->outCount = 0; goto keep_going; } #endif --- 2468,2475 ---- /* only retry once */ conn->allow_ssl_try = false; /* Must drop the old connection */ ! pqDropConnection(conn); conn->status = CONNECTION_NEEDED; goto keep_going; } #endif *************** keep_going: /* We will come back to *** 2509,2521 **** PQclear(res); conn->send_appname = false; /* Must drop the old connection */ ! pqsecure_close(conn); ! closesocket(conn->sock); ! conn->sock = -1; conn->status = CONNECTION_NEEDED; - /* Discard any unread/unsent data */ - conn->inStart = conn->inCursor = conn->inEnd = 0; - conn->outCount = 0; goto keep_going; } } --- 2629,2636 ---- PQclear(res); conn->send_appname = false; /* Must drop the old connection */ ! pqDropConnection(conn); conn->status = CONNECTION_NEEDED; goto keep_going; } } *************** makeEmptyPGconn(void) *** 2722,2728 **** conn->client_encoding = PG_SQL_ASCII; conn->std_strings = false; /* unless server says differently */ conn->verbosity = PQERRORS_DEFAULT; ! conn->sock = -1; conn->auth_req_received = false; conn->password_needed = false; conn->dot_pgpass_used = false; --- 2837,2844 ---- conn->client_encoding = PG_SQL_ASCII; conn->std_strings = false; /* unless server says differently */ conn->verbosity = PQERRORS_DEFAULT; ! conn->rsock = conn->wsock = -1; ! conn->postgres_pid = -1; conn->auth_req_received = false; conn->password_needed = false; conn->dot_pgpass_used = false; *************** freePGconn(PGconn *conn) *** 2802,2807 **** --- 2918,2927 ---- free(conn->pgport); if (conn->pgunixsocket) free(conn->pgunixsocket); + if (conn->standalone_datadir) + free(conn->standalone_datadir); + if (conn->standalone_backend) + free(conn->standalone_backend); if (conn->pgtty) free(conn->pgtty); if (conn->connect_timeout) *************** closePGconn(PGconn *conn) *** 2887,2893 **** * Note that the protocol doesn't allow us to send Terminate messages * during the startup phase. */ ! if (conn->sock >= 0 && conn->status == CONNECTION_OK) { /* * Try to send "close connection" message to backend. Ignore any --- 3007,3013 ---- * Note that the protocol doesn't allow us to send Terminate messages * during the startup phase. */ ! if (conn->wsock >= 0 && conn->status == CONNECTION_OK) { /* * Try to send "close connection" message to backend. Ignore any *************** closePGconn(PGconn *conn) *** 2909,2920 **** /* * Close the connection, reset all transient state, flush I/O buffers. */ ! if (conn->sock >= 0) ! { ! pqsecure_close(conn); ! closesocket(conn->sock); ! } ! conn->sock = -1; conn->status = CONNECTION_BAD; /* Well, not really _bad_ - just * absent */ conn->asyncStatus = PGASYNC_IDLE; --- 3029,3035 ---- /* * Close the connection, reset all transient state, flush I/O buffers. */ ! pqDropConnection(conn); conn->status = CONNECTION_BAD; /* Well, not really _bad_ - just * absent */ conn->asyncStatus = PGASYNC_IDLE; *************** closePGconn(PGconn *conn) *** 2943,2950 **** if (conn->lobjfuncs) free(conn->lobjfuncs); conn->lobjfuncs = NULL; - conn->inStart = conn->inCursor = conn->inEnd = 0; - conn->outCount = 0; #ifdef ENABLE_GSS { OM_uint32 min_s; --- 3058,3063 ---- *************** PQgetCancel(PGconn *conn) *** 3115,3121 **** if (!conn) return NULL; ! if (conn->sock < 0) return NULL; cancel = malloc(sizeof(PGcancel)); --- 3228,3234 ---- if (!conn) return NULL; ! if (conn->rsock < 0) return NULL; cancel = malloc(sizeof(PGcancel)); *************** PQrequestCancel(PGconn *conn) *** 3296,3302 **** if (!conn) return FALSE; ! if (conn->sock < 0) { strlcpy(conn->errorMessage.data, "PQrequestCancel() -- connection is not open\n", --- 3409,3415 ---- if (!conn) return FALSE; ! if (conn->rsock < 0) { strlcpy(conn->errorMessage.data, "PQrequestCancel() -- connection is not open\n", *************** PQsocket(const PGconn *conn) *** 5232,5238 **** { if (!conn) return -1; ! return conn->sock; } int --- 5345,5351 ---- { if (!conn) return -1; ! return conn->rsock; } int diff --git a/src/interfaces/libpq/fe-exec.c b/src/interfaces/libpq/fe-exec.c index 77124ef..0c81803 100644 *** a/src/interfaces/libpq/fe-exec.c --- b/src/interfaces/libpq/fe-exec.c *************** PQfn(PGconn *conn, *** 2523,2529 **** /* clear the error string */ resetPQExpBuffer(&conn->errorMessage); ! if (conn->sock < 0 || conn->asyncStatus != PGASYNC_IDLE || conn->result != NULL) { printfPQExpBuffer(&conn->errorMessage, --- 2523,2529 ---- /* clear the error string */ resetPQExpBuffer(&conn->errorMessage); ! if (conn->rsock < 0 || conn->asyncStatus != PGASYNC_IDLE || conn->result != NULL) { printfPQExpBuffer(&conn->errorMessage, diff --git a/src/interfaces/libpq/fe-misc.c b/src/interfaces/libpq/fe-misc.c index b5e5519..a34d203 100644 *** a/src/interfaces/libpq/fe-misc.c --- b/src/interfaces/libpq/fe-misc.c *************** static int pqPutMsgBytes(const void *buf *** 64,70 **** static int pqSendSome(PGconn *conn, int len); static int pqSocketCheck(PGconn *conn, int forRead, int forWrite, time_t end_time); ! static int pqSocketPoll(int sock, int forRead, int forWrite, time_t end_time); /* * PQlibVersion: return the libpq version number --- 64,72 ---- static int pqSendSome(PGconn *conn, int len); static int pqSocketCheck(PGconn *conn, int forRead, int forWrite, time_t end_time); ! static int pqSocketPoll(int rsock, int wsock, int forRead, int forWrite, ! time_t end_time); ! /* * PQlibVersion: return the libpq version number *************** pqReadData(PGconn *conn) *** 605,611 **** int someread = 0; int nread; ! if (conn->sock < 0) { printfPQExpBuffer(&conn->errorMessage, libpq_gettext("connection not open\n")); --- 607,613 ---- int someread = 0; int nread; ! if (conn->rsock < 0) { printfPQExpBuffer(&conn->errorMessage, libpq_gettext("connection not open\n")); *************** retry4: *** 780,790 **** * has been set already. */ definitelyFailed: conn->status = CONNECTION_BAD; /* No more connection to backend */ - pqsecure_close(conn); - closesocket(conn->sock); - conn->sock = -1; - return -1; } --- 782,789 ---- * has been set already. */ definitelyFailed: + pqDropConnection(conn); conn->status = CONNECTION_BAD; /* No more connection to backend */ return -1; } *************** pqSendSome(PGconn *conn, int len) *** 804,810 **** int remaining = conn->outCount; int result = 0; ! if (conn->sock < 0) { printfPQExpBuffer(&conn->errorMessage, libpq_gettext("connection not open\n")); --- 803,809 ---- int remaining = conn->outCount; int result = 0; ! if (conn->rsock < 0) { printfPQExpBuffer(&conn->errorMessage, libpq_gettext("connection not open\n")); *************** pqSocketCheck(PGconn *conn, int forRead, *** 1013,1019 **** if (!conn) return -1; ! if (conn->sock < 0) { printfPQExpBuffer(&conn->errorMessage, libpq_gettext("socket not open\n")); --- 1012,1018 ---- if (!conn) return -1; ! if (conn->rsock < 0) { printfPQExpBuffer(&conn->errorMessage, libpq_gettext("socket not open\n")); *************** pqSocketCheck(PGconn *conn, int forRead, *** 1031,1037 **** /* We will retry as long as we get EINTR */ do ! result = pqSocketPoll(conn->sock, forRead, forWrite, end_time); while (result < 0 && SOCK_ERRNO == EINTR); if (result < 0) --- 1030,1037 ---- /* We will retry as long as we get EINTR */ do ! result = pqSocketPoll(conn->rsock, conn->wsock, forRead, forWrite, ! end_time); while (result < 0 && SOCK_ERRNO == EINTR); if (result < 0) *************** pqSocketCheck(PGconn *conn, int forRead, *** 1048,1054 **** /* ! * Check a file descriptor for read and/or write data, possibly waiting. * If neither forRead nor forWrite are set, immediately return a timeout * condition (without waiting). Return >0 if condition is met, 0 * if a timeout occurred, -1 if an error or interrupt occurred. --- 1048,1054 ---- /* ! * Check file descriptor(s) for read and/or write data, possibly waiting. * If neither forRead nor forWrite are set, immediately return a timeout * condition (without waiting). Return >0 if condition is met, 0 * if a timeout occurred, -1 if an error or interrupt occurred. *************** pqSocketCheck(PGconn *conn, int forRead, *** 1057,1080 **** * if end_time is 0 (or indeed, any time before now). */ static int ! pqSocketPoll(int sock, int forRead, int forWrite, time_t end_time) { /* We use poll(2) if available, otherwise select(2) */ #ifdef HAVE_POLL ! struct pollfd input_fd; int timeout_ms; if (!forRead && !forWrite) return 0; ! input_fd.fd = sock; ! input_fd.events = POLLERR; ! input_fd.revents = 0; ! ! if (forRead) ! input_fd.events |= POLLIN; ! if (forWrite) ! input_fd.events |= POLLOUT; /* Compute appropriate timeout interval */ if (end_time == ((time_t) -1)) --- 1057,1100 ---- * if end_time is 0 (or indeed, any time before now). */ static int ! pqSocketPoll(int rsock, int wsock, int forRead, int forWrite, time_t end_time) { /* We use poll(2) if available, otherwise select(2) */ #ifdef HAVE_POLL ! struct pollfd poll_fds[2]; ! int nfds; int timeout_ms; if (!forRead && !forWrite) return 0; ! if (forRead && forWrite && rsock != wsock) ! { ! /* need two pollfds; this path is uncommon */ ! poll_fds[0].fd = rsock; ! poll_fds[0].events = POLLIN | POLLERR; ! poll_fds[0].revents = 0; ! poll_fds[1].fd = wsock; ! poll_fds[1].events = POLLOUT | POLLERR; ! poll_fds[1].revents = 0; ! nfds = 2; ! } ! else ! { ! poll_fds[0].events = POLLERR; ! poll_fds[0].revents = 0; ! if (forRead) ! { ! poll_fds[0].fd = rsock; ! poll_fds[0].events |= POLLIN; ! } ! if (forWrite) ! { ! poll_fds[0].fd = wsock; ! poll_fds[0].events |= POLLOUT; ! } ! nfds = 1; ! } /* Compute appropriate timeout interval */ if (end_time == ((time_t) -1)) *************** pqSocketPoll(int sock, int forRead, int *** 1089,1100 **** timeout_ms = 0; } ! return poll(&input_fd, 1, timeout_ms); #else /* !HAVE_POLL */ fd_set input_mask; fd_set output_mask; fd_set except_mask; struct timeval timeout; struct timeval *ptr_timeout; --- 1109,1121 ---- timeout_ms = 0; } ! return poll(poll_fds, nfds, timeout_ms); #else /* !HAVE_POLL */ fd_set input_mask; fd_set output_mask; fd_set except_mask; + int maxfd; struct timeval timeout; struct timeval *ptr_timeout; *************** pqSocketPoll(int sock, int forRead, int *** 1104,1115 **** FD_ZERO(&input_mask); FD_ZERO(&output_mask); FD_ZERO(&except_mask); if (forRead) ! FD_SET(sock, &input_mask); if (forWrite) ! FD_SET(sock, &output_mask); ! FD_SET(sock, &except_mask); /* Compute appropriate timeout interval */ if (end_time == ((time_t) -1)) --- 1125,1145 ---- FD_ZERO(&input_mask); FD_ZERO(&output_mask); FD_ZERO(&except_mask); + maxfd = -1; + if (forRead) ! { ! FD_SET(rsock, &input_mask); ! FD_SET(rsock, &except_mask); ! maxfd = rsock; ! } if (forWrite) ! { ! FD_SET(wsock, &output_mask); ! FD_SET(wsock, &except_mask); ! maxfd = Max(maxfd, wsock); ! } /* Compute appropriate timeout interval */ if (end_time == ((time_t) -1)) *************** pqSocketPoll(int sock, int forRead, int *** 1126,1132 **** ptr_timeout = &timeout; } ! return select(sock + 1, &input_mask, &output_mask, &except_mask, ptr_timeout); #endif /* HAVE_POLL */ } --- 1156,1162 ---- ptr_timeout = &timeout; } ! return select(maxfd + 1, &input_mask, &output_mask, &except_mask, ptr_timeout); #endif /* HAVE_POLL */ } diff --git a/src/interfaces/libpq/fe-protocol2.c b/src/interfaces/libpq/fe-protocol2.c index 1ba5885..16faca9 100644 *** a/src/interfaces/libpq/fe-protocol2.c --- b/src/interfaces/libpq/fe-protocol2.c *************** pqGetline2(PGconn *conn, char *s, int ma *** 1211,1217 **** { int result = 1; /* return value if buffer overflows */ ! if (conn->sock < 0 || conn->asyncStatus != PGASYNC_COPY_OUT) { *s = '\0'; --- 1211,1217 ---- { int result = 1; /* return value if buffer overflows */ ! if (conn->rsock < 0 || conn->asyncStatus != PGASYNC_COPY_OUT) { *s = '\0'; diff --git a/src/interfaces/libpq/fe-protocol3.c b/src/interfaces/libpq/fe-protocol3.c index d289f82..ab88109 100644 *** a/src/interfaces/libpq/fe-protocol3.c --- b/src/interfaces/libpq/fe-protocol3.c *************** handleSyncLoss(PGconn *conn, char id, in *** 430,438 **** pqSaveErrorResult(conn); conn->asyncStatus = PGASYNC_READY; /* drop out of GetResult wait loop */ ! pqsecure_close(conn); ! closesocket(conn->sock); ! conn->sock = -1; conn->status = CONNECTION_BAD; /* No more connection to backend */ } --- 430,436 ---- pqSaveErrorResult(conn); conn->asyncStatus = PGASYNC_READY; /* drop out of GetResult wait loop */ ! pqDropConnection(conn); conn->status = CONNECTION_BAD; /* No more connection to backend */ } *************** pqGetline3(PGconn *conn, char *s, int ma *** 1539,1545 **** { int status; ! if (conn->sock < 0 || conn->asyncStatus != PGASYNC_COPY_OUT || conn->copy_is_binary) { --- 1537,1543 ---- { int status; ! if (conn->rsock < 0 || conn->asyncStatus != PGASYNC_COPY_OUT || conn->copy_is_binary) { diff --git a/src/interfaces/libpq/fe-secure.c b/src/interfaces/libpq/fe-secure.c index b1ad776..22adf12 100644 *** a/src/interfaces/libpq/fe-secure.c --- b/src/interfaces/libpq/fe-secure.c *************** pqsecure_open_client(PGconn *conn) *** 257,263 **** /* Create a connection-specific SSL object */ if (!(conn->ssl = SSL_new(SSL_context)) || !SSL_set_app_data(conn->ssl, conn) || ! !SSL_set_fd(conn->ssl, conn->sock)) { char *err = SSLerrmessage(); --- 257,263 ---- /* Create a connection-specific SSL object */ if (!(conn->ssl = SSL_new(SSL_context)) || !SSL_set_app_data(conn->ssl, conn) || ! !SSL_set_fd(conn->ssl, conn->rsock)) { char *err = SSLerrmessage(); *************** rloop: *** 418,424 **** else #endif /* USE_SSL */ { ! n = recv(conn->sock, ptr, len, 0); if (n < 0) { --- 418,424 ---- else #endif /* USE_SSL */ { ! n = read(conn->rsock, ptr, len); if (n < 0) { *************** retry_masked: *** 588,594 **** DISABLE_SIGPIPE(conn, spinfo, return -1); ! n = send(conn->sock, ptr, len, flags); if (n < 0) { --- 588,597 ---- DISABLE_SIGPIPE(conn, spinfo, return -1); ! if (flags) ! n = send(conn->wsock, ptr, len, flags); ! else ! n = write(conn->wsock, ptr, len); if (n < 0) { *************** retry_masked: *** 600,606 **** * try the flag again, and retry the send(). */ #ifdef MSG_NOSIGNAL ! if (flags != 0 && result_errno == EINVAL) { conn->sigpipe_flag = false; flags = 0; --- 603,610 ---- * try the flag again, and retry the send(). */ #ifdef MSG_NOSIGNAL ! if (flags != 0 && ! (result_errno == EINVAL || result_errno == ENOTSOCK)) { conn->sigpipe_flag = false; flags = 0; diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h index 2bac59c..f146bac 100644 *** a/src/interfaces/libpq/libpq-int.h --- b/src/interfaces/libpq/libpq-int.h *************** struct pg_conn *** 303,308 **** --- 303,310 ---- char *pgunixsocket; /* the Unix-domain socket that the server is * listening on; if NULL, uses a default * constructed from pgport */ + char *standalone_datadir; /* data directory for standalone backend */ + char *standalone_backend; /* executable for standalone backend */ char *pgtty; /* tty on which the backend messages is * displayed (OBSOLETE, NOT USED) */ char *connect_timeout; /* connection timeout (numeric string) */ *************** struct pg_conn *** 360,367 **** PGnotify *notifyHead; /* oldest unreported Notify msg */ PGnotify *notifyTail; /* newest unreported Notify msg */ /* Connection data */ - int sock; /* Unix FD for socket, -1 if not connected */ SockAddr laddr; /* Local address */ SockAddr raddr; /* Remote address */ ProtocolVersion pversion; /* FE/BE protocol version in use */ --- 362,383 ---- PGnotify *notifyHead; /* oldest unreported Notify msg */ PGnotify *notifyTail; /* newest unreported Notify msg */ + /* + * File descriptors for the connection to the backend. Normally we use a + * bidirectional socket, so that rsock and wsock are equal. However, we + * can also use a pair of pipes, in which case they're not. Note that + * SSL, GSS, and much of the authentication code only work for the + * bidirectional-socket case. By convention, we use rsock when performing + * an operation that expects a socket; it doesn't really matter, but let's + * be consistent. + */ + int rsock; /* read FD, or -1 if not connected */ + int wsock; /* write FD, or -1 if not connected */ + + /* If we forked a child postgres process, its PID is kept here */ + pid_t postgres_pid; /* PID, or -1 if none */ + /* Connection data */ SockAddr laddr; /* Local address */ SockAddr raddr; /* Remote address */ ProtocolVersion pversion; /* FE/BE protocol version in use */ *************** extern char *const pgresStatus[]; *** 488,493 **** --- 504,510 ---- /* === in fe-connect.c === */ + extern void pqDropConnection(PGconn *conn); extern int pqPacketSend(PGconn *conn, char pack_type, const void *buf, size_t buf_len); extern bool pqGetHomeDirectory(char *buf, int bufsize);
On 03.09.2012 03:23, Tom Lane wrote: > 1. As you can see above, the feature is triggered by specifying the new > connection option "standalone_datadir", whose value must be the location > of the data directory. I also invented an option "standalone_backend", > which can be set to specify which postgres executable to launch. If the > latter isn't specified, libpq defaults to trying the installation PGBINDIR > that was selected by configure. (I don't think it can use the relative > path tricks we use in pg_ctl and elsewhere, since there's no good reason > to assume that it's running in a Postgres-supplied program.) I'm not > particularly wedded to these names or the syntax, but I think this is the > basic functionality we'd need. Are there security issues with this? If a user can specify libpq connection options, he can now execute any file he wants by passing it as standalone_backend. Granted, you shouldn't allow an untrusted user to specify libpq connection options, because allowing to open a TCP connection to an arbitrary address can be a problem by itself, but it seems like this might make the situation much worse. contrib/dblink springs to mind.. > 3. The bulk of the changes have to do with the fact that we need to keep > track of two file descriptors not one. This was a bit tedious, but fairly > straightforward --- though I was surprised to find that send() and recv() > don't work on pipes, at least not on Linux. You have to use read() and > write() instead. Would socketpair(2) be simpler? > 7. I haven't tried to make pg_upgrade use this yet. Hmm, pg_upgrade uses pg_dumpall rather than pg_dump, so it needs to connect once per database. That means launching the standalone backend multiple times. I guess that works, as long as pg_dumpall doesn't try to hold multiple connections open at any one time. - Heikki
Heikki Linnakangas <hlinnaka@iki.fi> writes: > On 03.09.2012 03:23, Tom Lane wrote: >> 1. As you can see above, the feature is triggered by specifying the new >> connection option "standalone_datadir", whose value must be the location >> of the data directory. I also invented an option "standalone_backend", >> which can be set to specify which postgres executable to launch. > Are there security issues with this? If a user can specify libpq > connection options, he can now execute any file he wants by passing it > as standalone_backend. Granted, you shouldn't allow an untrusted user to > specify libpq connection options, because allowing to open a TCP > connection to an arbitrary address can be a problem by itself, but it > seems like this might make the situation much worse. contrib/dblink > springs to mind.. Hmm, that's a good point. Maybe we should only allow the executable name to come from an environment variable? Seems kinda klugy though. >> 3. The bulk of the changes have to do with the fact that we need to keep >> track of two file descriptors not one. > Would socketpair(2) be simpler? Hm, yes, but is it portable enough? It seems to be required by SUS v2, so we're likely okay on the Unix side, but does Windows have this? regards, tom lane
On Sun, Sep 02, 2012 at 11:34:42PM -0400, Tom Lane wrote: > Heikki Linnakangas <hlinnaka@iki.fi> writes: > > On 03.09.2012 03:23, Tom Lane wrote: > >> 1. As you can see above, the feature is triggered by specifying the new > >> connection option "standalone_datadir", whose value must be the location > >> of the data directory. I also invented an option "standalone_backend", > >> which can be set to specify which postgres executable to launch. > > > Are there security issues with this? If a user can specify libpq > > connection options, he can now execute any file he wants by passing it > > as standalone_backend. Granted, you shouldn't allow an untrusted user to > > specify libpq connection options, because allowing to open a TCP > > connection to an arbitrary address can be a problem by itself, but it > > seems like this might make the situation much worse. contrib/dblink > > springs to mind.. > > Hmm, that's a good point. Maybe we should only allow the executable > name to come from an environment variable? Seems kinda klugy though. I don't think it's libpq's job to enforce security policy this way. In any event, it has no reason to presume an environment variable is a safer source. > >> 3. The bulk of the changes have to do with the fact that we need to keep > >> track of two file descriptors not one. > > > Would socketpair(2) be simpler? > > Hm, yes, but is it portable enough? It seems to be required by SUS v2, > so we're likely okay on the Unix side, but does Windows have this? Windows does not have socketpair(), nor a strict pipe() equivalent. I expect switching to socketpair() makes the Windows side trickier in some ways and simpler in others. +1 for exploring that direction first.
Noah Misch <noah@leadboat.com> writes: > On Sun, Sep 02, 2012 at 11:34:42PM -0400, Tom Lane wrote: >> Heikki Linnakangas <hlinnaka@iki.fi> writes: >>> Are there security issues with this? If a user can specify libpq >>> connection options, he can now execute any file he wants by passing it >>> as standalone_backend. >> Hmm, that's a good point. Maybe we should only allow the executable >> name to come from an environment variable? Seems kinda klugy though. > I don't think it's libpq's job to enforce security policy this way. In any > event, it has no reason to presume an environment variable is a safer source. One easy thing we could do that would at least narrow the risks is to only allow the executable's *directory* to be specified, hardwiring the executable file name to "postgres" (or "postgres.exe" I guess). I'm inclined to think though that environment variables *are* more secure in this context. In the contrib/dblink example, such a restriction would certainly help a lot. In any case, we should at least think of a way that an app using libpq can prevent this type of attack short of parsing the conn string for itself. regards, tom lane
Noah Misch <noah@leadboat.com> writes: > Windows does not have socketpair(), nor a strict pipe() equivalent. I expect > switching to socketpair() makes the Windows side trickier in some ways and > simpler in others. +1 for exploring that direction first. A bit of googling suggests that emulating socketpair() on Windows is not that hard: basically you create an accepting socket and connect to it. Ugly I guess but likely to be nicer than emulating the two-pipes trick exactly. regards, tom lane
On Mon, Sep 03, 2012 at 12:11:20AM -0400, Tom Lane wrote: > Noah Misch <noah@leadboat.com> writes: > > I don't think it's libpq's job to enforce security policy this way. In any > > event, it has no reason to presume an environment variable is a safer source. > > One easy thing we could do that would at least narrow the risks is to > only allow the executable's *directory* to be specified, hardwiring the > executable file name to "postgres" (or "postgres.exe" I guess). If the user has any interactive access to the machine, he could make a /tmp/X/postgres symbolic link to the program of his choosing. > I'm inclined to think though that environment variables *are* more > secure in this context. In the contrib/dblink example, such a > restriction would certainly help a lot. dblink_connect() should only let superusers specify standalone_datadir at all; normal users have no business manipulating other data directories visible to the backend running dblink. For superusers, setting both standalone_datadir and standalone_backend is fair game. Trusting the environment over connection strings is a wrong policy for, say, a setuid command-line program. Suppose such a program passes libpq a fixed connection string to access its embedded database. The program will find itself explicitly clearing this environment variable to regain safety. > In any case, we should at > least think of a way that an app using libpq can prevent this type > of attack short of parsing the conn string for itself. My recommendation to affected application authors is to pass the connection string through PQconninfoParse(). Walk the array, validating or rejecting arguments like "host" and "standalone_datadir". For maximum safety, the application would need to reject any unanticipated parameters. We might soften that by adding a "bool critical" field to PQconninfoOption that documents whether the option must be understood by a program validating a connection. Compare the idea of the PNG chunk header's "ancillary" bit. Parameters like "host" and "standalone_datadir" would be critical, while "application_name" and "connect_timeout" would be ancillary. But this is a tough line to draw rigorously. I was pondering the flexibility from letting the application fork/exec and supply the client-side descriptor number(s) to libpq. Suppose it wants to redirect the backend's stderr to a file. A single-threaded application would temporarily redirect its own stderr while calling PQconnectdb(). In a multithreaded application, that introduces a race condition when other threads concurrently write to the normal stderr. By handling redirection in its own fork/exec code, the application can achieve the outcome safely. Perhaps offering this can wait until the feature constitutes a more general embedded-database offering, though. Thanks, nm
On Mon, Sep 3, 2012 at 6:42 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Noah Misch <noah@leadboat.com> writes: >> Windows does not have socketpair(), nor a strict pipe() equivalent. I expect >> switching to socketpair() makes the Windows side trickier in some ways and >> simpler in others. +1 for exploring that direction first. > > A bit of googling suggests that emulating socketpair() on Windows is not > that hard: basically you create an accepting socket and connect to it. > Ugly I guess but likely to be nicer than emulating the two-pipes trick > exactly. That sounds a lot like what we were doing in pgpipe() before.. It was removed in d2c1740dc275543a46721ed254ba3623f63d2204, but that's because it was dead at the time. Do we need to bring it back? -- Magnus HaganderMe: http://www.hagander.net/Work: http://www.redpill-linpro.com/
> 5. The fork/exec code is pretty primitive with respect to error handling. > I didn't put much time into it since I'm afraid we may need to refactor it entirely before a Windows equivalent can be > written. (And I need somebody to write/test the Windows equivalent - any volunteers?) I think part of the code for windows can be written by referring function internal_forkexec(), If you are okay, I can take up this. Please confirm. > 8. PQcancel needs some work - it can't do what it does now, but it could do kill(conn->postgres_pid, SIGINT) instead. > At least in Unix. I have no idea what we'd do in Windows. This doesn't matter for pg_upgrade of course, but it'd be > important for manual use of this mode. Can pgkill(int pid, int sig) API of PG be used to achieve the same on Windows. With Regards, Amit Kapila.
Amit Kapila <amit.kapila@huawei.com> writes: > I think part of the code for windows can be written by referring function > internal_forkexec(), > If you are okay, I can take up this. Please confirm. Nobody else volunteered, so have at it. Note that I'm planning to redo that code to use socketpair(), so possibly you want to wait to see that before you do anything. >> 8. PQcancel needs some work - it can't do what it does now, but it could >> do kill(conn->postgres_pid, SIGINT) instead. At least in Unix. I have no >> idea what we'd do in Windows. This doesn't matter for pg_upgrade of course, >> but it'd be important for manual use of this mode. > Can pgkill(int pid, int sig) API of PG be used to achieve the same on > Windows. Hmm, after looking at src/port/kill.c it doesn't seem like there's much of a problem with doing that. I had had the idea that our kill emulation only worked within the backend environment, but of course pg_ctl wouldn't work if that were so. So this is easier than I thought. regards, tom lane
On Mon, Sep 3, 2012 at 7:07 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Amit Kapila <amit.kapila@huawei.com> writes: >>> 8. PQcancel needs some work - it can't do what it does now, but it could >>> do kill(conn->postgres_pid, SIGINT) instead. At least in Unix. I have no >>> idea what we'd do in Windows. This doesn't matter for pg_upgrade of course, >>> but it'd be important for manual use of this mode. > >> Can pgkill(int pid, int sig) API of PG be used to achieve the same on >> Windows. > > Hmm, after looking at src/port/kill.c it doesn't seem like there's much > of a problem with doing that. I had had the idea that our kill > emulation only worked within the backend environment, but of course > pg_ctl wouldn't work if that were so. So this is easier than I thought. Yeah, kill works fine from non-backend as long as the *receiver* has our backend environment. -- Magnus HaganderMe: http://www.hagander.net/Work: http://www.redpill-linpro.com/
Magnus Hagander <magnus@hagander.net> writes: > On Mon, Sep 3, 2012 at 7:07 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >> Hmm, after looking at src/port/kill.c it doesn't seem like there's much >> of a problem with doing that. I had had the idea that our kill >> emulation only worked within the backend environment, but of course >> pg_ctl wouldn't work if that were so. So this is easier than I thought. > Yeah, kill works fine from non-backend as long as the *receiver* has > our backend environment. I have another question after thinking about that for awhile: is there any security concern there? On Unix-oid systems, we expect the kernel to restrict who can do a kill() on a postgres process. If there's any similar restriction on who can send to that named pipe in the Windows version, it's not obvious from the code. Do we have/need any restriction there? regards, tom lane
On Mon, Sep 3, 2012 at 8:51 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Magnus Hagander <magnus@hagander.net> writes: >> On Mon, Sep 3, 2012 at 7:07 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >>> Hmm, after looking at src/port/kill.c it doesn't seem like there's much >>> of a problem with doing that. I had had the idea that our kill >>> emulation only worked within the backend environment, but of course >>> pg_ctl wouldn't work if that were so. So this is easier than I thought. > >> Yeah, kill works fine from non-backend as long as the *receiver* has >> our backend environment. > > I have another question after thinking about that for awhile: is there > any security concern there? On Unix-oid systems, we expect the kernel > to restrict who can do a kill() on a postgres process. If there's any > similar restriction on who can send to that named pipe in the Windows > version, it's not obvious from the code. Do we have/need any > restriction there? We use the default for CreateNamedPipe() which is: " The ACLs in the default security descriptor for a named pipe grant full control to the LocalSystem account, administrators, and the creator owner. They also grant read access to members of the Everyone group and the anonymous account." (ref: http://msdn.microsoft.com/en-us/library/windows/desktop/aa365150(v=vs.85).aspx) Given that we only respond to writes (we don't "publish information" over it), I think that's a reasonable default to use. -- Magnus HaganderMe: http://www.hagander.net/Work: http://www.redpill-linpro.com/
Magnus Hagander <magnus@hagander.net> writes: > On Mon, Sep 3, 2012 at 8:51 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >> I have another question after thinking about that for awhile: is there >> any security concern there? On Unix-oid systems, we expect the kernel >> to restrict who can do a kill() on a postgres process. If there's any >> similar restriction on who can send to that named pipe in the Windows >> version, it's not obvious from the code. Do we have/need any >> restriction there? > We use the default for CreateNamedPipe() which is: > " The ACLs in the default security descriptor for a named pipe grant > full control to the LocalSystem account, administrators, and the > creator owner. They also grant read access to members of the Everyone > group and the anonymous account." > (ref: http://msdn.microsoft.com/en-us/library/windows/desktop/aa365150(v=vs.85).aspx) Hm. The write protections sound fine ... but what's the semantics of reading, is it like Unix pipes? If so, couldn't a random third party drain the pipe by reading from it, and thereby cause signals to be lost? regards, tom lane
Noah Misch <noah@leadboat.com> writes: > On Mon, Sep 03, 2012 at 12:11:20AM -0400, Tom Lane wrote: >> One easy thing we could do that would at least narrow the risks is to >> only allow the executable's *directory* to be specified, hardwiring the >> executable file name to "postgres" (or "postgres.exe" I guess). > If the user has any interactive access to the machine, he could make a > /tmp/X/postgres symbolic link to the program of his choosing. I said "narrow", not "eliminate" ;-) >> I'm inclined to think though that environment variables *are* more >> secure in this context. In the contrib/dblink example, such a >> restriction would certainly help a lot. > Trusting the environment over connection strings is a wrong policy for, say, a > setuid command-line program. Suppose such a program passes libpq a fixed > connection string to access its embedded database. The program will find > itself explicitly clearing this environment variable to regain safety. Well, a program that was concerned with this would *already* want to clear every environment variable that affects libpq, else its database connections could get redirected somewhere surprising. libpq already provides enough infrastructure to get the list of such variables ... but only ones that are associated with connection string parameters. So for this purpose, making the standalone-control parameters not be accessible through connection strings would actually be worse. >> In any case, we should at >> least think of a way that an app using libpq can prevent this type >> of attack short of parsing the conn string for itself. > My recommendation to affected application authors is to pass the connection > string through PQconninfoParse(). Walk the array, validating or rejecting > arguments like "host" and "standalone_datadir". For maximum safety, the > application would need to reject any unanticipated parameters. We might > soften that by adding a "bool critical" field to PQconninfoOption that > documents whether the option must be understood by a program validating a > connection. Compare the idea of the PNG chunk header's "ancillary" bit. > Parameters like "host" and "standalone_datadir" would be critical, while > "application_name" and "connect_timeout" would be ancillary. But this is a > tough line to draw rigorously. Even more to the point, we can't seriously expect application authors to know what to do with connection parameters that didn't exist when they were writing their code. Every new security-critical parameter would represent a new set of bugs. I'm reluctantly coming to the conclusion that we can't pass these parameters through the regular libpq connection string mechanism, and will have to invent something else. That's awfully nasty though; it will pretty much cripple the idea that this would be a simple way to invoke a quasi-embedded variant of Postgres. regards, tom lane
On 09/03/2012 04:23 PM, Tom Lane wrote: > I'm reluctantly coming to the conclusion that we can't pass these > parameters through the regular libpq connection string mechanism, and > will have to invent something else. That's awfully nasty though; > it will pretty much cripple the idea that this would be a simple way to > invoke a quasi-embedded variant of Postgres. > > That would be a bit sad. BTW, how would this work for things like auto_vacuum, logging_collector and so on? cheers andrew
Hi, On Monday, September 03, 2012 10:23:52 PM Tom Lane wrote: > I'm reluctantly coming to the conclusion that we can't pass these > parameters through the regular libpq connection string mechanism, and > will have to invent something else. That's awfully nasty though; > it will pretty much cripple the idea that this would be a simple way to > invoke a quasi-embedded variant of Postgres. What I am asking myself is: why does that have to go through the normal PQconnectdb* api? This is something completely new and very well might grow more features that are not necessarily easy to press into PQconnectdb(). PQServer PQstartServer(const char **keywords, const char **values); or whatever seems to be more future proof especially considering the possibility that this will grow into something more featureful. Greetings, Andres -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
Andrew Dunstan <andrew@dunslane.net> writes: > On 09/03/2012 04:23 PM, Tom Lane wrote: >> I'm reluctantly coming to the conclusion that we can't pass these >> parameters through the regular libpq connection string mechanism, and >> will have to invent something else. That's awfully nasty though; >> it will pretty much cripple the idea that this would be a simple way to >> invoke a quasi-embedded variant of Postgres. > That would be a bit sad. BTW, how would this work for things like > auto_vacuum, logging_collector and so on? There's already an "options" connection-string option for passing random GUC settings through to the connected backend. My original plan was to just add any such settings to the standalone backend's command line. In this context that would work for postmaster-level GUCs. However, if we're going to redesign this then maybe something else is more appropriate. regards, tom lane
Andres Freund <andres@2ndquadrant.com> writes: > On Monday, September 03, 2012 10:23:52 PM Tom Lane wrote: >> I'm reluctantly coming to the conclusion that we can't pass these >> parameters through the regular libpq connection string mechanism, and >> will have to invent something else. That's awfully nasty though; >> it will pretty much cripple the idea that this would be a simple way to >> invoke a quasi-embedded variant of Postgres. > What I am asking myself is: why does that have to go through the normal > PQconnectdb* api? This is something completely new and very well might grow > more features that are not necessarily easy to press into PQconnectdb(). Well, what that's mostly going to result in is a huge amount of duplication :-(. psql, pg_dump, and anything else that wants to support this will need some alternative command line switch and an alternative code path to call PQstartServer. I'd hoped to avoid all that. Note that the POC patch involved not one single line of change in those application programs. regards, tom lane
On Monday, September 03, 2012 10:54:23 PM Tom Lane wrote: > Andres Freund <andres@2ndquadrant.com> writes: > > On Monday, September 03, 2012 10:23:52 PM Tom Lane wrote: > >> I'm reluctantly coming to the conclusion that we can't pass these > >> parameters through the regular libpq connection string mechanism, and > >> will have to invent something else. That's awfully nasty though; > >> it will pretty much cripple the idea that this would be a simple way to > >> invoke a quasi-embedded variant of Postgres. > > > > What I am asking myself is: why does that have to go through the normal > > PQconnectdb* api? This is something completely new and very well might > > grow more features that are not necessarily easy to press into > > PQconnectdb(). > > Well, what that's mostly going to result in is a huge amount of > duplication :-(. psql, pg_dump, and anything else that wants to support > this will need some alternative command line switch and an alternative > code path to call PQstartServer. I'd hoped to avoid all that. Note > that the POC patch involved not one single line of change in those > application programs. I can see why that would be nice, but is it really realistic? Don't we expect some more diligence in applications using this against letting such a child continue to run after ctrl-c/SIGTERMing e.g. pg_dump in comparison to closing a normal database connection? Besides the already mentioned security issues I would argue that its a good thing to make the applications authors think about the special requirements of "embedding" PG. Greetings, Andres -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
On Monday, September 03, 2012 10:37 PM Tom Lane wrote: Amit Kapila <amit.kapila@huawei.com> writes: >> I think part of the code for windows can be written by referring function >> internal_forkexec(), >> If you are okay, I can take up this. Please confirm. > Nobody else volunteered, so have at it. Note that I'm planning to redo > that code to use socketpair(), so possibly you want to wait to see that > before you do anything. I shall wait till you post your initial version which you think is ready for me to start for write/test Windows part of feature. With Regards, Amit Kapila.
On Mon, Sep 3, 2012 at 4:38 PM, Andres Freund <andres@2ndquadrant.com> wrote: > On Monday, September 03, 2012 10:23:52 PM Tom Lane wrote: >> I'm reluctantly coming to the conclusion that we can't pass these >> parameters through the regular libpq connection string mechanism, and >> will have to invent something else. That's awfully nasty though; >> it will pretty much cripple the idea that this would be a simple way to >> invoke a quasi-embedded variant of Postgres. > What I am asking myself is: why does that have to go through the normal > PQconnectdb* api? This is something completely new and very well might grow > more features that are not necessarily easy to press into PQconnectdb(). > > PQServer > PQstartServer(const char **keywords, const char **values); > > or whatever seems to be more future proof especially considering the > possibility that this will grow into something more featureful. I tend to agree. Another idea here might be to stick with Tom's original idea of making it a connection parameter, but have it be turned off by default. In other words, if an application wants to allow those parameters to be used, it would need to do PQenableStartServer() first. If it doesn't, those connection parameters will be rejected. Not 100% sure that's enough to fix the problem, but maybe... -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes: > I tend to agree. Another idea here might be to stick with Tom's > original idea of making it a connection parameter, but have it be > turned off by default. In other words, if an application wants to > allow those parameters to be used, it would need to do > PQenableStartServer() first. If it doesn't, those connection > parameters will be rejected. Hm, that might work. regards, tom lane
Andres Freund <andres@2ndquadrant.com> writes: > I can see why that would be nice, but is it really realistic? Don't we > expect some more diligence in applications using this against letting > such a child continue to run after ctrl-c/SIGTERMing e.g. pg_dump in > comparison to closing a normal database connection? Er, what? If you kill the client, the child postgres will see connection closure and will shut down. I already tested that with the POC patch, it worked fine. regards, tom lane
On Tue, Sep 04, 2012 at 12:01:17AM -0400, Robert Haas wrote: > Another idea here might be to stick with Tom's > original idea of making it a connection parameter, but have it be > turned off by default. In other words, if an application wants to > allow those parameters to be used, it would need to do > PQenableStartServer() first. If it doesn't, those connection > parameters will be rejected. +1
On Tuesday, September 04, 2012 06:20:59 AM Tom Lane wrote: > Andres Freund <andres@2ndquadrant.com> writes: > > I can see why that would be nice, but is it really realistic? Don't we > > expect some more diligence in applications using this against letting > > such a child continue to run after ctrl-c/SIGTERMing e.g. pg_dump in > > comparison to closing a normal database connection? > > Er, what? If you kill the client, the child postgres will see > connection closure and will shut down. I already tested that with the > POC patch, it worked fine. Well, but that will make scripting harder because you cannot start another single backend pg_dump before the old backend noticed it, checkpointed and shut down. Andres -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
On Tuesday, September 04, 2012 11:00 AM Andres Freund wrote: On Tuesday, September 04, 2012 06:20:59 AM Tom Lane wrote: > Andres Freund <andres@2ndquadrant.com> writes: >> > I can see why that would be nice, but is it really realistic? Don't we >> > expect some more diligence in applications using this against letting >> > such a child continue to run after ctrl-c/SIGTERMing e.g. pg_dump in >> > comparison to closing a normal database connection? > >> Er, what? If you kill the client, the child postgres will see >> connection closure and will shut down. I already tested that with the >> POC patch, it worked fine. > Well, but that will make scripting harder because you cannot start another > single backend pg_dump before the old backend noticed it, checkpointed and > shut down. But isn't that behavior will be similar when currently server is shutting down due to CTRL-C, and at that time new clientswill not be allowed to connect. As this new interface is an approach similar to embedded database where first API(StartServer) or at connect time it starts database and the other connection might not be allowed during shutdown state. With Regards, Amit Kapila.
On Tuesday, September 04, 2012 12:40 AM Tom Lane wrote: Magnus Hagander <magnus@hagander.net> writes: > On Mon, Sep 3, 2012 at 8:51 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >>> I have another question after thinking about that for awhile: is there >>> any security concern there? On Unix-oid systems, we expect the kernel >>> to restrict who can do a kill() on a postgres process. If there's any >>> similar restriction on who can send to that named pipe in the Windows >>> version, it's not obvious from the code. Do we have/need any >>> restriction there? >> We use the default for CreateNamedPipe() which is: >> " The ACLs in the default security descriptor for a named pipe grant >> full control to the LocalSystem account, administrators, and the >> creator owner. They also grant read access to members of the Everyone >> group and the anonymous account." >> (ref: http://msdn.microsoft.com/en-us/library/windows/desktop/aa365150(v=vs.85).as px) > Hm. The write protections sound fine ... but what's the semantics of > reading, is it like Unix pipes? If so, couldn't a random third party > drain the pipe by reading from it, and thereby cause signals to be lost? When a client connects to the server-end of a named pipe, the server-end of the pipe is now dedicated to the client. No more connections will be allowed to that server pipe instance until the client has disconnected. This is from paper: http://www.blakewatts.com/namedpipepaper.html, it mentions about security issues in named pipes. The function CallNamedPipe() used for sending signal in pgkill() has following definition: Connects to a message-type pipe (and waits if an instance of the pipe is not available), writes to and reads from the pipe, and then closes the pipe. (http://msdn.microsoft.com/en-us/library/windows/desktop/aa365144(v=vs.85).a spx) So I think based on above 2 points it can be deduced that the signal sent by pgkill() cannot be read by anyone else. With Regards, Amit Kapila.
On Wed, Sep 5, 2012 at 7:31 AM, Amit Kapila <amit.kapila@huawei.com> wrote: > On Tuesday, September 04, 2012 12:40 AM Tom Lane wrote: > Magnus Hagander <magnus@hagander.net> writes: >> On Mon, Sep 3, 2012 at 8:51 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >>>> I have another question after thinking about that for awhile: is there >>>> any security concern there? On Unix-oid systems, we expect the kernel >>>> to restrict who can do a kill() on a postgres process. If there's any >>>> similar restriction on who can send to that named pipe in the Windows >>>> version, it's not obvious from the code. Do we have/need any >>>> restriction there? > >>> We use the default for CreateNamedPipe() which is: >>> " The ACLs in the default security descriptor for a named pipe grant >>> full control to the LocalSystem account, administrators, and the >>> creator owner. They also grant read access to members of the Everyone >>> group and the anonymous account." >>> (ref: > http://msdn.microsoft.com/en-us/library/windows/desktop/aa365150(v=vs.85).as > px) > >> Hm. The write protections sound fine ... but what's the semantics of >> reading, is it like Unix pipes? If so, couldn't a random third party >> drain the pipe by reading from it, and thereby cause signals to be lost? > > When a client connects to the server-end of a named pipe, the server-end > of the pipe is now dedicated to the client. No > more connections will be allowed to that server pipe instance until the > client has disconnected. This is the main argument. yes. Each client gets it's own copy, so it can't get drained. > So I think based on above 2 points it can be deduced that the signal sent > by pgkill() cannot be read by anyone else. Agreed. Well, what someone else could do is create a pipe with our name before we do (since we use the actual name - it's \\.\pipe\pgsinal_<pid>), by guessing what pid we will have. If that happens, we'll go into a loop and try to recreate it while logging a warning message to eventlog/stderr. (this happens for every backend). We can't throw an error on this and kill the backend because the pipe is created in the background thread not the main one. -- Magnus HaganderMe: http://www.hagander.net/Work: http://www.redpill-linpro.com/
On Wednesday, September 05, 2012 3:58 PM Magnus Hagander wrote: On Wed, Sep 5, 2012 at 7:31 AM, Amit Kapila <amit.kapila@huawei.com> wrote: > On Tuesday, September 04, 2012 12:40 AM Tom Lane wrote: > Magnus Hagander <magnus@hagander.net> writes: >> On Mon, Sep 3, 2012 at 8:51 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >>>>> I have another question after thinking about that for awhile: is there >>>>> any security concern there? On Unix-oid systems, we expect the kernel >>>>> to restrict who can do a kill() on a postgres process. If there's any >>>>> similar restriction on who can send to that named pipe in the Windows >>>>> version, it's not obvious from the code. Do we have/need any >>>>> restriction there? > >>>> We use the default for CreateNamedPipe() which is: >>>> " The ACLs in the default security descriptor for a named pipe grant >>>> full control to the LocalSystem account, administrators, and the >>>> creator owner. They also grant read access to members of the Everyone >>>> group and the anonymous account." >>>> (ref: > http://msdn.microsoft.com/en-us/library/windows/desktop/aa365150(v=vs.85).as > px) > >>> Hm. The write protections sound fine ... but what's the semantics of >>> reading, is it like Unix pipes? If so, couldn't a random third party >>> drain the pipe by reading from it, and thereby cause signals to be lost? > >> When a client connects to the server-end of a named pipe, the server-end >> of the pipe is now dedicated to the client. No >> more connections will be allowed to that server pipe instance until the >> client has disconnected. > This is the main argument. yes. Each client gets it's own copy, so it > can't get drained. >> So I think based on above 2 points it can be deduced that the signal sent >> by pgkill() cannot be read by anyone else. > Agreed. > Well, what someone else could do is create a pipe with our name before > we do (since we use the actual name - it's \\.\pipe\pgsinal_<pid>), by > guessing what pid we will have. If that happens, we'll go into a loop > and try to recreate it while logging a warning message to > eventlog/stderr. (this happens for every backend). We can't throw an > error on this and kill the backend because the pipe is created in the > background thread not the main one. Once it is detected that already a same Named Pipe already exists, there can be following options: a. try to create with some other name, but in that case how to communicate the new name to client end of pipe. Some solution can be thought if this approach seems to be reasonable, though currently I don't have any in mind. b. give error, as creation of pipe is generally at beginning of process creation(backend), but you already mentioned it is not good approach. c. any other better solution? With Regards, Amit Kapila.
On Tuesday, September 04, 2012 12:11:28 PM Amit Kapila wrote: > On Tuesday, September 04, 2012 11:00 AM Andres Freund wrote: > > On Tuesday, September 04, 2012 06:20:59 AM Tom Lane wrote: > > Andres Freund <andres@2ndquadrant.com> writes: > >> > I can see why that would be nice, but is it really realistic? Don't we > >> > expect some more diligence in applications using this against letting > >> > such a child continue to run after ctrl-c/SIGTERMing e.g. pg_dump in > >> > comparison to closing a normal database connection? > >> > >> Er, what? If you kill the client, the child postgres will see > >> connection closure and will shut down. I already tested that with the > >> POC patch, it worked fine. > > > > Well, but that will make scripting harder because you cannot start > > another single backend pg_dump before the old backend noticed it, > > checkpointed and shut down. > > But isn't that behavior will be similar when currently server is shutting > down due to CTRL-C, and at that time new clients will not be allowed to > connect. As this new interface is an approach similar to embedded database > where first API (StartServer) or at connect time it starts database and > the other connection might not be allowed during shutdown state. I don't find that a convincing comparison. Normally don't need to shutdown the server between two pg_dump commands. Which very well might be scripted. Especially as for now, without a background writer/checkpointer writing stuff beforehand, the shutdown checkpoint won't be fast. IO isn't unlikely if youre doing a pg_dump because of hint bits... Greetings, Andres -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
Andres Freund <andres@2ndquadrant.com> writes: > I don't find that a convincing comparison. Normally don't need to shutdown the > server between two pg_dump commands. Which very well might be scripted. > Especially as for now, without a background writer/checkpointer writing stuff > beforehand, the shutdown checkpoint won't be fast. IO isn't unlikely if youre > doing a pg_dump because of hint bits... I still think this is a straw-man argument. There is no expectation that a standalone PG implementation would provide performance for a series of standalone sessions that is equivalent to what you'd get from a persistent server. If that scenario is what's important to you, you'd use a persistent server. The case where this sort of thing would be interesting is where minimizing administration complexity (by not having a server) is more important than performance. People currently use, eg, SQLite for that type of application, and it's not because of performance. regards, tom lane
Tom Lane <tgl@sss.pgh.pa.us> schrieb: >Andres Freund <andres@2ndquadrant.com> writes: >> I don't find that a convincing comparison. Normally don't need to >shutdown the >> server between two pg_dump commands. Which very well might be >scripted. > >> Especially as for now, without a background writer/checkpointer >writing stuff >> beforehand, the shutdown checkpoint won't be fast. IO isn't unlikely >if youre >> doing a pg_dump because of hint bits... > >I still think this is a straw-man argument. There is no expectation >that a standalone PG implementation would provide performance for a >series of standalone sessions that is equivalent to what you'd get from >a persistent server. If that scenario is what's important to you, >you'd >use a persistent server. The case where this sort of thing would be >interesting is where minimizing administration complexity (by not >having >a server) is more important than performance. People currently use, >eg, >SQLite for that type of application, and it's not because of >performance. I am not saying its bad that it is slower, that's absolutely OK. Just that it will take a variable amount of time till youcan run pgdump again and its not easily detectable without looping and trying again. Andres --- Please excuse the brevity and formatting - I am writing this on my mobile phone.
"anarazel@anarazel.de" <andres@anarazel.de> writes: > I am not saying its bad that it is slower, that's absolutely OK. Just that it will take a variable amount of time tillyou can run pgdump again and its not easily detectable without looping and trying again. Well, that's why the proposed libpq code is written to wait for the child postgres to exit when closing the connection. Admittedly, if you forcibly kill pg_dump (or some other client) and then immediately try to start a new one, it's not clear how long you'll have to wait. But so what? Anything we might do in this space is going to have pluses and minuses. regards, tom lane
Tom, > However, there are some additional things > we'd need to think about before advertising it as a fit solution for that. > Notably, while the lack of any background processes is just what you want > for pg_upgrade and disaster recovery, an ordinary application is probably > going to want to rely on autovacuum; and we need bgwriter and other > background processes for best performance. So I'm speculating about > having a postmaster process that isn't listening on any ports, but is > managing background processes in addition to a single child backend. > That's for another day though. Well, if you think about standalone mode as "developer" mode, it's not quite so clear that we'd need those things. Generally when people are testing code in development they don't care about vacuum or bgwriter because the database is small and ephemeral. So even without background processes, standalone mode would be useful for many users for development and automated testing. For that matter, applications which embed postgresql and have very small databases could also live without autovacuum and bgwriter. Heck, Postgres existed without them for many years. You just doc that, if you're running postgres standalone, you need to run a full VACUUM ANALYZE on the database cluster once per day. And you live with the herky-jerky write performance. If the database is 5GB, who's going to notice anyway? -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com
Josh Berkus <josh@agliodbs.com> writes: >> However, there are some additional things >> we'd need to think about before advertising it as a fit solution for that. >> Notably, while the lack of any background processes is just what you want >> for pg_upgrade and disaster recovery, an ordinary application is probably >> going to want to rely on autovacuum; and we need bgwriter and other >> background processes for best performance. So I'm speculating about >> having a postmaster process that isn't listening on any ports, but is >> managing background processes in addition to a single child backend. >> That's for another day though. > Well, if you think about standalone mode as "developer" mode, it's not > quite so clear that we'd need those things. Generally when people are > testing code in development they don't care about vacuum or bgwriter > because the database is small and ephemeral. So even without background > processes, standalone mode would be useful for many users for > development and automated testing. Only if startup and shutdown were near instantaneous, which as Andres was pointing out would be far from the truth. I am envisioning the use-case for this thing as stuff like desktop managers and mail programs, which tend to be rather lumbering on startup anyway. (And yes, a lot of those have got embedded databases in them these days. More often than not it's mysql.) I don't see people wanting to use this feature for unit tests. > For that matter, applications which embed postgresql and have very small > databases could also live without autovacuum and bgwriter. Heck, > Postgres existed without them for many years. Um ... true with respect to autovacuum, perhaps, but what about checkpoints? A standalone backend will never perform a checkpoint unless explicitly told to. (Before we invented the bgwriter, the postmaster was in charge of launching checkpoints every so often.) Again, this is probably just what you want for disaster recovery, but it wouldn't be terribly friendly for an embedded-database application. In general I think the selling point for such a feature would be "no administrative hassles", and I believe that has to go not only for the end-user experience but also for the application-developer experience. If you have to manage checkpointing and vacuuming in the application, you're probably soon going to look for another database. regards, tom lane
On Wed, Sep 5, 2012 at 01:50:06PM -0700, Josh Berkus wrote: > Tom, > > > However, there are some additional things > > we'd need to think about before advertising it as a fit solution for that. > > Notably, while the lack of any background processes is just what you want > > for pg_upgrade and disaster recovery, an ordinary application is probably > > going to want to rely on autovacuum; and we need bgwriter and other > > background processes for best performance. So I'm speculating about > > having a postmaster process that isn't listening on any ports, but is > > managing background processes in addition to a single child backend. > > That's for another day though. > > Well, if you think about standalone mode as "developer" mode, it's not > quite so clear that we'd need those things. Generally when people are > testing code in development they don't care about vacuum or bgwriter > because the database is small and ephemeral. So even without background > processes, standalone mode would be useful for many users for > development and automated testing. > > For that matter, applications which embed postgresql and have very small > databases could also live without autovacuum and bgwriter. Heck, > Postgres existed without them for many years. > > You just doc that, if you're running postgres standalone, you need to > run a full VACUUM ANALYZE on the database cluster once per day. And you > live with the herky-jerky write performance. If the database is 5GB, > who's going to notice anyway? If this mode slows down pg_upgrade, that is going to be a problem. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +
On 9/5/12 5:03 PM, Tom Lane wrote: > I don't see people wanting to use this feature for unit tests. If this is going to become an official feature (as opposed to an internal interface only for use by pg_upgrade), then I think that's exactly what people will want to use it for. In fact, it might even make it more likely that people will write unit tests suits to begin with.
On Wed, Sep 5, 2012 at 2:50 PM, Peter Eisentraut <peter_e@gmx.net> wrote: > On 9/5/12 5:03 PM, Tom Lane wrote: >> I don't see people wanting to use this feature for unit tests. > > If this is going to become an official feature (as opposed to an > internal interface only for use by pg_upgrade), then I think that's > exactly what people will want to use it for. In fact, it might even > make it more likely that people will write unit tests suits to begin with. I agree with this, even though in theory (but not in practice) creative use of unix sockets (sorry windows, perhaps some port-allocating and URL mangling can be done instead) and conventions for those would allow even better almost-like-embedded results, methinks. That may still be able to happen. The biggest improvement to that situation is the recent drastic reduction in use of shared memory, and that only became a thing recently. -- fdr
On 9/5/12 5:59 PM, Daniel Farina wrote: > I agree with this, even though in theory (but not in practice) > creative use of unix sockets (sorry windows, perhaps some > port-allocating and URL mangling can be done instead) and conventions > for those would allow even better almost-like-embedded results, > methinks. That may still be able to happen. Sure, everyone who cares can already do this, but some people probably don't care enough. Also, making this portable and robust for everyone to use, not just your local environment, is pretty tricky. See pg_upgrade test script, for a prominent example.
On 9/5/12 2:50 PM, Peter Eisentraut wrote: > On 9/5/12 5:03 PM, Tom Lane wrote: >> I don't see people wanting to use this feature for unit tests. > > If this is going to become an official feature (as opposed to an > internal interface only for use by pg_upgrade), then I think that's > exactly what people will want to use it for. In fact, it might even > make it more likely that people will write unit tests suits to begin with. Heck, *I'll* use it for unit tests. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com
> Um ... true with respect to autovacuum, perhaps, but what about > checkpoints? A standalone backend will never perform a checkpoint > unless explicitly told to. Hmmm, that's definitely an issue. > (Before we invented the bgwriter, the > postmaster was in charge of launching checkpoints every so often.) > Again, this is probably just what you want for disaster recovery, but > it wouldn't be terribly friendly for an embedded-database application. Yeah, we'd have to put in a clock-based thing which did checkpoints every 5 minutes and VACUUM ANALYZE every hour or something. That seems like a chunk of extra code. > In general I think the selling point for such a feature would be "no > administrative hassles", and I believe that has to go not only for the > end-user experience but also for the application-developer experience. > If you have to manage checkpointing and vacuuming in the application, > you're probably soon going to look for another database. Well, don't discount the development/testing case. If you do agile or TDD (a lot of people do), you often have a workload which looks like: 1) Start framework 2) Start database 3) Load database with test data 4) Run tests 5) Print results 6) Shut down database In a case like that, you can live without checkpointing, even; the database is ephemeral. In other words, let's make this a feature and document it for use in testing, and that it's not really usable for production embedded apps yet. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com
On Wed, Sep 5, 2012 at 3:17 PM, Peter Eisentraut <peter_e@gmx.net> wrote: > On 9/5/12 5:59 PM, Daniel Farina wrote: >> I agree with this, even though in theory (but not in practice) >> creative use of unix sockets (sorry windows, perhaps some >> port-allocating and URL mangling can be done instead) and conventions >> for those would allow even better almost-like-embedded results, >> methinks. That may still be able to happen. > > Sure, everyone who cares can already do this, but some people probably > don't care enough. Also, making this portable and robust for everyone > to use, not just your local environment, is pretty tricky. See > pg_upgrade test script, for a prominent example. To my knowledge, no one has even really seriously tried to package it yet and then told the tale of woe, and it was an especially un-gratifying exercise for quite a while on account of multiple postgreses not getting along on the same machine because of SysV shmem. The bar for testing is a lot different than pg_upgrade (where a negative consequence is confusing and stressful downtime), and many programs use fork/threads and multiple connections even in testing, making its requirements different. So consider me still skeptical given the current reasoning that unix sockets can't be a good-or-better substitute, and especially accounting for programs that need multiple backends. -- fdr
So, in the spirit of not painting ourselves into a tiny corner here on the whole "single backend" and "embedded database" problem with pg options, can we generalize this a bit? Any way we could make psql connect to a "given fd", as an option? In theory, that could be something opened by some out-side-of-postgresql tunnel with 3rd party auth in the same app that uses libpq directly, or it could be a fd prepared by something that specifically launched a single-backend postgres, like in the case of pg_upgrade, pg_uprade itself, and passed to psql, etc, which would be passed in as options. In theory, that might even allow the possibility of starting the single-backend only once and passing it to multiple clients in succession, instead of having to stop/start the backend between each client. And it would allow the possiblity of "something" (pg_upgrade, or some other application) to control the start/stop of the backend outside the libpq connection. Now, I'm familiar with the abilities related to passing fd's around in Linux, but have no idea if we'd have comparable methods to use on Windows. a. On Wed, Sep 5, 2012 at 8:11 PM, Daniel Farina <daniel@heroku.com> wrote: > On Wed, Sep 5, 2012 at 3:17 PM, Peter Eisentraut <peter_e@gmx.net> wrote: >> On 9/5/12 5:59 PM, Daniel Farina wrote: >>> I agree with this, even though in theory (but not in practice) >>> creative use of unix sockets (sorry windows, perhaps some >>> port-allocating and URL mangling can be done instead) and conventions >>> for those would allow even better almost-like-embedded results, >>> methinks. That may still be able to happen. >> >> Sure, everyone who cares can already do this, but some people probably >> don't care enough. Also, making this portable and robust for everyone >> to use, not just your local environment, is pretty tricky. See >> pg_upgrade test script, for a prominent example. > > To my knowledge, no one has even really seriously tried to package it > yet and then told the tale of woe, and it was an especially > un-gratifying exercise for quite a while on account of multiple > postgreses not getting along on the same machine because of SysV > shmem. > > The bar for testing is a lot different than pg_upgrade (where a > negative consequence is confusing and stressful downtime), and many > programs use fork/threads and multiple connections even in testing, > making its requirements different. > > So consider me still skeptical given the current reasoning that unix > sockets can't be a good-or-better substitute, and especially > accounting for programs that need multiple backends. > > -- > fdr > > > -- > Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-hackers > -- Aidan Van Dyk Create like a god, aidan@highrise.ca command like a king, http://www.highrise.ca/ work like a slave.
Aidan Van Dyk <aidan@highrise.ca> writes: > So, in the spirit of not painting ourselves into a tiny corner here on > the whole "single backend" and "embedded database" problem with pg > options, can we generalize this a bit? > Any way we could make psql connect to a "given fd", as an option? In > theory, that could be something opened by some out-side-of-postgresql > tunnel with 3rd party auth in the same app that uses libpq directly, > or it could be a fd prepared by something that specifically launched > a single-backend postgres, like in the case of pg_upgrade, pg_uprade > itself, and passed to psql, etc, which would be passed in as options. This seems to me to be going in exactly the wrong direction. What I visualize this feature as responding to is demand for a *simple*, minimal configuration, minimal administration, quasi-embedded database. What you propose above is not that, but is if anything even more complicated for an application to deal with than a regular persistent server. More complication is *the wrong thing* for this use case. The people who would be interested in this are currently using something like SQLite within a single application program. It hasn't got any of the features you're suggesting either, and I don't think anybody wishes it did. regards, tom lane
On 09/05/2012 10:14 PM, Tom Lane wrote: > Aidan Van Dyk <aidan@highrise.ca> writes: >> So, in the spirit of not painting ourselves into a tiny corner here on >> the whole "single backend" and "embedded database" problem with pg >> options, can we generalize this a bit? >> Any way we could make psql connect to a "given fd", as an option? In >> theory, that could be something opened by some out-side-of-postgresql >> tunnel with 3rd party auth in the same app that uses libpq directly, >> or it could be a fd prepared by something that specifically launched >> a single-backend postgres, like in the case of pg_upgrade, pg_uprade >> itself, and passed to psql, etc, which would be passed in as options. > This seems to me to be going in exactly the wrong direction. What > I visualize this feature as responding to is demand for a *simple*, > minimal configuration, minimal administration, quasi-embedded database. > What you propose above is not that, but is if anything even more > complicated for an application to deal with than a regular persistent > server. More complication is *the wrong thing* for this use case. > > The people who would be interested in this are currently using something > like SQLite within a single application program. It hasn't got any of > the features you're suggesting either, and I don't think anybody wishes > it did. > > Exactly. I think it's worth stating that this has a HUGE potential audience, and if we can get to this the deployment of Postgres could mushroom enormously. I'm really quite excited about it. cheers andrew
On Wed, Sep 5, 2012 at 7:14 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > This seems to me to be going in exactly the wrong direction. What > I visualize this feature as responding to is demand for a *simple*, > minimal configuration, minimal administration, quasi-embedded database. > What you propose above is not that, but is if anything even more > complicated for an application to deal with than a regular persistent > server. More complication is *the wrong thing* for this use case. > > The people who would be interested in this are currently using something > like SQLite within a single application program. It hasn't got any of > the features you're suggesting either, and I don't think anybody wishes > it did. I am failing to understand how one could easily replicate the SQLite feature of (even fairly poorly) using multiple processes addressing one database, and supporting multiple executors per-database (which correspond to every open 'connection' in SQLite, as far as I can understand). My best thoughts are in the direction of EXEC_BACKEND and hooking up to a cluster post-facto, but I wasn't really looking for solutions so much as to raise this (to me) important use-case. I'm just thinking about all the enormously popular prefork based web servers out there like unicorn (Ruby), gunicorn (Python), and even without forking language-specific database abstractions like that seen in Go ("database/sql") that have decided to make pooling the default interaction. It is easiest to use these prefork embedded servers in both in development and production, so people (rather sensibly) do that -- better parity, and no fuss. I really would rather not see that regress by appropriating special mechanics for test/development scenarios with regards to managing database connections (e.g. exactly one of them), so how do we not make that a restriction, unless I misunderstood and was a non-restriction already? -- fdr
On Wed, 2012-09-05 at 17:03 -0400, Tom Lane wrote: > In general I think the selling point for such a feature would be "no > administrative hassles", and I believe that has to go not only for the > end-user experience but also for the application-developer experience. > If you have to manage checkpointing and vacuuming in the application, > you're probably soon going to look for another database. Maybe there could be some hooks (e.g., right after completing a statement) that see whether a vacuum or checkpoint is required? VACUUM can't be run in a transaction block[1], so there are some details to work out, but it might be a workable approach. Regards,Jeff Davis [1]: It seems like the only reason for that is so a multi-table vacuum doesn't hold locks for longer than it needs to, but that's not much of a concern in this case.
Heikki Linnakangas <hlinnaka@iki.fi> writes: > Would socketpair(2) be simpler? Attached is a revised version of the patch that uses socketpair(2). This is definitely a lot less invasive --- the backend side of the patch, in particular, is far shorter, and there are fewer portability hazards since we're not trying to replace sockets with pipes. I've not done anything yet about the potential security issues associated with untrusted libpq connection strings. I think this is still at the proof-of-concept stage; in particular, it's probably time to see if we can make it work on Windows before we worry more about that. I'm a bit tempted though to pull out and apply the portions of the patch that replace libpq's assorted ad-hoc closesocket() calls with a centralized pqDropConnection routine. I think that's probably a good idea independently of this feature. regards, tom lane diff --git a/src/backend/main/main.c b/src/backend/main/main.c index 33c5a0a4e645624515016397da0011423d993c70..968959b85ef53aa2ec0ef8c5c5a9bc544291bfe5 100644 *** a/src/backend/main/main.c --- b/src/backend/main/main.c *************** main(int argc, char *argv[]) *** 191,196 **** --- 191,198 ---- AuxiliaryProcessMain(argc, argv); /* does not return */ else if (argc > 1 && strcmp(argv[1], "--describe-config") == 0) GucInfoMain(); /* does not return */ + else if (argc > 1 && strncmp(argv[1], "--child=", 8) == 0) + ChildPostgresMain(argc, argv, get_current_username(progname)); /* does not return */ else if (argc > 1 && strcmp(argv[1], "--single") == 0) PostgresMain(argc, argv, get_current_username(progname)); /* does not return */ else diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c index 73520a6ca2f4d3300f3d8939c0e9412064a911c3..5a51fa9cb9abb39b0ac02d21a25e5940159582ea 100644 *** a/src/backend/postmaster/postmaster.c --- b/src/backend/postmaster/postmaster.c *************** ExitPostmaster(int status) *** 4268,4273 **** --- 4268,4350 ---- proc_exit(status); } + + /* + * ChildPostgresMain - start a new-style standalone postgres process + * + * This may not belong here, but it does share a lot of code with ConnCreate + * and BackendInitialize. Basically what it has to do is set up a + * MyProcPort structure and then hand off control to PostgresMain. + * Beware that not very much stuff is initialized yet. + * + * In the future it might be interesting to support a "standalone + * multiprocess" mode in which we have a postmaster process that doesn't + * listen for connections, but does supervise autovacuum, bgwriter, etc + * auxiliary processes. So that's another reason why postmaster.c might be + * the right place for this. + */ + void + ChildPostgresMain(int argc, char *argv[], const char *username) + { + Port *port; + + /* + * Fire up essential subsystems: error and memory management + */ + MemoryContextInit(); + + /* + * Build a Port structure for the client connection + */ + if (!(port = (Port *) calloc(1, sizeof(Port)))) + ereport(FATAL, + (errcode(ERRCODE_OUT_OF_MEMORY), + errmsg("out of memory"))); + + /* + * GSSAPI specific state struct must exist even though we won't use it + */ + #if defined(ENABLE_GSS) || defined(ENABLE_SSPI) + port->gss = (pg_gssinfo *) calloc(1, sizeof(pg_gssinfo)); + if (!port->gss) + ereport(FATAL, + (errcode(ERRCODE_OUT_OF_MEMORY), + errmsg("out of memory"))); + #endif + + /* The file descriptor of the client socket is the argument of --child */ + if (sscanf(argv[1], "--child=%d", &port->sock) != 1) + ereport(FATAL, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("invalid argument for --child: \"%s\"", argv[1]))); + + /* Default assumption about protocol to use */ + FrontendProtocol = port->proto = PG_PROTOCOL_LATEST; + + /* save process start time */ + port->SessionStartTime = GetCurrentTimestamp(); + MyStartTime = timestamptz_to_time_t(port->SessionStartTime); + + /* set these to empty in case they are needed */ + port->remote_host = ""; + port->remote_port = ""; + + MyProcPort = port; + + /* + * We can now initialize libpq and then enable reporting of ereport errors + * to the client. + */ + pq_init(); /* initialize libpq to talk to client */ + whereToSendOutput = DestRemote; /* now safe to ereport to client */ + + /* And pass off control to PostgresMain */ + PostgresMain(argc, argv, username); + + abort(); /* not reached */ + } + + /* * sigusr1_handler - handle signal conditions from child processes */ diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c index f1248a851bf90188da8d3a7e8b61ac99bf78ebbd..c6a7de63c77f79c85be2170d4e5254b790f3fd5e 100644 *** a/src/backend/tcop/postgres.c --- b/src/backend/tcop/postgres.c *************** process_postgres_switches(int argc, char *** 3257,3264 **** { gucsource = PGC_S_ARGV; /* switches came from command line */ ! /* Ignore the initial --single argument, if present */ ! if (argc > 1 && strcmp(argv[1], "--single") == 0) { argv++; argc--; --- 3257,3266 ---- { gucsource = PGC_S_ARGV; /* switches came from command line */ ! /* Ignore the initial --single or --child argument, if present */ ! if (argc > 1 && ! (strcmp(argv[1], "--single") == 0 || ! strncmp(argv[1], "--child=", 8) == 0)) { argv++; argc--; *************** PostgresMain(int argc, char *argv[], con *** 3522,3539 **** * standalone). */ if (!IsUnderPostmaster) - { MyProcPid = getpid(); MyStartTime = time(NULL); - } /* * Fire up essential subsystems: error and memory management * ! * If we are running under the postmaster, this is done already. */ ! if (!IsUnderPostmaster) MemoryContextInit(); SetProcessingMode(InitProcessing); --- 3524,3541 ---- * standalone). */ if (!IsUnderPostmaster) MyProcPid = getpid(); + if (MyProcPort == NULL) MyStartTime = time(NULL); /* * Fire up essential subsystems: error and memory management * ! * If we are running under the postmaster, this is done already; likewise ! * in child-postgres mode. */ ! if (MyProcPort == NULL) MemoryContextInit(); SetProcessingMode(InitProcessing); diff --git a/src/include/postmaster/postmaster.h b/src/include/postmaster/postmaster.h index 0fe7ec26db7646472fca248dee6982dc5c595c6c..6d4245cdc05ea5fea4baccf5dbc52dade7c00ff0 100644 *** a/src/include/postmaster/postmaster.h --- b/src/include/postmaster/postmaster.h *************** extern int postmaster_alive_fds[2]; *** 47,52 **** --- 47,55 ---- extern const char *progname; extern void PostmasterMain(int argc, char *argv[]) __attribute__((noreturn)); + + extern void ChildPostgresMain(int argc, char *argv[], const char *username) __attribute__((noreturn)); + extern void ClosePostmasterPorts(bool am_syslogger); extern int MaxLivePostmasterChildren(void); diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c index 3dcd0c3f9c09ec06144c040cc333e382b9d7f78b..c578a8ebbe6874825afbd0cb27cdfa2a3d01aec7 100644 *** a/src/interfaces/libpq/fe-connect.c --- b/src/interfaces/libpq/fe-connect.c *************** *** 17,22 **** --- 17,23 ---- #include <sys/types.h> #include <sys/stat.h> + #include <sys/wait.h> #include <fcntl.h> #include <ctype.h> #include <time.h> *************** static const PQconninfoOption PQconninfo *** 259,264 **** --- 260,271 ---- {"replication", NULL, NULL, NULL, "Replication", "D", 5}, + {"standalone_datadir", NULL, NULL, NULL, + "Standalone-Data-Directory", "D", 64}, + + {"standalone_backend", NULL, NULL, NULL, + "Standalone-Backend", "D", 64}, + /* Terminating entry --- MUST BE LAST */ {NULL, NULL, NULL, NULL, NULL, NULL, 0} *************** pgthreadlock_t pg_g_threadlock = default *** 345,350 **** --- 352,390 ---- /* + * pqDropConnection + * + * Close any physical connection to the server, and reset associated + * state inside the connection object. We don't release state that + * would be needed to reconnect, though. + */ + void + pqDropConnection(PGconn *conn) + { + /* Drop any SSL state */ + pqsecure_close(conn); + /* Close the socket itself */ + if (conn->sock >= 0) + closesocket(conn->sock); + conn->sock = -1; + /* If we forked a child postgres process, wait for it to exit */ + if (conn->postgres_pid > 0) + { + while (waitpid(conn->postgres_pid, NULL, 0) < 0) + { + /* If interrupted by signal, keep waiting */ + if (errno != EINTR) + break; + } + } + conn->postgres_pid = -1; + /* Discard any unread/unsent data */ + conn->inStart = conn->inCursor = conn->inEnd = 0; + conn->outCount = 0; + } + + + /* * Connecting to a Database * * There are now six different ways a user of this API can connect to the *************** fillPGconn(PGconn *conn, PQconninfoOptio *** 621,626 **** --- 661,670 ---- conn->pghost = tmp ? strdup(tmp) : NULL; tmp = conninfo_getval(connOptions, "port"); conn->pgport = tmp ? strdup(tmp) : NULL; + tmp = conninfo_getval(connOptions, "standalone_datadir"); + conn->standalone_datadir = tmp ? strdup(tmp) : NULL; + tmp = conninfo_getval(connOptions, "standalone_backend"); + conn->standalone_backend = tmp ? strdup(tmp) : NULL; tmp = conninfo_getval(connOptions, "tty"); conn->pgtty = tmp ? strdup(tmp) : NULL; tmp = conninfo_getval(connOptions, "options"); *************** setKeepalivesWin32(PGconn *conn) *** 1289,1294 **** --- 1333,1384 ---- #endif /* SIO_KEEPALIVE_VALS */ #endif /* WIN32 */ + /* + * Fork and exec a command, connecting up a pair of anonymous sockets for + * communication. argv[fdarg] is modified by appending the child-side + * socket's file descriptor number. The parent-side socket's FD is returned + * in *psock, and the function result is the child's PID. + */ + static pid_t + fork_backend_child(char **argv, int fdarg, int *psock) + { + pid_t pid; + int socks[2]; + char newfdarg[32]; + + if (socketpair(AF_UNIX, SOCK_STREAM, 0, socks) < 0) + return -1; + + pid = fork(); + + if (pid < 0) + { + /* fork failed */ + close(socks[0]); + close(socks[1]); + return pid; + } + else if (pid == 0) + { + /* successful, in child process */ + close(socks[1]); + snprintf(newfdarg, sizeof(newfdarg), "%s%d", argv[fdarg], socks[0]); + argv[fdarg] = newfdarg; + execv(argv[0], argv); + perror(argv[0]); + exit(1); + } + else + { + /* successful, in parent process */ + close(socks[0]); + *psock = socks[1]; + } + + return pid; + } + + /* ---------- * connectDBStart - * Begin the process of making a connection to the backend. *************** connectDBStart(PGconn *conn) *** 1317,1322 **** --- 1407,1461 ---- conn->outCount = 0; /* + * If the standalone_datadir option was specified, ignore any host or + * port specifications and just try to fork a standalone backend. + */ + if (conn->standalone_datadir && conn->standalone_datadir[0]) + { + char *be_argv[10]; + + /* + * We set up the backend's command line in execv(3) style, so that + * we don't need to cope with shell quoting rules. + */ + if (conn->standalone_backend && conn->standalone_backend[0]) + be_argv[0] = conn->standalone_backend; + else /* assume we should use hard-wired path */ + be_argv[0] = PGBINDIR "/postgres"; + + be_argv[1] = "--child="; + be_argv[2] = "-D"; + be_argv[3] = conn->standalone_datadir; + be_argv[4] = (conn->dbName && conn->dbName[0]) ? conn->dbName : NULL; + be_argv[5] = NULL; + + conn->postgres_pid = fork_backend_child(be_argv, 1, &conn->sock); + if (conn->postgres_pid < 0) + { + char sebuf[256]; + + appendPQExpBuffer(&conn->errorMessage, + libpq_gettext("could not fork standalone backend: %s\n"), + pqStrerror(errno, sebuf, sizeof(sebuf))); + goto connect_errReturn; + } + + /* + * Go directly to CONNECTION_AUTH_OK state, since the standalone + * backend is not going to issue any authentication challenge to us. + * We're just waiting for startup to conclude. + */ + #ifdef USE_SSL + conn->allow_ssl_try = false; + #endif + conn->pversion = PG_PROTOCOL(3, 0); + conn->status = CONNECTION_AUTH_OK; + conn->asyncStatus = PGASYNC_BUSY; + + return 1; + } + + /* * Determine the parameters to pass to pg_getaddrinfo_all. */ *************** connectDBStart(PGconn *conn) *** 1416,1427 **** return 1; connect_errReturn: ! if (conn->sock >= 0) ! { ! pqsecure_close(conn); ! closesocket(conn->sock); ! conn->sock = -1; ! } conn->status = CONNECTION_BAD; return 0; } --- 1555,1561 ---- return 1; connect_errReturn: ! pqDropConnection(conn); conn->status = CONNECTION_BAD; return 0; } *************** keep_going: /* We will come back to *** 1644,1651 **** { if (!connectNoDelay(conn)) { ! closesocket(conn->sock); ! conn->sock = -1; conn->addr_cur = addr_cur->ai_next; continue; } --- 1778,1784 ---- { if (!connectNoDelay(conn)) { ! pqDropConnection(conn); conn->addr_cur = addr_cur->ai_next; continue; } *************** keep_going: /* We will come back to *** 1655,1662 **** appendPQExpBuffer(&conn->errorMessage, libpq_gettext("could not set socket to non-blocking mode: %s\n"), SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf))); ! closesocket(conn->sock); ! conn->sock = -1; conn->addr_cur = addr_cur->ai_next; continue; } --- 1788,1794 ---- appendPQExpBuffer(&conn->errorMessage, libpq_gettext("could not set socket to non-blocking mode: %s\n"), SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf))); ! pqDropConnection(conn); conn->addr_cur = addr_cur->ai_next; continue; } *************** keep_going: /* We will come back to *** 1667,1674 **** appendPQExpBuffer(&conn->errorMessage, libpq_gettext("could not set socket to close-on-exec mode: %s\n"), SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf))); ! closesocket(conn->sock); ! conn->sock = -1; conn->addr_cur = addr_cur->ai_next; continue; } --- 1799,1805 ---- appendPQExpBuffer(&conn->errorMessage, libpq_gettext("could not set socket to close-on-exec mode: %s\n"), SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf))); ! pqDropConnection(conn); conn->addr_cur = addr_cur->ai_next; continue; } *************** keep_going: /* We will come back to *** 1715,1722 **** if (err) { ! closesocket(conn->sock); ! conn->sock = -1; conn->addr_cur = addr_cur->ai_next; continue; } --- 1846,1852 ---- if (err) { ! pqDropConnection(conn); conn->addr_cur = addr_cur->ai_next; continue; } *************** keep_going: /* We will come back to *** 1802,1812 **** * failure and keep going if there are more addresses. */ connectFailureMessage(conn, SOCK_ERRNO); ! if (conn->sock >= 0) ! { ! closesocket(conn->sock); ! conn->sock = -1; ! } /* * Try the next address, if any. --- 1932,1938 ---- * failure and keep going if there are more addresses. */ connectFailureMessage(conn, SOCK_ERRNO); ! pqDropConnection(conn); /* * Try the next address, if any. *************** keep_going: /* We will come back to *** 1851,1856 **** --- 1977,1983 ---- * error message. */ connectFailureMessage(conn, optval); + pqDropConnection(conn); /* * If more addresses remain, keep trying, just as in the *************** keep_going: /* We will come back to *** 1858,1868 **** */ if (conn->addr_cur->ai_next != NULL) { - if (conn->sock >= 0) - { - closesocket(conn->sock); - conn->sock = -1; - } conn->addr_cur = conn->addr_cur->ai_next; conn->status = CONNECTION_NEEDED; goto keep_going; --- 1985,1990 ---- *************** keep_going: /* We will come back to *** 2137,2148 **** /* only retry once */ conn->allow_ssl_try = false; /* Must drop the old connection */ ! closesocket(conn->sock); ! conn->sock = -1; conn->status = CONNECTION_NEEDED; - /* Discard any unread/unsent data */ - conn->inStart = conn->inCursor = conn->inEnd = 0; - conn->outCount = 0; goto keep_going; } } --- 2259,2266 ---- /* only retry once */ conn->allow_ssl_try = false; /* Must drop the old connection */ ! pqDropConnection(conn); conn->status = CONNECTION_NEEDED; goto keep_going; } } *************** keep_going: /* We will come back to *** 2252,2264 **** { conn->pversion = PG_PROTOCOL(2, 0); /* Must drop the old connection */ ! pqsecure_close(conn); ! closesocket(conn->sock); ! conn->sock = -1; conn->status = CONNECTION_NEEDED; - /* Discard any unread/unsent data */ - conn->inStart = conn->inCursor = conn->inEnd = 0; - conn->outCount = 0; goto keep_going; } --- 2370,2377 ---- { conn->pversion = PG_PROTOCOL(2, 0); /* Must drop the old connection */ ! pqDropConnection(conn); conn->status = CONNECTION_NEEDED; goto keep_going; } *************** keep_going: /* We will come back to *** 2323,2334 **** /* only retry once */ conn->wait_ssl_try = false; /* Must drop the old connection */ ! closesocket(conn->sock); ! conn->sock = -1; conn->status = CONNECTION_NEEDED; - /* Discard any unread/unsent data */ - conn->inStart = conn->inCursor = conn->inEnd = 0; - conn->outCount = 0; goto keep_going; } --- 2436,2443 ---- /* only retry once */ conn->wait_ssl_try = false; /* Must drop the old connection */ ! pqDropConnection(conn); conn->status = CONNECTION_NEEDED; goto keep_going; } *************** keep_going: /* We will come back to *** 2343,2355 **** /* only retry once */ conn->allow_ssl_try = false; /* Must drop the old connection */ ! pqsecure_close(conn); ! closesocket(conn->sock); ! conn->sock = -1; conn->status = CONNECTION_NEEDED; - /* Discard any unread/unsent data */ - conn->inStart = conn->inCursor = conn->inEnd = 0; - conn->outCount = 0; goto keep_going; } #endif --- 2452,2459 ---- /* only retry once */ conn->allow_ssl_try = false; /* Must drop the old connection */ ! pqDropConnection(conn); conn->status = CONNECTION_NEEDED; goto keep_going; } #endif *************** keep_going: /* We will come back to *** 2509,2521 **** PQclear(res); conn->send_appname = false; /* Must drop the old connection */ ! pqsecure_close(conn); ! closesocket(conn->sock); ! conn->sock = -1; conn->status = CONNECTION_NEEDED; - /* Discard any unread/unsent data */ - conn->inStart = conn->inCursor = conn->inEnd = 0; - conn->outCount = 0; goto keep_going; } } --- 2613,2620 ---- PQclear(res); conn->send_appname = false; /* Must drop the old connection */ ! pqDropConnection(conn); conn->status = CONNECTION_NEEDED; goto keep_going; } } *************** makeEmptyPGconn(void) *** 2726,2731 **** --- 2825,2831 ---- conn->auth_req_received = false; conn->password_needed = false; conn->dot_pgpass_used = false; + conn->postgres_pid = -1; #ifdef USE_SSL conn->allow_ssl_try = true; conn->wait_ssl_try = false; *************** freePGconn(PGconn *conn) *** 2802,2807 **** --- 2902,2911 ---- free(conn->pgport); if (conn->pgunixsocket) free(conn->pgunixsocket); + if (conn->standalone_datadir) + free(conn->standalone_datadir); + if (conn->standalone_backend) + free(conn->standalone_backend); if (conn->pgtty) free(conn->pgtty); if (conn->connect_timeout) *************** closePGconn(PGconn *conn) *** 2909,2920 **** /* * Close the connection, reset all transient state, flush I/O buffers. */ ! if (conn->sock >= 0) ! { ! pqsecure_close(conn); ! closesocket(conn->sock); ! } ! conn->sock = -1; conn->status = CONNECTION_BAD; /* Well, not really _bad_ - just * absent */ conn->asyncStatus = PGASYNC_IDLE; --- 3013,3019 ---- /* * Close the connection, reset all transient state, flush I/O buffers. */ ! pqDropConnection(conn); conn->status = CONNECTION_BAD; /* Well, not really _bad_ - just * absent */ conn->asyncStatus = PGASYNC_IDLE; *************** closePGconn(PGconn *conn) *** 2943,2950 **** if (conn->lobjfuncs) free(conn->lobjfuncs); conn->lobjfuncs = NULL; - conn->inStart = conn->inCursor = conn->inEnd = 0; - conn->outCount = 0; #ifdef ENABLE_GSS { OM_uint32 min_s; --- 3042,3047 ---- diff --git a/src/interfaces/libpq/fe-misc.c b/src/interfaces/libpq/fe-misc.c index b5e5519c416aa767921a5549f7947f80e4cf8250..0cd0d4288f1c3a859eebc5708b6a44d7e1c3da52 100644 *** a/src/interfaces/libpq/fe-misc.c --- b/src/interfaces/libpq/fe-misc.c *************** retry4: *** 780,790 **** * has been set already. */ definitelyFailed: conn->status = CONNECTION_BAD; /* No more connection to backend */ - pqsecure_close(conn); - closesocket(conn->sock); - conn->sock = -1; - return -1; } --- 780,787 ---- * has been set already. */ definitelyFailed: + pqDropConnection(conn); conn->status = CONNECTION_BAD; /* No more connection to backend */ return -1; } diff --git a/src/interfaces/libpq/fe-protocol3.c b/src/interfaces/libpq/fe-protocol3.c index d289f82285fea00d5de20542e43ea103493f9e58..c605bcd734c9fd76d15c893db6d66f727bb6d876 100644 *** a/src/interfaces/libpq/fe-protocol3.c --- b/src/interfaces/libpq/fe-protocol3.c *************** handleSyncLoss(PGconn *conn, char id, in *** 430,438 **** pqSaveErrorResult(conn); conn->asyncStatus = PGASYNC_READY; /* drop out of GetResult wait loop */ ! pqsecure_close(conn); ! closesocket(conn->sock); ! conn->sock = -1; conn->status = CONNECTION_BAD; /* No more connection to backend */ } --- 430,436 ---- pqSaveErrorResult(conn); conn->asyncStatus = PGASYNC_READY; /* drop out of GetResult wait loop */ ! pqDropConnection(conn); conn->status = CONNECTION_BAD; /* No more connection to backend */ } diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h index 2bac59c3d879ecabce42ceab3b5133df03a0886a..a9070ace73cead020b8b42f2720eebc771f39e6e 100644 *** a/src/interfaces/libpq/libpq-int.h --- b/src/interfaces/libpq/libpq-int.h *************** struct pg_conn *** 303,308 **** --- 303,310 ---- char *pgunixsocket; /* the Unix-domain socket that the server is * listening on; if NULL, uses a default * constructed from pgport */ + char *standalone_datadir; /* data directory for standalone backend */ + char *standalone_backend; /* executable for standalone backend */ char *pgtty; /* tty on which the backend messages is * displayed (OBSOLETE, NOT USED) */ char *connect_timeout; /* connection timeout (numeric string) */ *************** struct pg_conn *** 373,378 **** --- 375,383 ---- bool sigpipe_so; /* have we masked SIGPIPE via SO_NOSIGPIPE? */ bool sigpipe_flag; /* can we mask SIGPIPE via MSG_NOSIGNAL? */ + /* If we forked a child postgres process, its PID is kept here */ + pid_t postgres_pid; /* PID, or -1 if none */ + /* Transient state needed while establishing connection */ struct addrinfo *addrlist; /* list of possible backend addresses */ struct addrinfo *addr_cur; /* the one currently being tried */ *************** extern char *const pgresStatus[]; *** 488,493 **** --- 493,499 ---- /* === in fe-connect.c === */ + extern void pqDropConnection(PGconn *conn); extern int pqPacketSend(PGconn *conn, char pack_type, const void *buf, size_t buf_len); extern bool pqGetHomeDirectory(char *buf, int bufsize);
On 07.09.2012 10:49, Tom Lane wrote: > Heikki Linnakangas<hlinnaka@iki.fi> writes: >> Would socketpair(2) be simpler? > > Attached is a revised version of the patch that uses socketpair(2). > This is definitely a lot less invasive --- the backend side of the > patch, in particular, is far shorter, and there are fewer portability > hazards since we're not trying to replace sockets with pipes. > > I've not done anything yet about the potential security issues > associated with untrusted libpq connection strings. I think this > is still at the proof-of-concept stage; in particular, it's probably > time to see if we can make it work on Windows before we worry more > about that. > > I'm a bit tempted though to pull out and apply the portions of the > patch that replace libpq's assorted ad-hoc closesocket() calls with > a centralized pqDropConnection routine. I think that's probably a good > idea independently of this feature. Sounds good. It's worth noting that now that libpq constructs the command line to execute "postgres --child= -D <datadir>", we'll be stuck with that set of arguments forever, because libpq needs to be able to talk to different versions. Or at least we'd need to teach libpq to check the version of binary and act accordingly, if we change that syntax. That's probably OK, I don't feel any pressure to change those command line arguments anyway. - Heikki
Heikki Linnakangas <hlinnaka@iki.fi> writes: > It's worth noting that now that libpq constructs the command line to > execute "postgres --child= -D <datadir>", we'll be stuck with that set > of arguments forever, because libpq needs to be able to talk to > different versions. Or at least we'd need to teach libpq to check the > version of binary and act accordingly, if we change that syntax. That's > probably OK, I don't feel any pressure to change those command line > arguments anyway. Yeah. The -D syntax seems safe enough from here. One thing that's on my to-do list for this patch is to add a -v switch to set the protocol version, so that we don't need to assume that libpq and backend have the same default protocol version. regards, tom lane
Heikki Linnakangas <hlinnaka@iki.fi> writes: > On 07.09.2012 10:49, Tom Lane wrote: >> I'm a bit tempted though to pull out and apply the portions of the >> patch that replace libpq's assorted ad-hoc closesocket() calls with >> a centralized pqDropConnection routine. I think that's probably a good >> idea independently of this feature. > Sounds good. Done; here's the rebased patch. regards, tom lane diff --git a/src/backend/main/main.c b/src/backend/main/main.c index 33c5a0a4e645624515016397da0011423d993c70..968959b85ef53aa2ec0ef8c5c5a9bc544291bfe5 100644 *** a/src/backend/main/main.c --- b/src/backend/main/main.c *************** main(int argc, char *argv[]) *** 191,196 **** --- 191,198 ---- AuxiliaryProcessMain(argc, argv); /* does not return */ else if (argc > 1 && strcmp(argv[1], "--describe-config") == 0) GucInfoMain(); /* does not return */ + else if (argc > 1 && strncmp(argv[1], "--child=", 8) == 0) + ChildPostgresMain(argc, argv, get_current_username(progname)); /* does not return */ else if (argc > 1 && strcmp(argv[1], "--single") == 0) PostgresMain(argc, argv, get_current_username(progname)); /* does not return */ else diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c index 73520a6ca2f4d3300f3d8939c0e9412064a911c3..5a51fa9cb9abb39b0ac02d21a25e5940159582ea 100644 *** a/src/backend/postmaster/postmaster.c --- b/src/backend/postmaster/postmaster.c *************** ExitPostmaster(int status) *** 4268,4273 **** --- 4268,4350 ---- proc_exit(status); } + + /* + * ChildPostgresMain - start a new-style standalone postgres process + * + * This may not belong here, but it does share a lot of code with ConnCreate + * and BackendInitialize. Basically what it has to do is set up a + * MyProcPort structure and then hand off control to PostgresMain. + * Beware that not very much stuff is initialized yet. + * + * In the future it might be interesting to support a "standalone + * multiprocess" mode in which we have a postmaster process that doesn't + * listen for connections, but does supervise autovacuum, bgwriter, etc + * auxiliary processes. So that's another reason why postmaster.c might be + * the right place for this. + */ + void + ChildPostgresMain(int argc, char *argv[], const char *username) + { + Port *port; + + /* + * Fire up essential subsystems: error and memory management + */ + MemoryContextInit(); + + /* + * Build a Port structure for the client connection + */ + if (!(port = (Port *) calloc(1, sizeof(Port)))) + ereport(FATAL, + (errcode(ERRCODE_OUT_OF_MEMORY), + errmsg("out of memory"))); + + /* + * GSSAPI specific state struct must exist even though we won't use it + */ + #if defined(ENABLE_GSS) || defined(ENABLE_SSPI) + port->gss = (pg_gssinfo *) calloc(1, sizeof(pg_gssinfo)); + if (!port->gss) + ereport(FATAL, + (errcode(ERRCODE_OUT_OF_MEMORY), + errmsg("out of memory"))); + #endif + + /* The file descriptor of the client socket is the argument of --child */ + if (sscanf(argv[1], "--child=%d", &port->sock) != 1) + ereport(FATAL, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("invalid argument for --child: \"%s\"", argv[1]))); + + /* Default assumption about protocol to use */ + FrontendProtocol = port->proto = PG_PROTOCOL_LATEST; + + /* save process start time */ + port->SessionStartTime = GetCurrentTimestamp(); + MyStartTime = timestamptz_to_time_t(port->SessionStartTime); + + /* set these to empty in case they are needed */ + port->remote_host = ""; + port->remote_port = ""; + + MyProcPort = port; + + /* + * We can now initialize libpq and then enable reporting of ereport errors + * to the client. + */ + pq_init(); /* initialize libpq to talk to client */ + whereToSendOutput = DestRemote; /* now safe to ereport to client */ + + /* And pass off control to PostgresMain */ + PostgresMain(argc, argv, username); + + abort(); /* not reached */ + } + + /* * sigusr1_handler - handle signal conditions from child processes */ diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c index f1248a851bf90188da8d3a7e8b61ac99bf78ebbd..c6a7de63c77f79c85be2170d4e5254b790f3fd5e 100644 *** a/src/backend/tcop/postgres.c --- b/src/backend/tcop/postgres.c *************** process_postgres_switches(int argc, char *** 3257,3264 **** { gucsource = PGC_S_ARGV; /* switches came from command line */ ! /* Ignore the initial --single argument, if present */ ! if (argc > 1 && strcmp(argv[1], "--single") == 0) { argv++; argc--; --- 3257,3266 ---- { gucsource = PGC_S_ARGV; /* switches came from command line */ ! /* Ignore the initial --single or --child argument, if present */ ! if (argc > 1 && ! (strcmp(argv[1], "--single") == 0 || ! strncmp(argv[1], "--child=", 8) == 0)) { argv++; argc--; *************** PostgresMain(int argc, char *argv[], con *** 3522,3539 **** * standalone). */ if (!IsUnderPostmaster) - { MyProcPid = getpid(); MyStartTime = time(NULL); - } /* * Fire up essential subsystems: error and memory management * ! * If we are running under the postmaster, this is done already. */ ! if (!IsUnderPostmaster) MemoryContextInit(); SetProcessingMode(InitProcessing); --- 3524,3541 ---- * standalone). */ if (!IsUnderPostmaster) MyProcPid = getpid(); + if (MyProcPort == NULL) MyStartTime = time(NULL); /* * Fire up essential subsystems: error and memory management * ! * If we are running under the postmaster, this is done already; likewise ! * in child-postgres mode. */ ! if (MyProcPort == NULL) MemoryContextInit(); SetProcessingMode(InitProcessing); diff --git a/src/include/postmaster/postmaster.h b/src/include/postmaster/postmaster.h index 0fe7ec26db7646472fca248dee6982dc5c595c6c..6d4245cdc05ea5fea4baccf5dbc52dade7c00ff0 100644 *** a/src/include/postmaster/postmaster.h --- b/src/include/postmaster/postmaster.h *************** extern int postmaster_alive_fds[2]; *** 47,52 **** --- 47,55 ---- extern const char *progname; extern void PostmasterMain(int argc, char *argv[]) __attribute__((noreturn)); + + extern void ChildPostgresMain(int argc, char *argv[], const char *username) __attribute__((noreturn)); + extern void ClosePostmasterPorts(bool am_syslogger); extern int MaxLivePostmasterChildren(void); diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c index 9eaf41025beb652c6c242035490d93319a6bc5d0..c578a8ebbe6874825afbd0cb27cdfa2a3d01aec7 100644 *** a/src/interfaces/libpq/fe-connect.c --- b/src/interfaces/libpq/fe-connect.c *************** *** 17,22 **** --- 17,23 ---- #include <sys/types.h> #include <sys/stat.h> + #include <sys/wait.h> #include <fcntl.h> #include <ctype.h> #include <time.h> *************** static const PQconninfoOption PQconninfo *** 259,264 **** --- 260,271 ---- {"replication", NULL, NULL, NULL, "Replication", "D", 5}, + {"standalone_datadir", NULL, NULL, NULL, + "Standalone-Data-Directory", "D", 64}, + + {"standalone_backend", NULL, NULL, NULL, + "Standalone-Backend", "D", 64}, + /* Terminating entry --- MUST BE LAST */ {NULL, NULL, NULL, NULL, NULL, NULL, 0} *************** pqDropConnection(PGconn *conn) *** 360,365 **** --- 367,383 ---- if (conn->sock >= 0) closesocket(conn->sock); conn->sock = -1; + /* If we forked a child postgres process, wait for it to exit */ + if (conn->postgres_pid > 0) + { + while (waitpid(conn->postgres_pid, NULL, 0) < 0) + { + /* If interrupted by signal, keep waiting */ + if (errno != EINTR) + break; + } + } + conn->postgres_pid = -1; /* Discard any unread/unsent data */ conn->inStart = conn->inCursor = conn->inEnd = 0; conn->outCount = 0; *************** fillPGconn(PGconn *conn, PQconninfoOptio *** 643,648 **** --- 661,670 ---- conn->pghost = tmp ? strdup(tmp) : NULL; tmp = conninfo_getval(connOptions, "port"); conn->pgport = tmp ? strdup(tmp) : NULL; + tmp = conninfo_getval(connOptions, "standalone_datadir"); + conn->standalone_datadir = tmp ? strdup(tmp) : NULL; + tmp = conninfo_getval(connOptions, "standalone_backend"); + conn->standalone_backend = tmp ? strdup(tmp) : NULL; tmp = conninfo_getval(connOptions, "tty"); conn->pgtty = tmp ? strdup(tmp) : NULL; tmp = conninfo_getval(connOptions, "options"); *************** setKeepalivesWin32(PGconn *conn) *** 1311,1316 **** --- 1333,1384 ---- #endif /* SIO_KEEPALIVE_VALS */ #endif /* WIN32 */ + /* + * Fork and exec a command, connecting up a pair of anonymous sockets for + * communication. argv[fdarg] is modified by appending the child-side + * socket's file descriptor number. The parent-side socket's FD is returned + * in *psock, and the function result is the child's PID. + */ + static pid_t + fork_backend_child(char **argv, int fdarg, int *psock) + { + pid_t pid; + int socks[2]; + char newfdarg[32]; + + if (socketpair(AF_UNIX, SOCK_STREAM, 0, socks) < 0) + return -1; + + pid = fork(); + + if (pid < 0) + { + /* fork failed */ + close(socks[0]); + close(socks[1]); + return pid; + } + else if (pid == 0) + { + /* successful, in child process */ + close(socks[1]); + snprintf(newfdarg, sizeof(newfdarg), "%s%d", argv[fdarg], socks[0]); + argv[fdarg] = newfdarg; + execv(argv[0], argv); + perror(argv[0]); + exit(1); + } + else + { + /* successful, in parent process */ + close(socks[0]); + *psock = socks[1]; + } + + return pid; + } + + /* ---------- * connectDBStart - * Begin the process of making a connection to the backend. *************** connectDBStart(PGconn *conn) *** 1339,1344 **** --- 1407,1461 ---- conn->outCount = 0; /* + * If the standalone_datadir option was specified, ignore any host or + * port specifications and just try to fork a standalone backend. + */ + if (conn->standalone_datadir && conn->standalone_datadir[0]) + { + char *be_argv[10]; + + /* + * We set up the backend's command line in execv(3) style, so that + * we don't need to cope with shell quoting rules. + */ + if (conn->standalone_backend && conn->standalone_backend[0]) + be_argv[0] = conn->standalone_backend; + else /* assume we should use hard-wired path */ + be_argv[0] = PGBINDIR "/postgres"; + + be_argv[1] = "--child="; + be_argv[2] = "-D"; + be_argv[3] = conn->standalone_datadir; + be_argv[4] = (conn->dbName && conn->dbName[0]) ? conn->dbName : NULL; + be_argv[5] = NULL; + + conn->postgres_pid = fork_backend_child(be_argv, 1, &conn->sock); + if (conn->postgres_pid < 0) + { + char sebuf[256]; + + appendPQExpBuffer(&conn->errorMessage, + libpq_gettext("could not fork standalone backend: %s\n"), + pqStrerror(errno, sebuf, sizeof(sebuf))); + goto connect_errReturn; + } + + /* + * Go directly to CONNECTION_AUTH_OK state, since the standalone + * backend is not going to issue any authentication challenge to us. + * We're just waiting for startup to conclude. + */ + #ifdef USE_SSL + conn->allow_ssl_try = false; + #endif + conn->pversion = PG_PROTOCOL(3, 0); + conn->status = CONNECTION_AUTH_OK; + conn->asyncStatus = PGASYNC_BUSY; + + return 1; + } + + /* * Determine the parameters to pass to pg_getaddrinfo_all. */ *************** makeEmptyPGconn(void) *** 2708,2713 **** --- 2825,2831 ---- conn->auth_req_received = false; conn->password_needed = false; conn->dot_pgpass_used = false; + conn->postgres_pid = -1; #ifdef USE_SSL conn->allow_ssl_try = true; conn->wait_ssl_try = false; *************** freePGconn(PGconn *conn) *** 2784,2789 **** --- 2902,2911 ---- free(conn->pgport); if (conn->pgunixsocket) free(conn->pgunixsocket); + if (conn->standalone_datadir) + free(conn->standalone_datadir); + if (conn->standalone_backend) + free(conn->standalone_backend); if (conn->pgtty) free(conn->pgtty); if (conn->connect_timeout) diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h index 4a6c8fedf2b6b7f2f72d588c9c504dcc8d26cc0f..a9070ace73cead020b8b42f2720eebc771f39e6e 100644 *** a/src/interfaces/libpq/libpq-int.h --- b/src/interfaces/libpq/libpq-int.h *************** struct pg_conn *** 303,308 **** --- 303,310 ---- char *pgunixsocket; /* the Unix-domain socket that the server is * listening on; if NULL, uses a default * constructed from pgport */ + char *standalone_datadir; /* data directory for standalone backend */ + char *standalone_backend; /* executable for standalone backend */ char *pgtty; /* tty on which the backend messages is * displayed (OBSOLETE, NOT USED) */ char *connect_timeout; /* connection timeout (numeric string) */ *************** struct pg_conn *** 373,378 **** --- 375,383 ---- bool sigpipe_so; /* have we masked SIGPIPE via SO_NOSIGPIPE? */ bool sigpipe_flag; /* can we mask SIGPIPE via MSG_NOSIGNAL? */ + /* If we forked a child postgres process, its PID is kept here */ + pid_t postgres_pid; /* PID, or -1 if none */ + /* Transient state needed while establishing connection */ struct addrinfo *addrlist; /* list of possible backend addresses */ struct addrinfo *addr_cur; /* the one currently being tried */
On Thu, Sep 6, 2012 at 12:56 PM, Jeff Davis <pgsql@j-davis.com> wrote: > On Wed, 2012-09-05 at 17:03 -0400, Tom Lane wrote: >> In general I think the selling point for such a feature would be "no >> administrative hassles", and I believe that has to go not only for the >> end-user experience but also for the application-developer experience. >> If you have to manage checkpointing and vacuuming in the application, >> you're probably soon going to look for another database. > > Maybe there could be some hooks (e.g., right after completing a > statement) that see whether a vacuum or checkpoint is required? VACUUM > can't be run in a transaction block[1], so there are some details to > work out, but it might be a workable approach. If it was me, I'd want finer grained control of if/when automatic background optimization work happened. Something like DoBackgroundWork(int forThisManySeconds). Of course, for that to work, we'd need to have resumable vacuum. I like the idea of keeping everything single threaded. merlin
On Friday, September 07, 2012 11:21:00 PM Merlin Moncure wrote: > On Thu, Sep 6, 2012 at 12:56 PM, Jeff Davis <pgsql@j-davis.com> wrote: > > On Wed, 2012-09-05 at 17:03 -0400, Tom Lane wrote: > >> In general I think the selling point for such a feature would be "no > >> administrative hassles", and I believe that has to go not only for the > >> end-user experience but also for the application-developer experience. > >> If you have to manage checkpointing and vacuuming in the application, > >> you're probably soon going to look for another database. > > > > Maybe there could be some hooks (e.g., right after completing a > > statement) that see whether a vacuum or checkpoint is required? VACUUM > > can't be run in a transaction block[1], so there are some details to > > work out, but it might be a workable approach. > > If it was me, I'd want finer grained control of if/when automatic > background optimization work happened. Something like > DoBackgroundWork(int forThisManySeconds). Of course, for that to > work, we'd need to have resumable vacuum. > > I like the idea of keeping everything single threaded. To me this path seems to be the best way to never get the feature at all... Andres -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
On 9/2/12 7:23 PM, Tom Lane wrote: > 4. As coded, the backend assumes the incoming pipe is on its FD 0 and the > outgoing pipe is on its FD 1. This made the command line simple but I'm > having second thoughts about it: if anything inside the backend tries to > read stdin or write stdout, unpleasant things will happen. It might be > better to not reassign the pipe FD numbers. In that case we'd have to > pass them on the command line, so that the syntax would be something > like "postgres --child 4,5 -D pgdata ...". Would it be sufficient to just hard-code the FD's to use? I'm not sure why someone would need to change them, so long aswe steer clear of STD(IN|OUT|ERR)... -- Jim C. Nasby, Database Architect jim@nasby.net 512.569.9461 (cell) http://jim.nasby.net
On Wed, Sep 5, 2012 at 10:29 PM, Andrew Dunstan <andrew@dunslane.net> wrote:
/me shares the feeling :)
On 09/05/2012 10:14 PM, Tom Lane wrote:
The people who would be interested in this are currently using something
like SQLite within a single application program.
Exactly. I think it's worth stating that this has a HUGE potential audience, and if we can get to this the deployment of Postgres could mushroom enormously. I'm really quite excited about it.
/me shares the feeling :)
--
Gurjeet Singh
A Dijous, 6 de setembre de 2012 00:30:53, Josh Berkus va escriure: > > In general I think the selling point for such a feature would be "no > > administrative hassles", and I believe that has to go not only for the > > end-user experience but also for the application-developer experience. > > If you have to manage checkpointing and vacuuming in the application, > > you're probably soon going to look for another database. > > Well, don't discount the development/testing case. If you do agile or > TDD (a lot of people do), you often have a workload which looks like: > > 1) Start framework > 2) Start database > 3) Load database with test data > 4) Run tests > 5) Print results > 6) Shut down database > > In a case like that, you can live without checkpointing, even; the > database is ephemeral. > > In other words, let's make this a feature and document it for use in > testing, and that it's not really usable for production embedded apps yet. +1. Some projects such as tryton would benefit from this feature. -- Albert Cervera i Areny http://www.NaN-tic.com Tel: +34 93 553 18 03 http://twitter.com/albertnan http://www.nan-tic.com/blog
On Friday, September 07, 2012 11:19 PM Tom Lane wrote: Heikki Linnakangas <hlinnaka@iki.fi> writes: >> Would socketpair(2) be simpler? >I've not done anything yet about the potential security issues >associated with untrusted libpq connection strings. I think this >is still at the proof-of-concept stage; in particular, it's probably > time to see if we can make it work on Windows before we worry more >about that. I have started working on this patch to make it work on Windows. The 3 main things to make it work are: 1. Windows equivalent for socketpair - This as suggested previously in this thread earlier code of pgpipe can suffice theneed. Infact I have checked on net as well, most implementations are similar to pgpipe implementation. So I prefered touse the existing code which was removed. 2. Windows equivalent for fork-execv - This part can be done by CreateProcess,it can be similar to internal_forkexec exceptfor path where it uses shared memory to pass parameters, I am trying to directly pass parameters to CreateProcess. 3. Windows equivalent for waitpid - Actually there can be 2 ways to accomplish this a. use waitforsingleobject with process handle, but in some places it is mentioned it might not work forall windows versions. Can someone pls confirm about. I shall try on my PC to test the same. b. use existing infrastructureof waitpid, however it is not for single process and it might need some changes to make it work for singleprocess or may be we can use it directly. However currentlyit is in postmaster.c, so it need to be moved so that we can access it from fe-connect.c in libpq as well. c. suggest if you know of other ways to handle it or which from above2 would be better? Some other doubts: 1. does this follow the behavior that admin users will not be allowed to invoke postgres child process? 2. to find standalone_backend incase user didn't input, do we need mechanism similar to getInstallationPaths()? Any other comments/suggestions? With Regards, Amit Kapila.
Amit kapila <amit.kapila@huawei.com> writes: > 1. does this follow the behavior that admin users will not be allowed to invoke postgres child process? That's an interesting question. I'm not sure if we'd want to disable the no-root check on the Unix side, but it might make sense to. But this has no bearing on what libpq does, does it? > 2. to find standalone_backend incase user didn't input, do we need mechanism similar to getInstallationPaths()? No. There is no reason at all for libpq to think it is part of a postgres-supplied program, so it can't use any relative-path tricks, even if it had the program's argv[0] which it does not. Either the compiled-in path works, or the user has to give one. (But having said that, if Windows provides a way for a DLL to find out its own filesystem location, maybe relative path from there would work.) regards, tom lane
Hi, On Wednesday, September 05, 2012 06:00:18 PM Tom Lane wrote: > "anarazel@anarazel.de" <andres@anarazel.de> writes: > > I am not saying its bad that it is slower, that's absolutely OK. Just > > that it will take a variable amount of time till you can run pgdump > > again and its not easily detectable without looping and trying again. > > Well, that's why the proposed libpq code is written to wait for the > child postgres to exit when closing the connection. > > Admittedly, if you forcibly kill pg_dump (or some other client) and then > immediately try to start a new one, it's not clear how long you'll have > to wait. Yep, thats the case I was talking about upthread ;) On Monday, September 03, 2012 11:04:15 PM Andres Freund wrote: > Don't we expect some more diligence in applications using this against > letting such a child continue to run after ctrl-c/SIGTERMing e.g. pg_dump > in comparison to closing a normal database connection? I would really expect any postgres supplied core tool to try to handle that case. Thats not exactly trivial to do right and requires cooperation of the application. Because that requires knowledge about the operation mode in those anyway I don't really see an extra libpq call as problematic... > But so what? Anything we might do in this space is going to > have pluses and minuses. True. Greetings, Andres -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
On Sunday, September 09, 2012 8:46 PM Tom Lane wrote: Amit kapila <amit.kapila@huawei.com> writes: >> 1. does this follow the behavior that admin users will not be allowed to invoke postgres child process? > That's an interesting question. I'm not sure if we'd want to disable > the no-root check on the Unix side, but it might make sense to. But > this has no bearing on what libpq does, does it? No, it has no bearing with what libpq does. Its only related to postgres invocation, because as soon as postgres get invoked, it first checks about root. If we want the root check can be avoided easily by checking argv. >> 2. to find standalone_backend incase user didn't input, do we need mechanism similar to getInstallationPaths()? > No. There is no reason at all for libpq to think it is part of a > postgres-supplied program, so it can't use any relative-path tricks, > even if it had the program's argv[0] which it does not. Either the > compiled-in path works, or the user has to give one. > (But having said that, if Windows provides a way for a DLL to find > out its own filesystem location, maybe relative path from there would > work.) At first, I shall go ahead with the current way (Either the compiled-in path works, or the user has to give one.).
On Sunday, September 09, 2012 1:37 PM Amit Kapila wrote: On Friday, September 07, 2012 11:19 PM Tom Lane wrote: Heikki Linnakangas <hlinnaka@iki.fi> writes: >>> Would socketpair(2) be simpler? >>I've not done anything yet about the potential security issues >>associated with untrusted libpq connection strings. I think this >>is still at the proof-of-concept stage; in particular, it's probably >> time to see if we can make it work on Windows before we worry more >>about that. > I have started working on this patch to make it work on Windows. The 3 main things to make it work are: > 1. Windows equivalent for socketpair - This as suggested previously in this thread earlier code of pgpipe can suffice > the need. Infact I have checked on net as well, most implementations are similar to pgpipe implementation. So I > prefered to use the existing code which was removed. > 2. Windows equivalent for fork-execv - This part can be done by CreateProcess,it can be similar to internal_forkexec > except for path where it uses shared memory to pass parameters, I am trying to directly pass parameters to > CreateProcess. Directly passing parameters doesn't suffice for all parameters, as for socket we need to duplicate the socket using WSADuplicateSocket() which returns little big structure which is better to be passed via shared memory. > 3. Windows equivalent for waitpid - Actually there can be 2 ways to accomplish this > a. use waitforsingleobject with process handle, but in some places it is mentioned it might not work for all > windows versions. Can someone pls confirm about. I shall try on my PC to test the same. > b. use existing infrastructure of waitpid, however it is not for single process and it might need some changes to > make it work for single process or may be we can use it directly. However currently it is in postmaster.c, so > it need to be moved so that we can access it from fe-connect.c in libpq as well. > c. suggest if you know of other ways to handle it or which from above 2 would be better? I have used method - a (waitforsingleobject) and it worked fine. With the above implementation, it is working on Windows. Now the work left is as follows: 1. Refactoring of code 2. Error handling in paths 3. Check if anything is missing and implement for same. 4. Test the patch for Windows. Any comments/suggestions? With Regards, Amit Kapila
On Sun, Sep 2, 2012 at 8:23 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Since we are forking a child process anyway, and potentially other auxiliary processes too, would it make sense to allow multiple backends too (allow multiple local applications connect to this instance)? I believe (I may be wrong) that embedded databases (SQLLite et. al.) use a library interface, in that the application makes a library call and waits for that API call to finish (unless, of course, the library supports async operations or the application uses threads). The implementation you are proposing uses socket communication, which lends itself very easily to client-server model, and if possible, it should be leveraged to provide for multiple applications talking to one local DB.
I have this use case in mind: An application is running using this interface, and an admin now wishes to do some maintenance, or inspect something, so they can launch local pgAdmin using the same connection string as used by the original application. This will allow an admin to perform tuning, etc. without having to first shutdown the application.
Notably, while the lack of any background processes is just what you want
for pg_upgrade and disaster recovery, an ordinary application is probably
going to want to rely on autovacuum; and we need bgwriter and other
background processes for best performance. So I'm speculating about
having a postmaster process that isn't listening on any ports, but is
managing background processes in addition to a single child backend.
That's for another day though.
Since we are forking a child process anyway, and potentially other auxiliary processes too, would it make sense to allow multiple backends too (allow multiple local applications connect to this instance)? I believe (I may be wrong) that embedded databases (SQLLite et. al.) use a library interface, in that the application makes a library call and waits for that API call to finish (unless, of course, the library supports async operations or the application uses threads). The implementation you are proposing uses socket communication, which lends itself very easily to client-server model, and if possible, it should be leveraged to provide for multiple applications talking to one local DB.
I have this use case in mind: An application is running using this interface, and an admin now wishes to do some maintenance, or inspect something, so they can launch local pgAdmin using the same connection string as used by the original application. This will allow an admin to perform tuning, etc. without having to first shutdown the application.
Here's how this might impact the design (I may very well be missing many other things, and I have no idea if this is implementable or not):
.) Database starts when the first such application is launched.
.) Database shuts down when the last such application disconnects.
.) Postgres behaves much like a regular Postgres installation, except that it does not accept connections over TCP/IP or Unix Doamin Sockets.
.) The above implies that we use regular Postmaster machinery, and not the --sinlgle machinery.
.) Second and subsequent applications use the postmaster.pid (or something similar) to find an already running instance, and connect to it.
.) There's a race condition where the second application is starting up, hoping to connect to an already running insatnce, but the first application disconnects (and hence shuts down the DB) before the second one can successfully connect.
I haven't thought much about the security implications of this yet. Maybe the socket permissions would restrict an unauthorized user user from connecting to this instance.
--
On Mon, Sep 10, 2012 at 11:12 AM, Gurjeet Singh <singh.gurjeet@gmail.com> wrote: > On Sun, Sep 2, 2012 at 8:23 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >> >> Notably, while the lack of any background processes is just what you want >> for pg_upgrade and disaster recovery, an ordinary application is probably >> going to want to rely on autovacuum; and we need bgwriter and other >> background processes for best performance. So I'm speculating about >> having a postmaster process that isn't listening on any ports, but is >> managing background processes in addition to a single child backend. >> That's for another day though. > > > Since we are forking a child process anyway, and potentially other auxiliary > processes too, would it make sense to allow multiple backends too (allow > multiple local applications connect to this instance)? I believe (I may be > wrong) that embedded databases (SQLLite et. al.) use a library interface, in > that the application makes a library call and waits for that API call to > finish (unless, of course, the library supports async operations or the > application uses threads). The implementation you are proposing uses socket > communication, which lends itself very easily to client-server model, and if > possible, it should be leveraged to provide for multiple applications > talking to one local DB. > > I have this use case in mind: An application is running using this > interface, and an admin now wishes to do some maintenance, or inspect > something, so they can launch local pgAdmin using the same connection string > as used by the original application. This will allow an admin to perform > tuning, etc. without having to first shutdown the application. > > Here's how this might impact the design (I may very well be missing many > other things, and I have no idea if this is implementable or not): > > .) Database starts when the first such application is launched. > .) Database shuts down when the last such application disconnects. > .) Postgres behaves much like a regular Postgres installation, except that > it does not accept connections over TCP/IP or Unix Doamin Sockets. > .) The above implies that we use regular Postmaster machinery, and not the > --sinlgle machinery. > .) Second and subsequent applications use the postmaster.pid (or something > similar) to find an already running instance, and connect to it. > .) There's a race condition where the second application is starting up, > hoping to connect to an already running insatnce, but the first application > disconnects (and hence shuts down the DB) before the second one can > successfully connect. > > I haven't thought much about the security implications of this yet. Maybe > the socket permissions would restrict an unauthorized user user from > connecting to this instance. That's kind of the reason why I suggested up thread tring to decouple the *starting* of the backend with the "options" to PQ connect... A "Helper function" in libpq could easily start the backend, and possibly return a conninfostring to give PQconnectdb... But if they are decoupled, I could easily envision an app that "pauses" it's use of the backend to allow some other libpq access to it for a period. You'd have to "trust" whatever else you let talk on the FD to the backend, but it might be useful... -- Aidan Van Dyk Create like a god, aidan@highrise.ca command like a king, http://www.highrise.ca/ work like a slave.
On 10.09.2012 18:12, Gurjeet Singh wrote: > On Sun, Sep 2, 2012 at 8:23 PM, Tom Lane<tgl@sss.pgh.pa.us> wrote: > >> Notably, while the lack of any background processes is just what you want >> for pg_upgrade and disaster recovery, an ordinary application is probably >> going to want to rely on autovacuum; and we need bgwriter and other >> background processes for best performance. So I'm speculating about >> having a postmaster process that isn't listening on any ports, but is >> managing background processes in addition to a single child backend. >> That's for another day though. > > Since we are forking a child process anyway, and potentially other > auxiliary processes too, would it make sense to allow multiple backends too > (allow multiple local applications connect to this instance)? I believe (I > may be wrong) that embedded databases (SQLLite et. al.) use a library > interface, in that the application makes a library call and waits for that > API call to finish (unless, of course, the library supports async > operations or the application uses threads). The implementation you are > proposing uses socket communication, which lends itself very easily to > client-server model, and if possible, it should be leveraged to provide for > multiple applications talking to one local DB. [scratches head] How's that different from the normal postmaster mode? - Heikki
On Mon, Sep 10, 2012 at 11:43 AM, Heikki Linnakangas <hlinnaka@iki.fi> wrote:
As I described in later paragraphs, it'd behave like an embedded database, like SQLite etc., so the database will startup and shutdown with the application, and provide other advantages we're currently trying to provide, like zero-maintenance. But it will not mandate that only one application talk to it at a time, and allow as many applications as it would in postmaster mode. So the database would be online as long as any application is connected to it, and it will shutdown when the last application disconnects.
As being implemented right now, there's very little difference between --single and --child modes. I guess I am asking for a --child mode implementation that is closer to a postmaster than --single.
On 10.09.2012 18:12, Gurjeet Singh wrote:[scratches head] How's that different from the normal postmaster mode?On Sun, Sep 2, 2012 at 8:23 PM, Tom Lane<tgl@sss.pgh.pa.us> wrote:Notably, while the lack of any background processes is just what you want
for pg_upgrade and disaster recovery, an ordinary application is probably
going to want to rely on autovacuum; and we need bgwriter and other
background processes for best performance. So I'm speculating about
having a postmaster process that isn't listening on any ports, but is
managing background processes in addition to a single child backend.
That's for another day though.
Since we are forking a child process anyway, and potentially other
auxiliary processes too, would it make sense to allow multiple backends too
(allow multiple local applications connect to this instance)? I believe (I
may be wrong) that embedded databases (SQLLite et. al.) use a library
interface, in that the application makes a library call and waits for that
API call to finish (unless, of course, the library supports async
operations or the application uses threads). The implementation you are
proposing uses socket communication, which lends itself very easily to
client-server model, and if possible, it should be leveraged to provide for
multiple applications talking to one local DB.
As I described in later paragraphs, it'd behave like an embedded database, like SQLite etc., so the database will startup and shutdown with the application, and provide other advantages we're currently trying to provide, like zero-maintenance. But it will not mandate that only one application talk to it at a time, and allow as many applications as it would in postmaster mode. So the database would be online as long as any application is connected to it, and it will shutdown when the last application disconnects.
As being implemented right now, there's very little difference between --single and --child modes. I guess I am asking for a --child mode implementation that is closer to a postmaster than --single.
Best regards,
--
Gurjeet Singh <singh.gurjeet@gmail.com> writes: > On Mon, Sep 10, 2012 at 11:43 AM, Heikki Linnakangas <hlinnaka@iki.fi>wrote: >> [scratches head] How's that different from the normal postmaster mode? > As I described in later paragraphs, it'd behave like an embedded database, > like SQLite etc., so the database will startup and shutdown with the > application, and provide other advantages we're currently trying to > provide, like zero-maintenance. But it will not mandate that only one > application talk to it at a time, and allow as many applications as it > would in postmaster mode. So the database would be online as long as any > application is connected to it, and it will shutdown when the last > application disconnects. I am having a hard time getting excited about that. To me it sounds like it's a regular postmaster, except with a response-time problem for connections that occur when there had been no active client before. The point of the proposal that I am making is to have a simple, low-maintenance solution for people who need a single-application database. A compromise somewhere in the middle isn't likely to be an improvement for anybody. For instance, if you want to have additional connections, you open up a whole collection of communication and authentication issues, which potential users of a single-application database don't want to cope with. > As being implemented right now, there's very little difference between > --single and --child modes. I guess I am asking for a --child mode > implementation that is closer to a postmaster than --single. There are good reasons for wanting something that is closer to --single: pg_upgrade being one, and having a friendlier user interface for disaster recovery in --single mode being another. In these cases, you not only don't need the capability for additional applications to connect, it is actually important that it's impossible for them to do so. regards, tom lane
> The point of the proposal that I am making is to have a simple, > low-maintenance solution for people who need a single-application > database. A compromise somewhere in the middle isn't likely to be an > improvement for anybody. For instance, if you want to have additional > connections, you open up a whole collection of communication and > authentication issues, which potential users of a single-application > database don't want to cope with. Yes, exactly. In fact, most of the folks who would want an embedded PostgreSQL either want no authentication at all, or only a single password. So supporting authentication options other than trust or md5 is not required, or desired AFAIK. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com
Josh Berkus <josh@agliodbs.com> wrote: > In fact, most of the folks who would want an embedded PostgreSQL > either want no authentication at all, or only a single password. > So supporting authentication options other than trust or md5 is > not required, or desired AFAIK. I don't know whether it's worth the trouble of doing so, but serializable transactions could skip all the SSI predicate locking and conflict checking when in single-connection mode. With only one connection the transactions could never overlap, so there would be no chance of serialization anomalies when running snapshot isolation. The reason I wonder whether it is worth the trouble is that it would only really matter if someone had code they wanted to run under both normal and single-connection modes. For single-connection only, they could just choose REPEATABLE READ to get exactly the same semantics. -Kevin
On Mon, Sep 10, 2012 at 9:59 AM, Josh Berkus <josh@agliodbs.com> wrote: > >> The point of the proposal that I am making is to have a simple, >> low-maintenance solution for people who need a single-application >> database. A compromise somewhere in the middle isn't likely to be an >> improvement for anybody. For instance, if you want to have additional >> connections, you open up a whole collection of communication and >> authentication issues, which potential users of a single-application >> database don't want to cope with. > > Yes, exactly. > > In fact, most of the folks who would want an embedded PostgreSQL either > want no authentication at all, or only a single password. So supporting > authentication options other than trust or md5 is not required, or > desired AFAIK. I agree people who want embedded postgres probably want no authentication. For the sake of test/local development use cases, I still question the usefulness of a quasi-embeddable mode that does not support multiple executors as SQLite does -- it's pretty murderous for the testing use case for a number of people if they intend to have parity with their production environment. I could see this being useful for, say, iOS (although there is a definite chance that one will want to use multiple threads/simultaneous xacts in applications), or a network router, except that pg_xlog and the catalog are rather enormous for those use cases. So while the embedded use case is really appealing in general, I'm still intuitively quite skeptical of the omission of multi-executor support, especially considering that people have gotten used to being able to that with the incredibly popular SQLite. Could EXEC_BACKEND be repurposed -- even on *nix-- to make this work someday? -- fdr
On Monday, September 10, 2012 8:20 PM Amit Kapila wrote: On Sunday, September 09, 2012 1:37 PM Amit Kapila wrote: On Friday, September 07, 2012 11:19 PM Tom Lane wrote: Heikki Linnakangas <hlinnaka@iki.fi> writes: >>> Would socketpair(2) be simpler? >>>I've not done anything yet about the potential security issues >>>associated with untrusted libpq connection strings. I think this >>is still at the proof-of-concept stage; in particular, it's probably >> time to see if we can make it work on Windows before we worry more >>about that. > I have started working on this patch to make it work on Windows. The 3 main things to make it work are: The patch which contains Windows implementation as well is attached with this mail. It contains changes related to following 1. waitpid 2. socketpair 3. fork-exec The following is still left: 1. Error handling in all paths 2. During test, I found if i try to run with admin user, it throws error but psql doesn't comes out. I will look into this issue. However as in previous mail discussion there is a decision pending whether in standalonemode we need admin user behavior. 3. Will do some more test in Windows. Currently I have prepared a patch on top of your changes, please let me know if that is okay. Also, it will be better for me if you can tell me how I can further contribute. With Regards, Amit Kapila
Attachment
On Tuesday, September 11, 2012 7:07 PM Amit Kapila wrote: On Monday, September 10, 2012 8:20 PM Amit Kapila wrote: On Sunday, September 09, 2012 1:37 PM Amit Kapila wrote: On Friday, September 07, 2012 11:19 PM Tom Lane wrote: Heikki Linnakangas <hlinnaka@iki.fi> writes: >>>> Would socketpair(2) be simpler? >>>>I've not done anything yet about the potential security issues >>>>associated with untrusted libpq connection strings. I think this >>>is still at the proof-of-concept stage; in particular, it's probably >>> time to see if we can make it work on Windows before we worry more >>>about that. >> I have started working on this patch to make it work on Windows. The 3 main things to make it work are: >The patch which contains Windows implementation as well is attached with this mail. It contains changes related to following >1. waitpid >2. socketpair >3. fork-exec >The following is still left: > 1. Error handling in all paths The modified version 2 contains error handling in all paths. With Regards, Amit Kapila
Attachment
On Fri, Sep 14, 2012 at 6:42 AM, Amit kapila <amit.kapila@huawei.com> wrote: > On Tuesday, September 11, 2012 7:07 PM Amit Kapila wrote: > On Monday, September 10, 2012 8:20 PM Amit Kapila wrote: > On Sunday, September 09, 2012 1:37 PM Amit Kapila wrote: > On Friday, September 07, 2012 11:19 PM Tom Lane wrote: > Heikki Linnakangas <hlinnaka@iki.fi> writes: >>>>> Would socketpair(2) be simpler? > > > >>>>>I've not done anything yet about the potential security issues >>>>>associated with untrusted libpq connection strings. I think this >>>>is still at the proof-of-concept stage; in particular, it's probably >>>> time to see if we can make it work on Windows before we worry more >>>>about that. > >>> I have started working on this patch to make it work on Windows. The 3 > main things to make it work are: > >>The patch which contains Windows implementation as well is attached with this mail. It contains changes related to following >>1. waitpid >>2. socketpair >>3. fork-exec > >>The following is still left: >> 1. Error handling in all paths > > The modified version 2 contains error handling in all paths. I didn't see that this patch was added to a commitfest -- should it have been? I very much like Tom's proposed starting point for this feature as a replacement for --single. Hate to see this die on the vine. Would some testing on windows be what's needed to get the ball rolling? merlin
On 10 September 2012 17:50, Tom Lane <tgl@sss.pgh.pa.us> wrote: > The point of the proposal that I am making is to have a simple, > low-maintenance solution for people who need a single-application > database. A compromise somewhere in the middle isn't likely to be an > improvement for anybody. For instance, if you want to have additional > connections, you open up a whole collection of communication and > authentication issues, which potential users of a single-application > database don't want to cope with. So the proposal is to implement a database that can't ever have 2 or more connections. And so any data it stores cannot ever be accessed by another connection, so all forms of replication are excluded and all maintenance actions force the database to be unavailable for a period of time. Those two things are barriers of the most major kind to anybody working in an enterprise with connected data and devices. The only people that want this are people with a very short term view of the purpose of their applications, and disregard for the value and permanence of the data stored. They may not want to cope with those issues *now* but they will later and won't thank us for implementing it in a way that means it can never be achieved. To be honest, I can't believe I'm reading this. And worse, it's on our Don't Want list, and nobody has said stop. It's almost impossible to purchase a CPU these days that doesn't have multiple cores, so the whole single-process architecture is just dead. Yes, we want Postgres installed everywhere, but this isn't the way to achieve that. I agree we should allow a PostgreSQL installation to work for a single user, but I don't see that requires other changes. This idea will cause endless bugs, thinkos and severely waste our time. So without a much better justification, I don't think we should do this. -- Simon Riggs http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
On Mon, Nov 12, 2012 at 1:21 PM, Simon Riggs <simon@2ndquadrant.com> wrote: > On 10 September 2012 17:50, Tom Lane <tgl@sss.pgh.pa.us> wrote: > >> The point of the proposal that I am making is to have a simple, >> low-maintenance solution for people who need a single-application >> database. A compromise somewhere in the middle isn't likely to be an >> improvement for anybody. For instance, if you want to have additional >> connections, you open up a whole collection of communication and >> authentication issues, which potential users of a single-application >> database don't want to cope with. > > So the proposal is to implement a database that can't ever have 2 or > more connections. And so any data it stores cannot ever be accessed by > another connection, so all forms of replication are excluded and all > maintenance actions force the database to be unavailable for a period > of time. Those two things are barriers of the most major kind to > anybody working in an enterprise with connected data and devices. The > only people that want this are people with a very short term view of > the purpose of their applications, and disregard for the value and > permanence of the data stored. They may not want to cope with those > issues *now* but they will later and won't thank us for implementing > it in a way that means it can never be achieved. > > To be honest, I can't believe I'm reading this. > > And worse, it's on our Don't Want list, and nobody has said stop. > > It's almost impossible to purchase a CPU these days that doesn't have > multiple cores, so the whole single-process architecture is just dead. > Yes, we want Postgres installed everywhere, but this isn't the way to > achieve that. > > I agree we should allow a PostgreSQL installation to work for a single > user, but I don't see that requires other changes. This idea will > cause endless bugs, thinkos and severely waste our time. So without a > much better justification, I don't think we should do this. I couldn't disagree more. The patch is small, logical, and fixes an awful problem, namely that --single mode is basically unusable. As to your wider point (namely, that you can't connect to it, therefore it's bad), it has got to be refuted by numerous competing solutions in the market such as http://www.firebirdsql.org/manual/fbmetasecur-embedded.html, and many others. While it's not as common as it used to be, now and then a piece of software needs to distribute an application as part of a boxed product. Postgres is horrible at this and doesn't have to be; imagine how much easier the lives of poker tracker would be (for *some* of its users) with an integrated standalone mode: google 'poker tracker postgresql' and take a good long look at problems people face in this scenario. merlin
On 12 November 2012 21:26, Merlin Moncure <mmoncure@gmail.com> wrote: > I couldn't disagree more. The patch is small, logical, and fixes an > awful problem, namely that --single mode is basically unusable. As to > your wider point (namely, that you can't connect to it, therefore it's > bad), it has got to be refuted by numerous competing solutions in the > market such as http://www.firebirdsql.org/manual/fbmetasecur-embedded.html, > and many others. Small is not an argument in favour, just a statement of ease, like jumping off a cliff. i.e. lemmings. > While it's not as common as it used to be, now and then a piece of > software needs to distribute an application as part of a boxed > product. Postgres is horrible at this and doesn't have to be; imagine > how much easier the lives of poker tracker would be (for *some* of its > users) with an integrated standalone mode: google 'poker tracker > postgresql' and take a good long look at problems people face in this > scenario. I get the installability thang, very very much, I just don't see the single process thing as the only solution. At very least an open minded analysis of the actual problem and ways of solving it is called for, not just reach for a close to hand solution. I don't ever want to hear someone reject a patch cos it would mess up poker tracker. The point is it complicates the code, introduces restrictions into what is possible and is just more inertia onto development. -- Simon Riggs http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
On Monday, November 12, 2012 8:31 PM Merlin Moncure wrote: On Fri, Sep 14, 2012 at 6:42 AM, Amit kapila <amit.kapila@huawei.com> wrote: > On Tuesday, September 11, 2012 7:07 PM Amit Kapila wrote: > On Monday, September 10, 2012 8:20 PM Amit Kapila wrote: > On Sunday, September 09, 2012 1:37 PM Amit Kapila wrote: > On Friday, September 07, 2012 11:19 PM Tom Lane wrote: > Heikki Linnakangas <hlinnaka@iki.fi> writes: > >>>>> Would socketpair(2) be simpler? > >>>> I have started working on this patch to make it work on Windows. The 3 > main things to make it work are: > >>>The patch which contains Windows implementation as well is attached with this mail. It contains changes related to following >>>1. waitpid >>>2. socketpair >>3. fork-exec > >>>The following is still left: >>> 1. Error handling in all paths > >> The modified version 2 contains error handling in all paths. > I didn't see that this patch was added to a commitfest -- should it > have been? I very much like Tom's proposed starting point for this > feature as a replacement for --single. Hate to see this die on the > vine. Would some testing on windows be what's needed to get the ball > rolling? After Windows implementation, I have done first level tests also to make sure it works. I think Tom is the right person to comment on how to see this patch move forward. I am not sure what's in his mind that he didn't provide the feedback or proceeded to complete it. Is it due to time or he might have forseen some design or usecase problem, if it's due to time then I think it can be persuaded. With Regards, Amit Kapila.
On Tuesday, November 13, 2012 3:11 AM Simon Riggs wrote: On 12 November 2012 21:26, Merlin Moncure <mmoncure@gmail.com> wrote: >> I couldn't disagree more. The patch is small, logical, and fixes an >> awful problem, namely that --single mode is basically unusable. As to >> your wider point (namely, that you can't connect to it, therefore it's >> bad), it has got to be refuted by numerous competing solutions in the >> market such as http://www.firebirdsql.org/manual/fbmetasecur-embedded.html, >> and many others. As far as I remember even MySQL provides such a mode. > Small is not an argument in favour, just a statement of ease, like >jumping off a cliff. i.e. lemmings. >> While it's not as common as it used to be, now and then a piece of >> software needs to distribute an application as part of a boxed >> product. Postgres is horrible at this and doesn't have to be; imagine >> how much easier the lives of poker tracker would be (for *some* of its >> users) with an integrated standalone mode: google 'poker tracker >> postgresql' and take a good long look at problems people face in this >> scenario. >I get the installability thang, very very much, I just don't see the >single process thing as the only solution. At very least an open >minded analysis of the actual problem and ways of solving it is called >for, not just reach for a close to hand solution. Some other usecase where I have seen it required is in telecom billing apps. In telecom application where this solution works, needs other maintainence connections as well. Some of the reasons for its use are performance and less maintainence overhead and also their data requirements are also not so high. So even if this solution doesn't meet all requirements of single process solution (and neither I think it is written to addressall) but can't we think of it as first version and then based on requirements extend it to have other capabilities: a. to have a mechnism for other background processes (autovacuum, checkpoint, ..). b. more needs to be thought of.. With Regards, Amit Kapila.
On 2012-11-12 19:21:28 +0000, Simon Riggs wrote: > On 10 September 2012 17:50, Tom Lane <tgl@sss.pgh.pa.us> wrote: > > > The point of the proposal that I am making is to have a simple, > > low-maintenance solution for people who need a single-application > > database. A compromise somewhere in the middle isn't likely to be an > > improvement for anybody. For instance, if you want to have additional > > connections, you open up a whole collection of communication and > > authentication issues, which potential users of a single-application > > database don't want to cope with. > > So the proposal is to implement a database that can't ever have 2 or > more connections. > ... > It's almost impossible to purchase a CPU these days that doesn't have > multiple cores, so the whole single-process architecture is just dead. > Yes, we want Postgres installed everywhere, but this isn't the way to > achieve that. > > I agree we should allow a PostgreSQL installation to work for a single > user, but I don't see that requires other changes. This idea will > cause endless bugs, thinkos and severely waste our time. So without a > much better justification, I don't think we should do this. I personally think that a usable & scriptable --single mode is justification enough, even if you don't aggree with the other goals. Having to wait for hours just enter one more command because --single doesn't support any scripts sucks. Especially in recovery situations. I also don't think a single-backend without further child processes is all that helpful - but I think this might be a very useful stepping stone. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
On 13 November 2012 06:14, Amit kapila <amit.kapila@huawei.com> wrote: >>I get the installability thang, very very much, I just don't see the >>single process thing as the only solution. At very least an open >>minded analysis of the actual problem and ways of solving it is called >>for, not just reach for a close to hand solution. > > Some other usecase where I have seen it required is in telecom billing apps. > In telecom application where this solution works, needs other maintainence connections as well. > Some of the reasons for its use are performance and less maintainence overhead and also their data requirements are > also not so high. > So even if this solution doesn't meet all requirements of single process solution (and neither I think it is written toaddress all) but can't we think of it as first version and then based on requirements extend it to have other capabilities: > a. to have a mechnism for other background processes (autovacuum, checkpoint, ..). > b. more needs to be thought of.. Why would we spend time trying to put back something that is already there? Why not simply avoid removing it in the first place? -- Simon Riggs http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
Simon Riggs escribió: > > So even if this solution doesn't meet all requirements of single > > process solution (and neither I think it is written to address all) > > but can't we think of it as first version and then based on > > requirements extend it to have other capabilities: > > a. to have a mechnism for other background processes (autovacuum, checkpoint, ..). > > b. more needs to be thought of.. > > Why would we spend time trying to put back something that is already > there? Why not simply avoid removing it in the first place? Actually, the whole point of this solution originally was just to serve pg_upgrade needs, so that it doesn't have to start a complete postmaster environment just to have to turn off most of what postmaster does, and with enough protections to disallow everyone else from connecting. -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
On 13 November 2012 13:05, Alvaro Herrera <alvherre@2ndquadrant.com> wrote: > Simon Riggs escribió: > >> > So even if this solution doesn't meet all requirements of single >> > process solution (and neither I think it is written to address all) >> > but can't we think of it as first version and then based on >> > requirements extend it to have other capabilities: >> > a. to have a mechnism for other background processes (autovacuum, checkpoint, ..). >> > b. more needs to be thought of.. >> >> Why would we spend time trying to put back something that is already >> there? Why not simply avoid removing it in the first place? > > Actually, the whole point of this solution originally was just to serve > pg_upgrade needs, so that it doesn't have to start a complete postmaster > environment just to have to turn off most of what postmaster does, and > with enough protections to disallow everyone else from connecting. I don't see anything that pg_upgrade is doing that causes the need to support a special mode. From other people's comments it's clear that "single user mode" is desirable to many and *will* be widely deployed if we allow it. I support the wish to allow a database server to be limited by configuration to a single user. However, supporting a specifically targeted mode that presents single user as an architectural design/limitation is a regressive step that I am strongly opposed to. The most popular relational database in the world is Microsoft Access, not MySQL. Access appears desirable because it allows a single user to create and use a database (which is very good). But all business databases have a requirement for at least one of: high availability, multi-user access or downstream processing in other parts of the business. Businesses worldwide curse the difficulties caused by having critical business data in desktop databases. And worldwide, there are also many that don't understand the problems that disconnected data causes because they can't see past the initial benefit. The lessons from that are that its OK to start with a database used by a single person, but that database soon needs to allow access from multiple users or automated agents. Many database systems support embedded or single user mode as an architectural option. All of those systems cause headaches in all of the businesses where they are used. They also cause problems on small detached devices such as phones, because even on very small systems there is a requirement for multiple concurrently active processes each of which may need database access. PostgreSQL was designed from the ground up as a multi-user database. This is the very fact that puts us in a good position to become pervasive. A single database system that works the same on all devices, with useful replication to connect data together. The embedded or single mode concept has long been on the "do not want" list. I believe that is a completely rational and strongly desirable thing. Supporting multiple architectures is extra work, and the restrictive architecture bites people in the long term. The fact that its an "easy patch" is not a great argument for changing that position, and in fact, its not easy, since it comes with a request to make it work on Windows (= extra work). The "easy" bit is not proven since people are already starting to ask about bgwriter and autovacuum. In this release there is much work happening around providing additional autonomous agents (bgworker) and other work around flexible replication (BDR), all of which would be nullified by the introduction and eventual wide usage of a restrictive new architecture. Single user configuration option, yes. Architecturally limited special version of PostgreSQL, no. -- Simon Riggs http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
Simon Riggs <simon@2ndquadrant.com> writes: > The most popular relational database in the world is Microsoft Access, > not MySQL. Access appears desirable because it allows a single user to > create and use a database (which is very good). But all business > databases have a requirement for at least one of: high availability, > multi-user access or downstream processing in other parts of the > business. That's a mighty sweeping claim, which you haven't offered adequate evidence for. The fact of the matter is that there is *lots* of demand for simple single-user databases, and what I'm proposing is at least a first step towards getting there. The main disadvantage of approaching this via the existing single-user mode is that you won't have any autovacuum, bgwriter, etc, support. But the flip side is that that lack of infrastructure is a positive advantage for certain admittedly narrow use-cases, such as disaster recovery and pg_upgrade. So while I agree that this isn't the only form of single-user mode that we'd like to support, I think it is *a* form we'd like to support, and I don't see why you appear to be against having it at all. A more reasonable objection would be that we need to make sure that this isn't foreclosing the option of having a multi-process environment with a single user connection. I don't see that it is, but it might be wise to sketch exactly how that case would work before accepting this. regards, tom lane
On Tue, Nov 13, 2012 at 12:38 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Simon Riggs <simon@2ndquadrant.com> writes: >> The most popular relational database in the world is Microsoft Access, >> not MySQL. Access appears desirable because it allows a single user to >> create and use a database (which is very good). But all business >> databases have a requirement for at least one of: high availability, >> multi-user access or downstream processing in other parts of the >> business. > > That's a mighty sweeping claim, which you haven't offered adequate > evidence for. The fact of the matter is that there is *lots* of demand > for simple single-user databases, and what I'm proposing is at least a > first step towards getting there. > > The main disadvantage of approaching this via the existing single-user > mode is that you won't have any autovacuum, bgwriter, etc, support. > But the flip side is that that lack of infrastructure is a positive > advantage for certain admittedly narrow use-cases, such as disaster > recovery and pg_upgrade. So while I agree that this isn't the only > form of single-user mode that we'd like to support, I think it is *a* > form we'd like to support, and I don't see why you appear to be against > having it at all. > > A more reasonable objection would be that we need to make sure that this > isn't foreclosing the option of having a multi-process environment with > a single user connection. I don't see that it is, but it might be wise > to sketch exactly how that case would work before accepting this. I'm not particularly excited about providing more single-user mode options, but I think it's worth having this particular thing because it makes pg_upgrade more robust. Whether we do anything else is something we can litigate when the time comes. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On 13 November 2012 17:38, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Simon Riggs <simon@2ndquadrant.com> writes: >> The most popular relational database in the world is Microsoft Access, >> not MySQL. Access appears desirable because it allows a single user to >> create and use a database (which is very good). But all business >> databases have a requirement for at least one of: high availability, >> multi-user access or downstream processing in other parts of the >> business. > > That's a mighty sweeping claim, which you haven't offered adequate > evidence for. The fact of the matter is that there is *lots* of demand > for simple single-user databases, and what I'm proposing is at least a > first step towards getting there. I agree there is lots of demand for simple single-user databases and I wish that too. What I don't agree with is something that casts that requirement in stone by architecturally/permanently disallowing secondary connections. Evidence for claims: * The whole Business Intelligence industry relies on being able to re-purpose existing data, forming integrated webs of interconnecting databases. All of that happens after the initial developers write the first version of the database application. * Everybody wants a remote backup, whether its for your mobile phone contact list or your enterprise datastore. People are migrating away from embedded databases in droves for these very reasons. > The main disadvantage of approaching this via the existing single-user > mode is that you won't have any autovacuum, bgwriter, etc, support. > But the flip side is that that lack of infrastructure is a positive > advantage for certain admittedly narrow use-cases, such as disaster > recovery and pg_upgrade. So while I agree that this isn't the only > form of single-user mode that we'd like to support, I think it is *a* > form we'd like to support, and I don't see why you appear to be against > having it at all. I have no problem with people turning things off, I reject the idea that we should encourage people to never be able to turn them back on. > A more reasonable objection would be that we need to make sure that this > isn't foreclosing the option of having a multi-process environment with > a single user connection. I don't see that it is, but it might be wise > to sketch exactly how that case would work before accepting this. Whatever we provide will become the norm. I don't have a problem with you providing BOTH the proposed single user mode AND the multi-process single user connection mode in this release. But if you provide just one of them and its the wrong one, we will be severely hampered in the future. Yes, I am very much against this project producing a new DBMS architecture that works on top of PostgreSQL data files, yet prevents maintenance, backup, replication and multi-user modes. I see this decision as a critical point for this project, so please consider this objection and where it comes from. -- Simon Riggs http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
Simon Riggs <simon@2ndQuadrant.com> writes: > On 13 November 2012 17:38, Tom Lane <tgl@sss.pgh.pa.us> wrote: >> ... The fact of the matter is that there is *lots* of demand >> for simple single-user databases, and what I'm proposing is at least a >> first step towards getting there. > I agree there is lots of demand for simple single-user databases and I > wish that too. What I don't agree with is something that casts that > requirement in stone by architecturally/permanently disallowing > secondary connections. If you want secondary connections, then I think you want a postmaster. We already have umpteen ways to limit who can connect (for example, putting the socket in a directory with limited access rights), and in that sort of situation I don't see why you'd really want a database that is only accessible when the "main" client is running. The case that this patch is meant to address is one where there is only one client application, period, and you'd rather that the database starts and stops automatically with that application instead of needing any management complexity. Now we can debate whether we want only one process or multiple processes underneath the client application, but I think the restriction to one client connection is a key *feature* not a bug, precisely because it removes a whole bunch of user-visible complexity that we cannot escape otherwise. > People are migrating away from embedded databases in droves for these > very reasons. [ shrug... ] If they don't want an embedded database, they won't want this either, but there are still plenty of people left who do want an embedded database. We've never had an adequate offering for those people before. If we ratchet up the management complexity of "single user" mode then it still won't be an adequate offering for them. > I see this decision as a critical point for this project, so please > consider this objection and where it comes from. I think this is nonsense. It's not critical; it's a very small patch that provides a feature of interest to a limited audience. And I don't believe it's foreclosing providing other operating modes later, unless maybe people feel this is "almost good enough" and lose motivation to work on those other operating modes. But if that happens, then I'd say the demand for the other modes isn't as high as you think. regards, tom lane
Preface: I think there's some great commentary here, and find myself agreeing
pretty whole-heartedly.
When confronted by a difficult problem, solve it by reducing it to the
question, "How would the Lone Ranger handle this?"
pretty whole-heartedly.
On Tue, Nov 13, 2012 at 2:45 PM, Simon Riggs <simon@2ndquadrant.com> wrote:
There seems to be a continuum of different sorts of scenarios of more-to-less
concurrency that are desirable for some different reasons. From most-to-least,
I can see:
1 - Obviously, there's the case that Postgres is eminently good at, of supporting
many users concurrently using a database. We love that, let's not break it :-).
2 - We have found it useful to have some extra work processes that do some
useful internal things, such as vacuuming, forcing background writes,
collecting statistics. And an online backup requires having a second process.
3 - People doing embedded systems find it attractive to attach all the data
to the singular user running the system. Witness the *heavy* deployment
of SQLite on Android and iOS. People make an assumption that this is
a single-process thing, but I am inclined to be a bit skeptical. What they
*do* know is that it's convenient to not spawn extra processes and do
IPC. That's not quite the same thing as it being a certainty that they
definitely want not to have more than one process.
4 - There are times when there *is* certainty about not wanting there to be more
than one process. When running pg_upgrade, or, at certain times, when
doing streaming replication node status switches, one might have that
certainty. Or when reindexing system tables, which needs single user mode.
For us to conflate the 3rd and 4th items seems like a mistake to me.
Yep. That seems like conflating #2 with #4.
It's mighty attractive to have a forcible "single process mode" to add safety to
certain activities.
I think we need a sharper knife, though, so we don't ablate off stuff like #2, just
because someone imagined that "Must Have Single Process!!!" was the right
doctrine.
I don't think we're necessarily *hugely* hampered by doing one of these;
I don't think it precludes doing the other.
Pointedly, I think it's a fine thing to have a way to force the system into
a single process to support #4 (e.g. - Single Process Mode to support
recovery from failover, pg_upgrade, and other pretty forcible SUM stuff).
That's useful to lots of people doing non-embedded cases.
That doesn't preclude allowing a somewhat more in-between case later.
Perhaps in-between is supported by slightly richer configuration:
- pg_hba.conf knows about replication connections; perhaps we could
augment it to have rules for the "internal daemons" like autovac, stats
collector, and background writer.
- pg_hba.conf might be augmented to forbid >1 connection per user,
for regular connections?
Possibly I'm not solving the true problem, of course.
-- On 13 November 2012 17:38, Tom Lane <tgl@sss.pgh.pa.us> wrote:I agree there is lots of demand for simple single-user databases and I
> Simon Riggs <simon@2ndquadrant.com> writes:
>> The most popular relational database in the world is Microsoft Access,
>> not MySQL. Access appears desirable because it allows a single user to
>> create and use a database (which is very good). But all business
>> databases have a requirement for at least one of: high availability,
>> multi-user access or downstream processing in other parts of the
>> business.
>
> That's a mighty sweeping claim, which you haven't offered adequate
> evidence for. The fact of the matter is that there is *lots* of demand
> for simple single-user databases, and what I'm proposing is at least a
> first step towards getting there.
wish that too. What I don't agree with is something that casts that
requirement in stone by architecturally/permanently disallowing
secondary connections.
Evidence for claims:
* The whole Business Intelligence industry relies on being able to
re-purpose existing data, forming integrated webs of interconnecting
databases. All of that happens after the initial developers write the
first version of the database application.
* Everybody wants a remote backup, whether its for your mobile phone
contact list or your enterprise datastore.
People are migrating away from embedded databases in droves for these
very reasons.
There seems to be a continuum of different sorts of scenarios of more-to-less
concurrency that are desirable for some different reasons. From most-to-least,
I can see:
1 - Obviously, there's the case that Postgres is eminently good at, of supporting
many users concurrently using a database. We love that, let's not break it :-).
2 - We have found it useful to have some extra work processes that do some
useful internal things, such as vacuuming, forcing background writes,
collecting statistics. And an online backup requires having a second process.
3 - People doing embedded systems find it attractive to attach all the data
to the singular user running the system. Witness the *heavy* deployment
of SQLite on Android and iOS. People make an assumption that this is
a single-process thing, but I am inclined to be a bit skeptical. What they
*do* know is that it's convenient to not spawn extra processes and do
IPC. That's not quite the same thing as it being a certainty that they
definitely want not to have more than one process.
4 - There are times when there *is* certainty about not wanting there to be more
than one process. When running pg_upgrade, or, at certain times, when
doing streaming replication node status switches, one might have that
certainty. Or when reindexing system tables, which needs single user mode.
For us to conflate the 3rd and 4th items seems like a mistake to me.
> The main disadvantage of approaching this via the existing single-userI have no problem with people turning things off, I reject the idea
> mode is that you won't have any autovacuum, bgwriter, etc, support.
> But the flip side is that that lack of infrastructure is a positive
> advantage for certain admittedly narrow use-cases, such as disaster
> recovery and pg_upgrade. So while I agree that this isn't the only
> form of single-user mode that we'd like to support, I think it is *a*
> form we'd like to support, and I don't see why you appear to be against
> having it at all.
that we should encourage people to never be able to turn them back on.
Yep. That seems like conflating #2 with #4.
It's mighty attractive to have a forcible "single process mode" to add safety to
certain activities.
I think we need a sharper knife, though, so we don't ablate off stuff like #2, just
because someone imagined that "Must Have Single Process!!!" was the right
doctrine.
> A more reasonable objection would be that we need to make sure that thisWhatever we provide will become the norm. I don't have a problem with
> isn't foreclosing the option of having a multi-process environment with
> a single user connection. I don't see that it is, but it might be wise
> to sketch exactly how that case would work before accepting this.
you providing BOTH the proposed single user mode AND the multi-process
single user connection mode in this release. But if you provide just
one of them and its the wrong one, we will be severely hampered in the
future.
Yes, I am very much against this project producing a new DBMS
architecture that works on top of PostgreSQL data files, yet prevents
maintenance, backup, replication and multi-user modes.
I see this decision as a critical point for this project, so please
consider this objection and where it comes from.
I don't think we're necessarily *hugely* hampered by doing one of these;
I don't think it precludes doing the other.
Pointedly, I think it's a fine thing to have a way to force the system into
a single process to support #4 (e.g. - Single Process Mode to support
recovery from failover, pg_upgrade, and other pretty forcible SUM stuff).
That's useful to lots of people doing non-embedded cases.
That doesn't preclude allowing a somewhat more in-between case later.
Perhaps in-between is supported by slightly richer configuration:
- pg_hba.conf knows about replication connections; perhaps we could
augment it to have rules for the "internal daemons" like autovac, stats
collector, and background writer.
- pg_hba.conf might be augmented to forbid >1 connection per user,
for regular connections?
Possibly I'm not solving the true problem, of course.
When confronted by a difficult problem, solve it by reducing it to the
question, "How would the Lone Ranger handle this?"
Tom Lane <tgl@sss.pgh.pa.us> writes: >> I agree there is lots of demand for simple single-user databases and I >> wish that too. What I don't agree with is something that casts that >> requirement in stone by architecturally/permanently disallowing >> secondary connections. > > If you want secondary connections, then I think you want a postmaster. I would agree. I think you're both talking above each other, and that what Simon is worried about (but I haven't asked him about that before sending that email) is how to change the application setup to switch from single user mode to multi user mode. IIRC the way to implement single user mode in your application is quite low-level with this patch, so switching to multi-user mode is not about just changing the connection string, or is it? > The case that this patch is meant to address is one where there is only > one client application, period, and you'd rather that the database > starts and stops automatically with that application instead of needing > any management complexity. Now we can debate whether we want only one > process or multiple processes underneath the client application, but > I think the restriction to one client connection is a key *feature* > not a bug, precisely because it removes a whole bunch of user-visible > complexity that we cannot escape otherwise. Well I think your patch would be easier to accept as is if it was documented only as a psql friendly single-user mode. I would really welcome that. > embedded database. We've never had an adequate offering for those > people before. If we ratchet up the management complexity of "single > user" mode then it still won't be an adequate offering for them. Now, if we're talking about single user mode as in embedded database, I really do think this patch should include a solution to run online maintainance, logical and physical backups, replication, archiving and all the production grade features you expect from PostgreSQL. And then I understand Simon's POV about code complexity and bgworkers for examples, which will *need* to be taken care of in that solution. > I think this is nonsense. It's not critical; it's a very small patch > that provides a feature of interest to a limited audience. And I don't Yes, it's providing full psql capabilities where we only had that bizare postgres --single interface. Maybe it will make initdb and debugging it easier too. > believe it's foreclosing providing other operating modes later, unless > maybe people feel this is "almost good enough" and lose motivation to > work on those other operating modes. But if that happens, then I'd say > the demand for the other modes isn't as high as you think. Again, my concern on that point after reading Simon's comments is only about the production procedure you have to follow to switch your application from single user mode to multi user mode. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
I have gone through the mail chain of this thread and tried to find the different concerns or open ends for this patch. Summarisation of the discussion and concerns for this patch: 1. Security concern in interface 2. Security concern in Windows implementation 3. Handling of Ctrl-C/SIGTERM 4. Secondary connections for maintenance activities, replication 5. Windows Implementation - what should be behaviour for admin users? 6. Restricting operation's in single backend mode 7. Proposal related to maintenance activities Description of each concern ---------------------------------------------- 1. Security concern in interface - Interface --------------- $ psql "standalone_datadir = $PGDATA dbname = regression" There is another option "standalone_backend", which can be set to specify which postgres executable to launch. If the latter isn't specified, libpq defaults to trying the installation PGBINDIR that was selected by configure. Security Concern ----------------------------- If a user can specify libpq connection options, he can now execute any file he wants by passing it as standalone_backend. Method to resolve Security concern -------------------------------------------------------- If an application wants to allow these connection parameters to be used, it would need to do PQenableStartServer() first. If it doesn't, those connection parameters will be rejected. 2. Security concern in Windows implementation - Interface --------------- PQcancel - In Unix, need to use kill(conn->postgres_pid, SIGINT) In Windows, pgkill(int pid, int sig) API can be used. Security concern --------------------------- pgkill is used to send cancel signal using pipe mechanism, so someone else can create a pipe with our name before we do (since we use the actual name - it's \\.\pipe\pgsinal_<pid>), by guessing what pid we will have. If that happens, we'll go into a loop and try to recreate it while logging a warning message to eventlog/stderr. (this happens for every backend). We can't throw an error on this and kill the backend because the pipe is created in the background thread not the main one. Some suggestions ------------------------------ Once it is detected that already a same Named Pipe already exists, there can be following options: a. try to create with some other name, but in that case how to communicate the new name to client end of pipe. Some solution can be thought if this approach seems to be reasonable, though currently I don't have any in mind. b. give error, as creation of pipe is generally at beginning of process creation(backend) c. any other better solution? 3. Handling of Ctrl-C/SIGTERM Behaviour --------------- If you kill the client, the child postgres will see connection closure and will shut down. Concern -------------- will make scripting harder because you cannot start another single backend pg_dump before the old backend noticed it, checkpointed and shut down. It can happen if you forcibly kill pg_dump (or some other client) and then immediately try to start a new one, it's not clear how long you'll have to wait. Suggestions for alternatives for this case ------------------------------------------------------------- a. There is no expectation that a standalone PG implementation would provide performance for a series of standalone sessions that is equivalent to what you'd get from a persistent server. If that scenario is what's important to you, you'd use a persistent server. b. An extra libpq call to handle this case can be helpful. 4. Secondary connections for data access Proposal --------------- A single-user connection database with "no administrative hassles" Concerns ----------------- As this proposal will not allow any data it stores to be accessed by another connection, so all forms of replication are excluded and all maintenance actions force the database to be unavailable for a period of time. Those two things are barriers of the most major kind to anybody working in an enterprise with connected data and devices. Suggestions for it's use or make it usable ---------------------------------------------------------------- a. a usable & scriptable --single mode is justification enough. Having to wait for hours just enter one more command because --single doesn't support any scripts sucks. Especially in recovery situations. b. it's worth having this particular thing because it makes pg_upgrade more robust. c. some competing solutions already provide similar solution (http://www.firebirdsql.org/manual/fbmetasecur-embedded.html). d. we need to make sure that this isn't foreclosing the option of having a multi-process environment with a single user connection. I don't see that it is, but it might be wise to sketch exactly how that case would work before accepting this. 5. Windows Implementation - what should be behaviour for admin users Behavior clarification - does this follow the behavior that admin users will not be allowed to invoke postgres child process? 6. Restricting operation's in single backend mode Serializable transactions could skip all the SSI predicate locking and conflict checking when in single-connection mode. With only one connection the transactions could never overlap, so there would be no chance of serialization anomalies when running snapshot isolation. It could be of use if someone had code they wanted to run under both normal and single-connection modes. For single-connection only, they could just choose REPEATABLE READ to get exactly the same semantics. 7. Proposal related to maintainence activities For maintainence activities, in longer run, we can have a postmaster process that isn't listening on any ports, but is managing background processes in addition to a single child backend. As per my understanding, to complete this patch we need to a. complete the work for #1, #2, #5 b. #6 and #7 can be done as enhancements after the initial feature is committed c. need to decide what should we do for #3 and #4. Rebased the patch (changes in patch) ----------------------------------------------------------- a. fillPGconn(), loops through each connection option, so no need to do it separately for standalone_datadir and standalone_backend b. In function, ChildPostgresMain()->PostgresMain() pass third parameter dbname as NULL. c. Changed second parameter of read_standalone_child_variables() from "int *" to "pgsocket *" to remove warning. d. removed trailing white spaces. e. update PQconninfoOptions array to include offset. Any objections for adding this idea/patch to CF? With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
Attachment
Amit Kapila <amit.kapila16@gmail.com> writes: > Any objections for adding this idea/patch to CF? You should certainly add it to the CF. You've listed lots of matters for review, but that's no reason to not get it in the queue to be reviewed. regards, tom lane
On 14 November 2013 03:41, Amit Kapila <amit.kapila16@gmail.com> wrote: > I have gone through the mail chain of this thread and tried to find > the different concerns or open ends for this patch. Not enough. This feature is clearly being suggested as a way to offer Postgres in embedded mode for users by a back door. Doing that forces us to turn off many of the server's features and we will take a huge step backwards in features, testing, maintainability of code and wasted community time. "No administrative hassles" is just a complete fiction. Admin will become a huge burden for any user in this mode, which will bite the community and cause us to waste much time redesigning the server to operate on a single session. -1 from me > 4. Secondary connections for data access > > Proposal > --------------- > A single-user connection database with "no administrative hassles" > > Concerns > ----------------- > As this proposal will not allow any data it stores to be accessed > by another connection, so all forms of replication are excluded and > all maintenance actions force the database to be > unavailable for a period of time. Those two things are barriers of > the most major kind to anybody working in an enterprise with connected > data and devices. > > Suggestions for it's use or make it usable > ---------------------------------------------------------------- > a. a usable & scriptable --single mode is justification enough. > Having to wait for hours just enter one more command because --single > doesn't support any scripts sucks. Especially in > recovery situations. > b. it's worth having this particular thing because it makes > pg_upgrade more robust. > c. some competing solutions already provide similar solution > (http://www.firebirdsql.org/manual/fbmetasecur-embedded.html). > d. we need to make sure that this isn't foreclosing the option of > having a multi-process environment with a single user connection. I > don't see that it is, but it might be wise to sketch > exactly how that case would work before accepting this. Why is not feasible to run a normal server with 1 connection. Are we really following what Firebird is doing? Why? > 6. Restricting operation's in single backend mode > > Serializable transactions could skip all the SSI predicate locking > and conflict checking when in single-connection mode. With only one > connection the transactions could never overlap, so > there would be no chance of serialization anomalies when running > snapshot isolation. > > It could be of use if someone had code they wanted to run under > both normal and single-connection modes. For single-connection only, > they could just choose REPEATABLE READ to > get exactly the same semantics. This is an example of my concern that we would begin optimising for the case of single user mode and encourage its use by users. This shows that the feature is not being suggested just for recovery. PostgreSQL has been designed from the ground up to support concurrency. If we begin optimising for single user mode it will take years to unpick our work and eventually we'll have a conflict and someone will say "we can't do that because it will be a problem in single user mode". > 7. Proposal related to maintainence activities > > For maintainence activities, in longer run, we can have a > postmaster process that isn't listening on any ports, but is managing > background processes in addition to a single child backend. > > As per my understanding, to complete this patch we need to > a. complete the work for #1, #2, #5 > b. #6 and #7 can be done as enhancements after the initial feature is committed > c. need to decide what should we do for #3 and #4. Huh? Multi-process mode already works. Why would do we need "a solution" for the problem that single process mode uses only one process? Don't use it. -- Simon Riggs http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
On 2013-11-15 09:51:28 -0200, Simon Riggs wrote: > On 14 November 2013 03:41, Amit Kapila <amit.kapila16@gmail.com> wrote: > > > I have gone through the mail chain of this thread and tried to find > > the different concerns or open ends for this patch. > > Not enough. This feature is clearly being suggested as a way to offer > Postgres in embedded mode for users by a back door. I think fixing single user mode to work halfway reasonable is enough justification for the feature. Having to deal with that when solving critical issues is just embarassing. > Doing that forces > us to turn off many of the server's features and we will take a huge > step backwards in features, testing, maintainability of code and > wasted community time. I think the patch as proposed actually reduces maintenance overhead since we don't have to deal with the strange separate codepaths for single user mode. But: I very, very much agree with the other concerns around this. This should be a patch to fix single user mode, not one to make postgres into a single process database. It's not, and trying to make it by using single user mode for it will start to hinder development of normal postgres because we suddenly need to be concerned about performance and features in situations where we previously weren't. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
Andres Freund <andres@2ndquadrant.com> writes: > I think fixing single user mode to work halfway reasonable is enough > justification for the feature. Having to deal with that when solving > critical issues is just embarassing. +1 > But: I very, very much agree with the other concerns around this. This > should be a patch to fix single user mode, not one to make postgres into > a single process database. It's not, and trying to make it by using > single user mode for it will start to hinder development of normal > postgres because we suddenly need to be concerned about performance and > features in situations where we previously weren't. +1 Maybe what needs to happen to this patch is away to restrict its usage to --single. I'm thinking that postgres --single maybe could be made to fork the server process underneath the psql controler client process transparently. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
On 15 November 2013 09:00, Andres Freund <andres@2ndquadrant.com> wrote: > This > should be a patch to fix single user mode, not one to make postgres into > a single process database. +1 -- Simon Riggs http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
On Fri, Nov 15, 2013 at 6:06 AM, Dimitri Fontaine <dimitri@2ndquadrant.fr> wrote: >> But: I very, very much agree with the other concerns around this. This >> should be a patch to fix single user mode, not one to make postgres into >> a single process database. It's not, and trying to make it by using >> single user mode for it will start to hinder development of normal >> postgres because we suddenly need to be concerned about performance and >> features in situations where we previously weren't. > > +1 > > Maybe what needs to happen to this patch is away to restrict its usage > to --single. I'm thinking that postgres --single maybe could be made to > fork the server process underneath the psql controler client process > transparently. I personally would prefer not to do that. My main non-administrative interest in this mode is doing things like benchmarking protocol overhead. I'm OK with not supporting (and optimizing) for single user code paths but I don't like the idea of building walls that serve no purpose other than to make it difficult for other people mess around. Just document strenuously that this mode is not intended for application bundling and move on... merlin
On Fri, Nov 15, 2013 at 5:21 PM, Simon Riggs <simon@2ndquadrant.com> wrote: > On 14 November 2013 03:41, Amit Kapila <amit.kapila16@gmail.com> wrote: > >> I have gone through the mail chain of this thread and tried to find >> the different concerns or open ends for this patch. > > Not enough. This feature is clearly being suggested as a way to offer > Postgres in embedded mode for users by a back door. Current patch doesn't have such facility and I don't think somebody can use it as an embedded database. > Doing that forces > us to turn off many of the server's features and we will take a huge > step backwards in features, testing, maintainability of code and > wasted community time. > > "No administrative hassles" is just a complete fiction. Admin will > become a huge burden for any user in this mode, which will bite the > community and cause us to waste much time redesigning the server to > operate on a single session. > > -1 from me What I could understand from your objection is that you don't want users to get the impression of this feature as an embedded database. I think as the patch stands, it doesn't have such facility, so advertising it as an substitute for embedded database would be anyway inappropriate. The use case is to provide a standalone mode which will be useful for cases where today --single mode is required/used and I think documenting the feature that way is the right way to proceed. If this addresses your concern, then we can proceed to discuss solutions for other concerns like security? With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
On Fri, Nov 15, 2013 at 6:51 AM, Simon Riggs <simon@2ndquadrant.com> wrote: > Not enough. This feature is clearly being suggested as a way to offer > Postgres in embedded mode for users by a back door. Doing that forces > us to turn off many of the server's features and we will take a huge > step backwards in features, testing, maintainability of code and > wasted community time. That's not clear to me at all. IIRC, the original idea was Tom's, and the idea is to make it possible to have, for example, a psql session connected to a standalone database, which can't be done right now. I don't use standalone mode much, but when I do, I'd sure like to have the psql interface rather than the existing standalone mode interface.I'm not aware that there's anything in this patch whichtargets any other use case; if there is, sure, rip it out. But let's not assume this is going in a bad direction, especially considering who it was that suggested the idea originally. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes: > On Fri, Nov 15, 2013 at 6:51 AM, Simon Riggs <simon@2ndquadrant.com> wrote: >> Not enough. This feature is clearly being suggested as a way to offer >> Postgres in embedded mode for users by a back door. Doing that forces >> us to turn off many of the server's features and we will take a huge >> step backwards in features, testing, maintainability of code and >> wasted community time. > That's not clear to me at all. IIRC, the original idea was Tom's, and > the idea is to make it possible to have, for example, a psql session > connected to a standalone database, which can't be done right now. I > don't use standalone mode much, but when I do, I'd sure like to have > the psql interface rather than the existing standalone mode interface. pg_dump from a standalone backend seems like another core use case. That's not just more convenience, it's functionality you just don't have right now. I think that it might someday be interesting to allow a full server to be started on-demand in this way. But the patch as proposed is not that, it's just a nicer interface to the existing standalone mode. regards, tom lane
On 11/14/13, 1:41 AM, Amit Kapila wrote: > Security Concern > ----------------------------- > If a user can specify libpq connection options, he can now execute > any file he wants by passing it as standalone_backend. > > Method to resolve Security concern > -------------------------------------------------------- > If an application wants to allow these connection parameters to be > used, it would need to do PQenableStartServer() first. If it doesn't, > those connection parameters will be rejected. I don't think this really helps. You can't tell with reasonable effort or certainty whether a given program is calling PQenableStartServer(), so you cannot audit this from the outside. Also, someone could, depending on circumstances, dynamically load a module that calls PQenableStartServer(), thus circumventing this check. And maybe before long someone will patch up all drivers to call PQenableStartServer() automatically, because why shouldn't I be able to run a standalone backend from PHP or Ruby? Also, at some point at least, something like phpPgAdmin called pg_dump internally, so you could imagine that in situations like that, assuming that pg_dump called PQenableStartServer(), with a little bit craftiness, you could expose the execute-any-file hole through a web server. I don't have a better idea right now how to set up these connection parameters in a way that you can only set them in certain "safe" circumstances. I would consider sidestepping this entire issue by having the stand-alone backend create a Unix-domain socket and have a client connect to that in the normal way. At least if you split the patch that way, you might alleviate some concerns of others about whether this patch is about fixing standalone mode vs. allowing using standalone mode with psql vs. making a fully embedded database.
Peter Eisentraut <peter_e@gmx.net> writes: > I would consider sidestepping this entire issue by having the > stand-alone backend create a Unix-domain socket and have a client > connect to that in the normal way. Hmm. But that requires the "stand-alone backend" to take on at least some properties of a postmaster; at the very least, it would need to accept some form of shutdown signal (not just EOF on its stdin). Perhaps more to the point, I think this approach actually breaks one of the principal good-thing-in-emergencies attributes of standalone mode, namely being sure that nobody but you can connect. With this, you're right back to having a race condition as to whether your psql command gets to the socket before somebody else. I think we'd be better off trying to fix the security issue by constraining what can be executed as a "standalone backend". Would it work to insist that psql/pg_dump launch the program named postgres from the same bin directory they're in, rather than accepting a path from the connection string? regards, tom lane
On 2013-11-20 10:48:20 -0500, Tom Lane wrote: > constraining what can be executed as a "standalone backend". Would > it work to insist that psql/pg_dump launch the program named postgres > from the same bin directory they're in, rather than accepting a path > from the connection string? But why do we want to start the server through the connection string using PQconnectb() in the first place? That doesn't really seem right to me. Something like PQstartSingleUser(dsn) returning a established connection seems better to me. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
Andres Freund <andres@2ndquadrant.com> writes: > On 2013-11-20 10:48:20 -0500, Tom Lane wrote: >> constraining what can be executed as a "standalone backend". Would >> it work to insist that psql/pg_dump launch the program named postgres >> from the same bin directory they're in, rather than accepting a path >> from the connection string? > But why do we want to start the server through the connection string > using PQconnectb() in the first place? That doesn't really seem right to > me. > Something like PQstartSingleUser(dsn) returning a established connection > seems better to me. That just pushes the problem up a level --- how are you going to tell psql, pg_dump, or other programs that they should do that? regards, tom lane
On 2013-11-20 11:08:33 -0500, Tom Lane wrote: > Andres Freund <andres@2ndquadrant.com> writes: > > On 2013-11-20 10:48:20 -0500, Tom Lane wrote: > >> constraining what can be executed as a "standalone backend". Would > >> it work to insist that psql/pg_dump launch the program named postgres > >> from the same bin directory they're in, rather than accepting a path > >> from the connection string? > > > But why do we want to start the server through the connection string > > using PQconnectb() in the first place? That doesn't really seem right to > > me. > > Something like PQstartSingleUser(dsn) returning a established connection > > seems better to me. > > That just pushes the problem up a level --- how are you going to tell > psql, pg_dump, or other programs that they should do that? An explicit parameter. A program imo explicitly needs to be aware that a PQconnect() suddenly starts forking and such. What if it is using threads? What if it has it's own SIGCHLD handler for other business it's doing? Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
* Tom Lane (tgl@sss.pgh.pa.us) wrote: > I think we'd be better off trying to fix the security issue by > constraining what can be executed as a "standalone backend". Would > it work to insist that psql/pg_dump launch the program named postgres > from the same bin directory they're in, rather than accepting a path > from the connection string? Couldn't that be an issue for people who have multiple major versions of binaries installed? In particular, the "default" on the system for psql might be 9.3 while the cluster you're trying to recover may be 9.2. Of course, in that case you might say to use the 9.2 psql, which would be fair, but what if you're looking to get the data out of the 9.2 DB and into the 9.3? In that case, we'd recommend using the 9.3 pg_dump. Basically, I'd suggest that we try and avoid things like "the binaries have to be in the same directory".. With regard to access to the socket, perhaps we create our own socket w/ 0600 and use that? Seems like it'd be sufficient to prevent the 'normal' users from getting into the DB while we're working on it. If there's two different individuals gettings into the same system and trying to start the same cluster as the same unix user, well.. I'm not convinced we'd be able to come up with a perfect solution to that anyway. Thanks, Stephen
On 2013-11-20 17:19:42 +0100, Andres Freund wrote: > > That just pushes the problem up a level --- how are you going to tell > > psql, pg_dump, or other programs that they should do that? > > An explicit parameter. A program imo explicitly needs to be aware that a > PQconnect() suddenly starts forking and such. What if it is using > threads? What if it has it's own SIGCHLD handler for other business it's > doing? Just as an example, consider what happens if somebody does pg_dump -j? Or somebody specifies such a connection for primary_conninfo? I am also not sure whether vacuumdb -a/reindexdb -a (both not unlikely commands to use for single user mode) are careful enough not to have parallel connections open? Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
Andres Freund <andres@2ndquadrant.com> writes: > On 2013-11-20 11:08:33 -0500, Tom Lane wrote: >> Andres Freund <andres@2ndquadrant.com> writes: >>> Something like PQstartSingleUser(dsn) returning a established connection >>> seems better to me. >> That just pushes the problem up a level --- how are you going to tell >> psql, pg_dump, or other programs that they should do that? > An explicit parameter. A program imo explicitly needs to be aware that a > PQconnect() suddenly starts forking and such. What if it is using > threads? What if it has it's own SIGCHLD handler for other business it's > doing? Hm. That's a fair point. I don't especially buy your other argument about additional connections --- if the program tries such, they'll just fail, which can hardly be said to be unexpected. But it's reasonable to worry that programs might need to be aware that they now have a child process. (It occurs to me that we'll need to provide a way to get the PID of the child, too.) regards, tom lane
On 11/20/13, 11:31 AM, Stephen Frost wrote: > Couldn't that be an issue for people who have multiple major versions of > binaries installed? In particular, the "default" on the system for psql > might be 9.3 while the cluster you're trying to recover may be 9.2. Of > course, in that case you might say to use the 9.2 psql, which would be > fair, but what if you're looking to get the data out of the 9.2 DB and > into the 9.3? In that case, we'd recommend using the 9.3 pg_dump. Right. And also, in emergency situations you might have a custom built postgres binary lying around in a separate path that includes a patch from a mailing list you're supposed to test or something. Best not to make that even more difficult.
On 11/20/13, 10:48 AM, Tom Lane wrote: > Perhaps more to the point, I think this approach actually breaks one of > the principal good-thing-in-emergencies attributes of standalone mode, > namely being sure that nobody but you can connect. With this, you're > right back to having a race condition as to whether your psql command > gets to the socket before somebody else. I don't disagree, except maybe about the relative gravity of the various competing concerns. But I want to see if we can split the proposed patch into smaller, more acceptable parts. There is elegance in being able to start a standalone backend from libpq connection parameters. But there are also security concerns and some general concerns about promoting an embedded database mode. If we allow single-user backends to speak protocol over sockets, then we have at least solved the problem of being able to use standard tools in emergency mode. And I don't think it precludes adding some of the other functionality later.
On Wed, Nov 20, 2013 at 10:13 AM, Peter Eisentraut <peter_e@gmx.net> wrote: > On 11/14/13, 1:41 AM, Amit Kapila wrote: >> Security Concern >> ----------------------------- >> If a user can specify libpq connection options, he can now execute >> any file he wants by passing it as standalone_backend. >> >> Method to resolve Security concern >> -------------------------------------------------------- >> If an application wants to allow these connection parameters to be >> used, it would need to do PQenableStartServer() first. If it doesn't, >> those connection parameters will be rejected. > > I don't think this really helps. You can't tell with reasonable effort > or certainty whether a given program is calling PQenableStartServer(), > so you cannot audit this from the outside. Also, someone could, > depending on circumstances, dynamically load a module that calls > PQenableStartServer(), thus circumventing this check. What?! The complaint is that somebody who only has access to set connection parameters could cause a server to be started. There's a tremendous gulf between "I can set the connection string" and "I can set LD_PRELOAD". If you can set LD_PRELOAD to a value of your choice, I'm pretty sure you can do things that are far more entertaining than calling a hypothetical PQenableStartServer() function. > And maybe before > long someone will patch up all drivers to call PQenableStartServer() > automatically, because why shouldn't I be able to run a standalone > backend from PHP or Ruby? Also, at some point at least, something like > phpPgAdmin called pg_dump internally, so you could imagine that in > situations like that, assuming that pg_dump called > PQenableStartServer(), with a little bit craftiness, you could expose > the execute-any-file hole through a web server. The point is that client applications should expose whether or not to set this function as a command-line switch separate from whatever they accept in terms of connection strings. So pg_dump should have a flag called --standalone-server or something like, and it should all PQenableStartServer() only when that flag is used. So if the user has a shell script that invokes pg_dump -d "$1", the user cannot contrive a server. If they write the script as pg_dump --standalone-server -d "$1", then they can, but by putting that option in there you pretty much bought the farm. Any program that calls that function unconditionally while at the same time accepting untrusted user input will be insecure, but chmod -R u+s /bin is insecure, too. That's why we don't do that. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On 11/20/13, 3:24 PM, Robert Haas wrote: > The point is that client applications should expose whether or not to > set this function as a command-line switch separate from whatever they > accept in terms of connection strings. So pg_dump should have a flag > called --standalone-server or something like, and it should all > PQenableStartServer() only when that flag is used. The argument elsewhere in this thread was that the reason for putting this in the connection options was so that you do *not* have to patch up every client to be able to use this functionality. If you have to add separate options everywhere, then you might as well just have a separate libpq function to initiate the session.
On Wed, Nov 20, 2013 at 3:32 PM, Peter Eisentraut <peter_e@gmx.net> wrote: > On 11/20/13, 3:24 PM, Robert Haas wrote: >> The point is that client applications should expose whether or not to >> set this function as a command-line switch separate from whatever they >> accept in terms of connection strings. So pg_dump should have a flag >> called --standalone-server or something like, and it should all >> PQenableStartServer() only when that flag is used. > > The argument elsewhere in this thread was that the reason for putting > this in the connection options was so that you do *not* have to patch up > every client to be able to use this functionality. If you have to add > separate options everywhere, then you might as well just have a separate > libpq function to initiate the session. Well, that's fair enough. I don't care much what the syntax is for invoking the postmaster this way, as long as it's reasonably convenient. I just want there to be one. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Peter Eisentraut <peter_e@gmx.net> writes: > The argument elsewhere in this thread was that the reason for putting > this in the connection options was so that you do *not* have to patch up > every client to be able to use this functionality. If you have to add > separate options everywhere, then you might as well just have a separate > libpq function to initiate the session. Right, Andres was saying that we had to do both (special switches that lead to calling a special connection function). I'm not terribly happy about that, because it will greatly constrain the set of programs that are able to connect to standalone backends --- but I think that there are some in this discussion who want that, anyway. In practice, as long as psql and pg_dump and pg_upgrade can do it, I think we've covered most of the interesting bases. To my mind, the "create a socket and hope nobody else can get to it" approach is exactly one of the main things we're trying to avoid here. If you'll recall, awhile back we had a big discussion about how pg_upgrade could positively guarantee that nobody messed with the source database while it was working, and we still don't have a bulletproof guarantee there. I would like to fix that by making pg_upgrade use only standalone backends to talk to the source database, never starting a real postmaster at all. But if the standalone-pg_dump mode goes through a socket, we're back to square one on that concern. regards, tom lane
On Wed, Nov 20, 2013 at 3:44 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
To my mind, the "create a socket and hope nobody else can get to it"
approach is exactly one of the main things we're trying to avoid here.
If you'll recall, awhile back we had a big discussion about how pg_upgrade
could positively guarantee that nobody messed with the source database
while it was working, and we still don't have a bulletproof guarantee
there. I would like to fix that by making pg_upgrade use only standalone
backends to talk to the source database, never starting a real postmaster
at all. But if the standalone-pg_dump mode goes through a socket, we're
back to square one on that concern.
(I couldn't find the pg_upgrade-related thread mentioned above).
I am not sure of the mechanics of this, but can we not launch the postmaster with a random magic-cookie, and use that cookie while initiating the connection from libpq. The postmaster will then reject any connections that don't provide the cookie.
We do something similar to enable applications to send cancellation signals (postmaster.c:Backend.cancel_key), just that it's establishing trust in the opposite direction.
Best regards,
--
On 2013-11-20 15:44:03 -0500, Tom Lane wrote: > In practice, as long as psql and pg_dump and pg_upgrade can do it, I > think we've covered most of the interesting bases. I'd say vacuumdb/reindexdb should be added to that list. In my experience xid wraparound and corrupted system indexes are the most frequent use-case of single user mode. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
On Wed, Nov 20, 2013 at 05:38:14PM -0500, Gurjeet Singh wrote: > On Wed, Nov 20, 2013 at 3:44 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > > > To my mind, the "create a socket and hope nobody else can get to it" > approach is exactly one of the main things we're trying to avoid here. > If you'll recall, awhile back we had a big discussion about how pg_upgrade > could positively guarantee that nobody messed with the source database > while it was working, and we still don't have a bulletproof guarantee > there. I would like to fix that by making pg_upgrade use only standalone > backends to talk to the source database, never starting a real postmaster > at all. But if the standalone-pg_dump mode goes through a socket, we're > back to square one on that concern. > > > (I couldn't find the pg_upgrade-related thread mentioned above). > > I am not sure of the mechanics of this, but can we not launch the postmaster > with a random magic-cookie, and use that cookie while initiating the connection > from libpq. The postmaster will then reject any connections that don't provide > the cookie. > > We do something similar to enable applications to send cancellation signals > (postmaster.c:Backend.cancel_key), just that it's establishing trust in the > opposite direction. The magic cookie can be tha application_name. I had pg_upgrade code to prevent anyone from connecting unless their application_name was "pg_upgrade", but the idea was rejected. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. +
On Thu, Nov 21, 2013 at 2:14 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Peter Eisentraut <peter_e@gmx.net> writes: >> The argument elsewhere in this thread was that the reason for putting >> this in the connection options was so that you do *not* have to patch up >> every client to be able to use this functionality. If you have to add >> separate options everywhere, then you might as well just have a separate >> libpq function to initiate the session. > > Right, Andres was saying that we had to do both (special switches that > lead to calling a special connection function). Doesn't the new option 'standalone_datadir' (which is already in patch) a good candidate for special switch? How does having one more new switch helps better? With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
On Thu, Nov 21, 2013 at 9:11 AM, Amit Kapila <amit.kapila16@gmail.com> wrote: > On Thu, Nov 21, 2013 at 2:14 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >> Peter Eisentraut <peter_e@gmx.net> writes: >>> The argument elsewhere in this thread was that the reason for putting >>> this in the connection options was so that you do *not* have to patch up >>> every client to be able to use this functionality. If you have to add >>> separate options everywhere, then you might as well just have a separate >>> libpq function to initiate the session. >> >> Right, Andres was saying that we had to do both (special switches that >> lead to calling a special connection function). > > Doesn't the new option 'standalone_datadir' (which is already in > patch) a good candidate for special switch? > How does having one more new switch helps better? Here what I have in mind is that: a. In pg_dump or other internal utilities where we want to use this feature, they should call PQenableStart() or some other API before calling PQConnect() which will indicate that it wantsto operate as a standalone mode. b. In psql, if user specifies this special switch ( 'standalone_datadir'), then internally we will call PQenableStart() and use postgres from same directory. So standalone_backend option will not be exposed through psql, but other internal tools can use it. With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
Amit Kapila <amit.kapila16@gmail.com> writes: > Here what I have in mind is that: > a. In pg_dump or other internal utilities where we want to use this > feature, they should call PQenableStart() or some other API before > calling PQConnect() which will indicate that it wants to operate > as a standalone mode. > b. In psql, if user specifies this special switch ( > 'standalone_datadir'), then internally we will call PQenableStart() > and use postgres from same > directory. Why would you make psql behave differently from our other command-line clients? That seems bizarre. If we're going to use a special switch to enable standalone mode, it should be the same on every program that supports it. regards, tom lane
On Thu, Nov 21, 2013 at 8:14 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Amit Kapila <amit.kapila16@gmail.com> writes: >> Here what I have in mind is that: >> a. In pg_dump or other internal utilities where we want to use this >> feature, they should call PQenableStart() or some other API before >> calling PQConnect() which will indicate that it wants to operate >> as a standalone mode. >> b. In psql, if user specifies this special switch ( >> 'standalone_datadir'), then internally we will call PQenableStart() >> and use postgres from same >> directory. > > Why would you make psql behave differently from our other command-line > clients? No, psql should not behave different from other clients. Sorry, I was under assumption that for other programs we will not take backend executable path. One other thing which is not clear to me is that how by calling some special/new API we can ensure that the path provided by user is a valid path, are we going to validate file given in 'standalone_backend' switch in some way? With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
On Thu, Nov 21, 2013 at 9:54 PM, Amit Kapila <amit.kapila16@gmail.com> wrote: > On Thu, Nov 21, 2013 at 8:14 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >> Amit Kapila <amit.kapila16@gmail.com> writes: >>> Here what I have in mind is that: >> >> Why would you make psql behave differently from our other command-line >> clients? > > No, psql should not behave different from other clients. Sorry, I > was under assumption that for other programs we will not take backend > executable > path. > One other thing which is not clear to me is that how by calling > some special/new API we can ensure that the path provided by user is > a valid path, are we going to validate file given in > 'standalone_backend' switch in some way? Here, do we mean that if user specifies special switch, then psql/pg_dump will call new API (eg. PQstartSingleUser(dsn)) which will use postgres from same bin directory they are in, rather than using from standalone_backend. Can we consider this as a way to proceed for this patch? On a side note, today while reading what other software's does to protect them from such a security threat, I came across this link (http://osxbook.com/book/bonus/chapter7/binaryprotection/index.html) which suggests to have encrypted binaries. The use for encrypted binaries is somewhat similar to what we discussed as a security threat in this mail thread. Some text from link which made me think that this is relevant. For example, "one could turn the requirement around and say that a given system must not run any binaries unless they are from a certain source (or set of sources). This could be used to create an admission-control mechanism for executables, which in turn could be used in defending against malware. In a draconian managed environment, it might be desired to limit program execution on managed systems to a predefined set of programs—nothing else will execute." With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
On Thu, 2013-11-14 at 12:11 +0530, Amit Kapila wrote: > If an application wants to allow these connection parameters to be > used, it would need to do PQenableStartServer() first. If it doesn't, > those connection parameters will be rejected. Stupid idea: Would it work that we require an environment variable to be set before we allow the standalone_backend connection parameter? That's easy to do, easy to audit, and doesn't require any extra code in the individual clients.
On Thu, Dec 5, 2013 at 7:25 AM, Peter Eisentraut <peter_e@gmx.net> wrote: > On Thu, 2013-11-14 at 12:11 +0530, Amit Kapila wrote: >> If an application wants to allow these connection parameters to be >> used, it would need to do PQenableStartServer() first. If it doesn't, >> those connection parameters will be rejected. > > Stupid idea: Would it work that we require an environment variable to be > set before we allow the standalone_backend connection parameter? That's > easy to do, easy to audit, and doesn't require any extra code in the > individual clients. This is certainly not a stupid idea, rather something on similar lines has been discussed previously in this thread. Tom has suggested something similar, but I am not sure if there was a conclusion on that point. Please see the relavant discussion at below link: http://www.postgresql.org/message-id/17384.1346645480@sss.pgh.pa.us I think the basic question at that time was why should we consider an environment variable more safe. With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
On Thu, 2013-12-05 at 09:02 +0530, Amit Kapila wrote: > This is certainly not a stupid idea, rather something on similar lines > has been discussed previously in this thread. > Tom has suggested something similar, but I am not sure if there was a > conclusion on that point. Please see the > relavant discussion at below link: > http://www.postgresql.org/message-id/17384.1346645480@sss.pgh.pa.us Yeah, I think the environment variable idea wasn't actually refuted there.
On 5 December 2013 01:55, Peter Eisentraut <peter_e@gmx.net> wrote: > On Thu, 2013-11-14 at 12:11 +0530, Amit Kapila wrote: >> If an application wants to allow these connection parameters to be >> used, it would need to do PQenableStartServer() first. If it doesn't, >> those connection parameters will be rejected. > > Stupid idea: Would it work that we require an environment variable to be > set before we allow the standalone_backend connection parameter? That's > easy to do, easy to audit, and doesn't require any extra code in the > individual clients. I like the idea... should it be in pg_hba.conf ? Or should it be next to listen_addresses in postgresql.conf? hba might be less convenient but seems like the correct place -- Simon Riggs http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
On 2013-12-04 20:55:08 -0500, Peter Eisentraut wrote: > On Thu, 2013-11-14 at 12:11 +0530, Amit Kapila wrote: > > If an application wants to allow these connection parameters to be > > used, it would need to do PQenableStartServer() first. If it doesn't, > > those connection parameters will be rejected. > > Stupid idea: Would it work that we require an environment variable to be > set before we allow the standalone_backend connection parameter? That's > easy to do, easy to audit, and doesn't require any extra code in the > individual clients. I still don't think it's ok to start forking in arbitrary applications without their knowledge. So I don't think that buys us enough. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
On 12/5/13, 6:07 AM, Simon Riggs wrote: > On 5 December 2013 01:55, Peter Eisentraut <peter_e@gmx.net> wrote: >> On Thu, 2013-11-14 at 12:11 +0530, Amit Kapila wrote: >>> If an application wants to allow these connection parameters to be >>> used, it would need to do PQenableStartServer() first. If it doesn't, >>> those connection parameters will be rejected. >> >> Stupid idea: Would it work that we require an environment variable to be >> set before we allow the standalone_backend connection parameter? That's >> easy to do, easy to audit, and doesn't require any extra code in the >> individual clients. > > I like the idea... should it be in pg_hba.conf ? > Or should it be next to listen_addresses in postgresql.conf? No, it's an environment variable.
I think this proposal is a bit deadlocked now. - There are technical concerns about launching a server executable from within a client. - There are conceptual concerns about promoting an embedded database mode. On the other hand: - Everyone would like to have a way to use psql (and other basic clients) in stand-alone mode. The compromise would be to not launch the server from within the client, but have client and server communicate over external mechanisms (e.g., Unix-domain socket). The concern about that was that it would open up standalone mode to accidental third-party connections. While there are some ways around that (socket in private directory), they are not easy and not portable.So standalone mode would became less robust and reliableoverall. The only solutions I see are: 1. do nothing 2. do everything (i.e., existing terminal mode plus socket mode plus embedded mode), letting the user work out the differences Pick one. ;-)
On 2013-12-05 11:39:29 -0500, Peter Eisentraut wrote: > I think this proposal is a bit deadlocked now. > > - There are technical concerns about launching a server executable from > within a client. > > - There are conceptual concerns about promoting an embedded database mode. > > On the other hand: > > - Everyone would like to have a way to use psql (and other basic > clients) in stand-alone mode. > The only solutions I see are: > > 1. do nothing > > 2. do everything (i.e., existing terminal mode plus socket mode plus > embedded mode), letting the user work out the differences > > Pick one. ;-) 3) make it an explicit parameter, outside the database DSN, and let the clients contain a tiny bit of explict code aboutit. There really aren't that many clients that can use such a mode sensibly. If we ever want to support a real embedded mode, much, much more than this is needed. I don't think we should let that stop us from improving single user mode. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
On Thu, Dec 5, 2013 at 11:52 AM, Andres Freund <andres@2ndquadrant.com> wrote: > On 2013-12-05 11:39:29 -0500, Peter Eisentraut wrote: >> I think this proposal is a bit deadlocked now. >> >> - There are technical concerns about launching a server executable from >> within a client. >> >> - There are conceptual concerns about promoting an embedded database mode. >> >> On the other hand: >> >> - Everyone would like to have a way to use psql (and other basic >> clients) in stand-alone mode. >> The only solutions I see are: >> >> 1. do nothing >> >> 2. do everything (i.e., existing terminal mode plus socket mode plus >> embedded mode), letting the user work out the differences >> >> Pick one. ;-) > > 3) make it an explicit parameter, outside the database DSN, and let the > clients contain a tiny bit of explict code about it. There really > aren't that many clients that can use such a mode sensibly. > > If we ever want to support a real embedded mode, much, much more than > this is needed. I don't think we should let that stop us from improving > single user mode. Yeah, seriously. I don't understand what the big deal is here. The right design here is 99.44% clear here, and the committer (presumably Tom) can handle the other 0.56% however he'd like. Let's do this and move on. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes: > Yeah, seriously. I don't understand what the big deal is here. The > right design here is 99.44% clear here, and the committer (presumably > Tom) can handle the other 0.56% however he'd like. Let's do this and > move on. Yeah, but the remaining 0.56% is an important decision, not least because it's got security implications. I think we need some consensus not just a unilateral committer decision. I'm pretty much persuaded by Andres' point that we should not allow a child process to be launched under a client app without clear permission from the code of the app (and *not* just some environment variable that might have been set far away, perhaps by someone who doesn't know what the app assumes about SIGCHLD etc). So a separate connection call seems like not a bad idea. In the case of psql and pg_dump it'd be reasonable to invent a separate command line switch that drives use of this call instead of normal PQconnect. Doing that, and *not* allowing the text of the connection string to determine it, seems like it pretty well solves any security objections. It might be unpleasant to use in some cases, though. Another issue is that we have too many variants of PQconnect already; which of them are we prepared to clone for this hypothetical new connection method? regards, tom lane
On Thu, Dec 5, 2013 at 3:05 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > I'm pretty much persuaded by Andres' point that we should not allow a > child process to be launched under a client app without clear permission > from the code of the app (and *not* just some environment variable that > might have been set far away, perhaps by someone who doesn't know what the > app assumes about SIGCHLD etc). So a separate connection call seems like > not a bad idea. In the case of psql and pg_dump it'd be reasonable to > invent a separate command line switch that drives use of this call instead > of normal PQconnect. Doing that, and *not* allowing the text of the > connection string to determine it, seems like it pretty well solves any > security objections. Yep. > It might be unpleasant to use in some cases, though. Why would there be more than a few cases in the first place? Who is going to use this beyond psql, pg_dump(all), and pg_upgrade, and why? > Another issue is that we have too many variants of PQconnect already; > which of them are we prepared to clone for this hypothetical new > connection method? PQconnectdbParams, I assume. Isn't that the one to rule them all, modulo async connect which I can't think is relevant here? Or don't clone that one but instead have PQnextConnectionShouldForkThisBinary('...') and let the psql/pg_dump switch be --standalone=full-path-to-the-postgres-binary. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On 12/05/2013 10:37 PM, Robert Haas wrote: > On Thu, Dec 5, 2013 at 3:05 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >> It might be unpleasant to use in some cases, though. > > Why would there be more than a few cases in the first place? Who is > going to use this beyond psql, pg_dump(all), and pg_upgrade, and why? Well, you might want to use pgAdmin, or your other favorite admin tool. I'm not sure how well it would work, and I think it's OK if we say "sorry, can't do that", but it's not a crazy thing to want. >> Another issue is that we have too many variants of PQconnect already; >> which of them are we prepared to clone for this hypothetical new >> connection method? > > PQconnectdbParams, I assume. Isn't that the one to rule them all, > modulo async connect which I can't think is relevant here? Right. Not all of the parameters will make sense for a stand-alone backend though, like the hostname and port number. And I think you need need a new parameter to pass the path to the 'postgres' executable, unless we re-use the host parameter for that. > Or don't clone that one but instead have > PQnextConnectionShouldForkThisBinary('...') and let the psql/pg_dump > switch be --standalone=full-path-to-the-postgres-binary. I think a separate function makes more sense. - Heikki
On 2013-12-05 23:01:28 +0200, Heikki Linnakangas wrote: > On 12/05/2013 10:37 PM, Robert Haas wrote: > >On Thu, Dec 5, 2013 at 3:05 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > >>It might be unpleasant to use in some cases, though. > > > >Why would there be more than a few cases in the first place? Who is > >going to use this beyond psql, pg_dump(all), and pg_upgrade, and why? > > Well, you might want to use pgAdmin, or your other favorite admin tool. I'm > not sure how well it would work, and I think it's OK if we say "sorry, can't > do that", but it's not a crazy thing to want. Pgadmin wouldn't work, it uses multiple connections for anything but the most trivial tasks. You can't even send a manual sql query using only one connection. I think that's true for most of the non-trivial tools. > >>Another issue is that we have too many variants of PQconnect > >>already; which of them are we prepared to clone for this > >>hypothetical new connection method? > > > >PQconnectdbParams, I assume. Isn't that the one to rule them all, > >modulo async connect which I can't think is relevant here? > Right. Not all of the parameters will make sense for a stand-alone backend > though, like the hostname and port number. And I think you need need a new > parameter to pass the path to the 'postgres' executable, unless we re-use > the host parameter for that. Hm. I'd guessed that we wouldn't use the connection string to pass down the executable name and the datadir now that we're inventing a separate function. But maybe that's unneccessary. What parameters do we require to be set for that mode: * path to postgres * data directory * database name (single mode after all) * port, because of the shmem key? I'd say that's not important enough I think we also need to be able to pass some additional parameters to postgres: - config_file, hba_file, ... might be required to start pg in some environments - -P, -O , are sometimes required in cases single user mode is neccessary for data recovery. So I think we should just allow passing through arguments to postgres. Not sure if we need anything but the pid of the postmaster be returned? > >Or don't clone that one but instead have > >PQnextConnectionShouldForkThisBinary('...') and let the psql/pg_dump > >switch be --standalone=full-path-to-the-postgres-binary. Yuck, that's ugly. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
Andres Freund <andres@2ndquadrant.com> writes: > On 2013-12-05 23:01:28 +0200, Heikki Linnakangas wrote: >> Right. Not all of the parameters will make sense for a stand-alone backend >> though, like the hostname and port number. And I think you need need a new >> parameter to pass the path to the 'postgres' executable, unless we re-use >> the host parameter for that. > Hm. I'd guessed that we wouldn't use the connection string to pass down > the executable name and the datadir now that we're inventing a separate > function. But maybe that's unneccessary. > What parameters do we require to be set for that mode: > * path to postgres > * data directory > * database name (single mode after all) > * port, because of the shmem key? I'd say that's not important enough > I think we also need to be able to pass some additional parameters to > postgres: > - config_file, hba_file, ... might be required to start pg in some environments > - -P, -O , are sometimes required in cases single user mode is > neccessary for data recovery. Right, so by the time we're done, we'd still need a connection string or the moral equivalent. My feeling is that we should just treat the executable name and data directory path as new connection parameters, which'd be ignored in normal-connection mode, just as some other parameters will be ignored in single-user mode. Otherwise we'll find ourselves building parameter setting infrastructure that pretty much duplicates what's there for the existing connection parameters. I think the special-purpose command line switches you mention can be passed through PGOPTIONS, rather than inventing a new parameter -- do you have an objection to that? > Not sure if we need anything but the pid of the postmaster be returned? The new PQconnect routine would certainly hand back a PGconn. I think we'd need a new query function PQchildPid(PGconn *) or some such to provide access to the child process PID. regards, tom lane
On 2013-12-06 11:02:48 -0500, Tom Lane wrote: > Andres Freund <andres@2ndquadrant.com> writes: > My feeling is that we should just treat the executable name and data > directory path as new connection parameters, which'd be ignored in > normal-connection mode, just as some other parameters will be ignored in > single-user mode. Otherwise we'll find ourselves building parameter > setting infrastructure that pretty much duplicates what's there for the > existing connection parameters. Right. > I think the special-purpose command line switches you mention can be > passed through PGOPTIONS, rather than inventing a new parameter -- do you > have an objection to that? I am not sure if they currently will get recognized early enough and whether permission checking will interferes, but if so, that's probably fixable. > > Not sure if we need anything but the pid of the postmaster be returned? > > The new PQconnect routine would certainly hand back a PGconn. I think > we'd need a new query function PQchildPid(PGconn *) or some such to > provide access to the child process PID. I was thinking of a pid_t* argument to the new routine, but it's likely unneccessary as we're probably going to end up storing it in PGconn anyway. There's the question what we're going to end up doing with the current single user mode? There's some somewhat ugly code around for it... Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
Andres Freund <andres@2ndquadrant.com> writes: > On 2013-12-06 11:02:48 -0500, Tom Lane wrote: >> I think the special-purpose command line switches you mention can be >> passed through PGOPTIONS, rather than inventing a new parameter -- do you >> have an objection to that? > I am not sure if they currently will get recognized early enough and > whether permission checking will interferes, but if so, that's probably > fixable. Shouldn't be a problem --- the single-user mode will just concatenate the options parameter onto the command line it builds. > There's the question what we're going to end up doing with the current > single user mode? There's some somewhat ugly code around for it... Nothing, in the short term. In a release or two we can get rid of it, probably, but I'd hesitate to provide no overlap at all of these usage modes. regards, tom lane
On Fri, Dec 6, 2013 at 04:04:36PM +0100, Andres Freund wrote: > On 2013-12-05 23:01:28 +0200, Heikki Linnakangas wrote: > > On 12/05/2013 10:37 PM, Robert Haas wrote: > > >On Thu, Dec 5, 2013 at 3:05 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > > >>It might be unpleasant to use in some cases, though. > > > > > >Why would there be more than a few cases in the first place? Who is > > >going to use this beyond psql, pg_dump(all), and pg_upgrade, and why? > > > > Well, you might want to use pgAdmin, or your other favorite admin tool. I'm > > not sure how well it would work, and I think it's OK if we say "sorry, can't > > do that", but it's not a crazy thing to want. > > Pgadmin wouldn't work, it uses multiple connections for anything but the > most trivial tasks. You can't even send a manual sql query using only > one connection. > I think that's true for most of the non-trivial tools. FYI, pg_upgrade in parallel mode needs multiple database connections too. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. +