Date: Sun, 2 Jun 1996 14:51:00 +0200 (GMT+0200)
From: Alan Barrett <apb@iafrica.com>
To: inn-workers@vix.com
Subject: innd doing do too much each time round the select loop
Message-ID: <Pine.NEB.3.91.960602140428.14661E-100000@apb.iafrica.com>
MIME-Version: 1.0
Content-Type: TEXT/PLAIN; charset=US-ASCII

With the increasing pupularity of streaming feeds, it is becoming more
common for the amount of work presented to innd each time round the select
loop to be so great that it takes more than a few seconds per cycle.  By
cranking up the number of unacknowledged CHECK or TAKETHIS commands that a
remote site is prepared to send, I can cause innd to take more than 10
minutes per cycle of the select loop.  Newsreader clients don't want to
wait that long before getting the welcome banner when they connect.

I have addressed this problem in two ways:  By making the select loop
check for new connection requests more often than it checks for work on
most other channels (in much the same way as it checks for requests on the
control channel more often than it checks for ordinary work), and by
reading no more than BUFSIZE characters at a time from NNTP feeds. 

Here's a patch relative to inn-1.4unoff4.

--apb (Alan Barrett)

--- innd/chan.c	Sat Jan 27 07:12:45 1996
+++ innd/chan.c	Sun Jun  2 14:39:30 1996
@@ -17,6 +17,12 @@
 STATIC CHANNEL	*CHANcc;
 STATIC CHANNEL	CHANnull = { CTfree, CSerror, -1 };
 
+#define PRIORITISE_REMCONN
+#ifdef PRIORITISE_REMCONN
+STATIC int	CHANrcfd;
+STATIC CHANNEL	*CHANrc;
+#endif /* PRIORITISE_REMCONN */
+
 
 /*
 **  Set a buffer's contents, ignoring anything that might have
@@ -137,6 +143,13 @@
 	CHANcc = cp;
 	CHANccfd = fd;
     }
+#ifdef PRIORITISE_REMCONN
+    /* Note remconn channel, for efficiency */
+    if (Type == CTremconn) {
+	CHANrc = cp;
+	CHANrcfd = fd;
+    }
+#endif /* PRIORITISE_REMCONN */
     return cp;
 }
 
@@ -500,10 +513,26 @@
 
     p = CHANname(cp);
 
-    /* Read in whatever is there. */
+    /* Read in whatever is there, up to some reasonable limit. */
+    /*
+     * XXX We really want to limit the amount of time it takes to
+     * process the incoming data for this channel.  But there's
+     * no easy way of doing that, so we restrict the data size instead.
+     * If the data is part of a single large article, then reading
+     * and processing many kilobytes at a time costs very little.
+     * If the data is a long list of CHECK commands from a streaming
+     * feed, then every line of data will require a history lookup, and
+     * we probably don't want to do more than about 10 of those per
+     * channel on each cycle of the main select() loop (otherwise we
+     * might take too long before giving other channels a turn).  10
+     * lines of CHECK commands suggests a limit of about 1 kilobyte of
+     * data, or less.  BUFSIZ is often about 1 kilobyte, and is
+     * attractive for other reasons, so let's use that as our size limit.
+     */
     bp = &cp->In;
     bp->Left = bp->Size - bp->Used;
-    i = read(cp->fd, &bp->Data[bp->Used], bp->Left - 1);
+    i = read(cp->fd, &bp->Data[bp->Used],
+	      (bp->Left - 1 > BUFSIZ ? BUFSIZ : bp->Left - 1));
     if (i < 0) {
 #ifdef SUNOS5
     /* return of -2 indicates EAGAIN, for SUNOS5.4 poll() bug workaround */
@@ -759,6 +788,16 @@
 	    (*CHANcc->Reader)(CHANcc);
 	    FD_CLR(CHANccfd, &MyRead);
 	}
+
+#ifdef PRIORITISE_REMCONN
+	/* Try the remconn channel next. */
+	if (FD_ISSET(CHANrcfd, &RCHANmask) && FD_ISSET(CHANrcfd, &MyRead)) {
+	    count--;
+	    if (count > 3) count = 3; /* might be more requests */
+	    (*CHANrc->Reader)(CHANrc);
+	    FD_CLR(CHANrcfd, &MyRead);
+	}
+#endif /* PRIORITISE_REMCONN */
 
 	/* Loop through all active channels.  Somebody could have closed
 	 * closed a channel so we double-check the global mask before