Package: libnss-pgsql1 Version: 1.3.1 Severity: wishlist Tags: patch Hello,
While debugging some poor performance on alioth, I found that libnss-pgsql1 does the following on a getgrent syscall: create a cursor for select group_name, group_password, gid from table for each row fetched, select usernames from other_table where gid = table.gid This has a negligible performance hit on setups where the number of groups is reasonably low, but on alioth, there are roughly 8000 groups, resulting 8000 queries (and 8000 connection setups and teardowns, which is probably worse). The attached patch retains backwards compatibility with the current arrangement, but allows the admin to define a new query type, querygrouparray, which returns an array in the fourth column that contains all the members of the group struct. This reduces the total number of connection setups and select calls to one for the getgrent syscall. Testing on my testbed at home and on the new alioth xen instance appear so far to be giving a several order of magnitude increase in performance, as well as generally lower overhead on the system, as postgres isn't spinning madly all the time. Ideally, I'd like to see the global _usearrays go away, and just get passed around as an argument, but as we're already stuck with one global variable, I just added another in the interests of getting it done ahead of the alioth migration. Please feel free to rework that bit or to make it a little smarter in general - I do not regularly write C these days, and I don't have a huge amount of experience in the nss routines, so I may have snuck in a bug that has so far eluded testing. More eyes are always welcome. Thanks, -- System Information: Debian Release: 3.1 Architecture: i386 (i686) Kernel: Linux 2.6.8-3-686-smp Locale: LANG=en_US.ISO-8859-1, LC_CTYPE=en_US.ISO-8859-1 (charmap=ISO-8859-1) (ignored: LC_ALL set to en_US.ISO-8859-1) Versions of packages libnss-pgsql1 depends on: ii libc6 2.3.2.ds1-22sarge4 GNU C Library: Shared libraries an pn libpgsql2 Not found. -- ----------------------------------------------------------------- | ,''`. Stephen Gran | | : :' : [EMAIL PROTECTED] | | `. `' Debian user, admin, and developer | | `- http://www.debian.org | -----------------------------------------------------------------
diff -Nru /tmp/UXpEUNXpbZ/libnss-pgsql-1.3.1/src/backend.c
/tmp/TVRFj2XQm4/libnss-pgsql-1.3.2/src/backend.c
--- /tmp/UXpEUNXpbZ/libnss-pgsql-1.3.1/src/backend.c 2005-03-25
15:25:41.000000000 -0700
+++ /tmp/TVRFj2XQm4/libnss-pgsql-1.3.2/src/backend.c 2006-09-26
19:02:37.000000000 -0600
@@ -30,12 +30,14 @@
#define GROUP_NAME 0
#define GROUP_PASSWD 1
#define GROUP_GID 2
+#define GROUP_GROUPS 3
// Not used
#define GROUP_MEMBER 4
static PGconn *_conn = NULL;
static int _isopen = 0;
+static int _usearrays = 0;
int backend_isopen()
{
@@ -137,11 +139,16 @@
*/
inline enum nss_status backend_prepare_group()
{
- char *stmt, *cfgname;
+ char *stmt, *cfgname;
PGresult *res;
ExecStatusType status;
- if ( (cfgname=getcfg("querygroup")) != NULL) {
+ if ( (cfgname=getcfg("querygrouparray")) != NULL) {
+ asprintf(&stmt, "DECLARE nss_pgsql_internal_group_curs CURSOR
FOR "
+ "%s FOR READ ONLY",
+ cfgname);
+ _usearrays = 1;
+ } else if ( (cfgname=getcfg("querygroup")) != NULL) {
asprintf(&stmt, "DECLARE nss_pgsql_internal_group_curs CURSOR
FOR "
"%s FOR READ ONLY",
cfgname);
@@ -241,6 +248,84 @@
return NSS_STATUS_SUCCESS;
}
+enum nss_status
+copy_attr_grouparray(char *sptr,
+ char **valptr, char **buffer, size_t *buflen)
+{
+
+ size_t slen;
+
+ slen = strlen(sptr);
+ if(*buflen < slen+1) {
+ return NSS_STATUS_TRYAGAIN;
+ }
+ strncpy(*buffer, sptr, slen);
+ (*buffer)[slen] = '\0';
+
+ *valptr = *buffer;
+
+ *buffer += slen + 1;
+ *buflen -= slen + 1;
+
+ return NSS_STATUS_SUCCESS;
+}
+
+
+enum nss_status getgroupmemarray(PGresult *res,
+ struct group *result,
+ char *buffer, size_t buflen, int row)
+{
+ char *sptr, *saveptr;
+ int end = 0;
+ int n;
+ int t;
+
+ enum nss_status status = NSS_STATUS_NOTFOUND;
+ size_t ptrsize;
+
+ // TODO - parse array member and get number of elements
+
+ sptr = PQgetvalue(res, row, GROUP_GROUPS);
+ end = strlen(sptr) - 1;
+
+ if (end < 2) {
+ end = 0;
+ }
+ char *token[end+1];
+ if ( (0 == strncmp(sptr, "{", 1)) && (0 == strncmp(sptr+end, "}", 1))
) {
+ sptr[end] = '\0';
+ sptr++;
+ }
+ for (n = 0 ; ; sptr = NULL, n++) {
+ if ( (token[n] = strtok_r(sptr, ",", &saveptr) ) == NULL )
+ break;
+ }
+
+ // Make sure there's enough room for the array of pointers to group
member names
+ ptrsize = (n+1) * sizeof(const char *);
+ if(buflen < ptrsize) {
+ status = NSS_STATUS_TRYAGAIN;
+ return status;
+ }
+
+ result->gr_mem = (char**)buffer;
+
+ buffer += (ptrsize+3)&(~0x3);
+ buflen -= (ptrsize+3)&(~0x3);
+
+ status = NSS_STATUS_SUCCESS;
+
+ for(t = 0; t < n; t++) {
+ status = copy_attr_grouparray(token[t], &(result->gr_mem[t]),
&buffer, &buflen);
+ if(status != NSS_STATUS_SUCCESS)
+ return status;
+ }
+ result->gr_mem[t] = NULL;
+
+ return status;
+
+}
+
/*
* return array of strings containing usernames that are member of group with
gid 'gid'
*/
@@ -336,7 +421,12 @@
result->gr_gid = (gid_t) atol(PQgetvalue(res, 0, GROUP_GID));
- status = getgroupmem(result->gr_gid, result, buffer, buflen);
+ if ( _usearrays ) {
+ status = getgroupmemarray(res, result, buffer, buflen, 0);
+ } else {
+ status = getgroupmem(result->gr_gid, result, buffer, buflen);
+ }
+ if(status != NSS_STATUS_SUCCESS) goto BAIL_OUT;
#ifdef DEBUG
print_msg("Converted a res to a grp:\n");
@@ -582,7 +672,13 @@
ename = malloc(2*len+1);
sql_escape(name, ename, len);
- if ( (cfgname=getcfg("querygroup")) != NULL) {
+ if ( (cfgname=getcfg("querygrouparray")) != NULL) {
+ asprintf(&stmt, "%s WHERE %s = '%s'",
+ cfgname,
+ getcfg("group_name"),
+ ename);
+ _usearrays = 1;
+ } else if ( (cfgname=getcfg("querygroup")) != NULL) {
asprintf(&stmt, "%s WHERE %s = '%s'",
cfgname,
getcfg("group_name"),
@@ -625,7 +721,13 @@
PGresult *res;
enum nss_status status = NSS_STATUS_NOTFOUND;
- if ( (cfgname=getcfg("querygroup")) != NULL) {
+ if ( (cfgname=getcfg("querygrouparray")) != NULL) {
+ asprintf(&stmt, "%s WHERE %s = %d",
+ cfgname,
+ getcfg("group_gid"),
+ gid);
+ _usearrays = 1;
+ } else if ( (cfgname=getcfg("querygroup")) != NULL) {
asprintf(&stmt, "%s WHERE %s = %d",
cfgname,
getcfg("group_gid"),
signature.asc
Description: Digital signature
