justanhduc / task-spooler

A scheduler for GPU/CPU tasks
https://justanhduc.github.io/2021/02/03/Task-Spooler.html
GNU General Public License v2.0
273 stars 24 forks source link

Enhancement request: bigger queue size #52

Closed tomaszklim closed 10 months ago

tomaszklim commented 1 year ago

Currently task spooler uses a constant max queue size:

MAXCONN = 1000

(in server.c file)

Such limit was perfectly enough 20-30 years ago, but with recent kernels, running on recent hardware, this limit greatly narrows the scope of possible task spooler usage.

I already looked at the code and I think that increasing this limit to 10000 would be safe.

For compatibility with older/special systems, I suggest:

justanhduc commented 1 year ago

Hey @tomaszklim. Thanks for the suggesstion. Do you want to send a PR?

technojoe commented 11 months ago

Changed server.c to dynamically allocate memory for client_cs from getrlimit, which removes the hardcoded, arbitrary limit of 1000. [issue #52]

--- task-spooler-main/server.c  2023-11-26 21:13:44.622381307 -0800
+++ task-spooler/server.c   2023-11-26 20:31:51.485209017 -0800
@@ -29,10 +29,6 @@

 #include "main.h"

-enum {
-    MAXCONN = 1000
-};
-
 enum Break {
     BREAK,
     NOBREAK,
@@ -63,7 +59,7 @@
 };

 /* Globals */
-static struct Client_conn client_cs[MAXCONN];
+static struct Client_conn *client_cs;
 static int nconnections;
 static char *path;
 static int max_descriptors;
@@ -131,12 +127,16 @@

 static int get_max_descriptors() {
     const int MARGIN = 5; /* stdin, stderr, listen socket, and whatever */
-    int max;
+    int max = 1000; /* initial value used only if getrlimit fails to return a value */
     struct rlimit rlim;
     int res;
     const char *str;

-    max = MAXCONN;
+    res = getrlimit(RLIMIT_NOFILE, &rlim);
+    if (res != 0)
+        warning("getrlimit for open files");
+    else
+        max = rlim.rlim_cur - MARGIN;

     str = getenv("TS_MAXCONN");
     if (str != NULL) {
@@ -149,17 +149,6 @@
     if (max > FD_SETSIZE)
         max = FD_SETSIZE - MARGIN;

-    /* I'd like to use OPEN_MAX or NR_OPEN, but I don't know if any
-     * of them is POSIX compliant */
-
-    res = getrlimit(RLIMIT_NOFILE, &rlim);
-    if (res != 0)
-        warning("getrlimit for open files");
-    else {
-        if (max > rlim.rlim_cur)
-            max = rlim.rlim_cur - MARGIN;
-    }
-
     if (max < 1)
         error("Too few opened descriptors available");

@@ -174,6 +163,9 @@

     process_type = SERVER;
     max_descriptors = get_max_descriptors();
+    
+    /* allocate dynamic memory for client_cs, thus removing any arbitrary limit */
+    client_cs = malloc(max_descriptors * sizeof(struct Client_conn));

     /* Arbitrary limit, that will block the enqueuing, but should allow space
      * for usual ts queries */
@@ -316,6 +308,7 @@
     /* This comes from the parent, in the fork after server_main.
      * This is the last use of path in this process.*/
     free(path);
+    free(client_cs);
     free(logdir);
 #ifndef CPU
     cleanupGpu();
justanhduc commented 10 months ago

Hey @technojoe. Great job! Can you send a PR for this? Thanks!

technojoe commented 10 months ago

When I am off work, I will need help with that. I have little experience with GIT and no experience with GitHub.

justanhduc commented 10 months ago

@technojoe let me know if you need any help

technojoe commented 10 months ago

https://github.com/justanhduc/task-spooler/pull/53

technojoe commented 10 months ago

@justanhduc Anyone going to pull this in?

justanhduc commented 10 months ago

hey @technojoe. I'm a bit occupied these days. I will check it asap. Thanks a lot for your contrib!

justanhduc commented 10 months ago

53