From c3c4bea969ef1cd2fb52b3d513c318e0414fb0b8 Mon Sep 17 00:00:00 2001 From: Darafei Praliaskouski Date: Mon, 4 May 2026 01:49:36 +0400 Subject: [PATCH 1/3] feat(renderd): configure request queue limits --- docs/man/renderd.conf.5 | 15 ++++++ docs/renderd-queue-tuning.md | 68 ++++++++++++++++++++++++++++ etc/renderd/renderd.conf | 2 + etc/renderd/renderd.conf.examples | 4 ++ etc/renderd/renderd.conf.in | 2 + includes/render_config.h | 9 ++-- includes/renderd.h | 2 + includes/request_queue.h | 3 ++ src/renderd.c | 2 +- src/renderd_config.c | 16 +++++++ src/request_queue.c | 53 +++++++++++++++++++--- tests/gen_tile_test.cpp | 44 ++++++++++++++---- tests/renderd_config_test.cpp | 47 +++++++++++++++++++ tests/renderd_config_test_helper.cpp | 12 +++++ 14 files changed, 258 insertions(+), 21 deletions(-) create mode 100644 docs/renderd-queue-tuning.md diff --git a/docs/man/renderd.conf.5 b/docs/man/renderd.conf.5 index 4148d0d1..c7204dd6 100644 --- a/docs/man/renderd.conf.5 +++ b/docs/man/renderd.conf.5 @@ -44,6 +44,19 @@ Specify the number of threads to be used for \fBrenderd\fR. A value of \fB'-1'\fR will configure \fBnum_threads\fR to the number of cores on the system. The default value is \fB'4'\fR (macro definition \fB'NUM_THREADS'\fR). +.TP +.B request_queue_limit +Specify the maximum number of requests kept in each time-critical request queue: priority, normal, low priority, and bulk. +When one of these queues is full, new requests overflow into the dirty queue if space is available. +The default value is \fB'256'\fR when metatiles are enabled, or \fB'512'\fR without metatiles (macro definition \fB'DEFAULT_REQUEST_QUEUE_LIMIT'\fR). + +.TP +.B dirty_queue_limit +Specify the maximum number of requests kept in the dirty queue. +The dirty queue is the background queue for expired or overflowed tiles where no client is waiting for the response. +Larger values can preserve more expired work for quiet periods, but they can also increase memory use and make it easier for important overflowed tiles to sit behind lower-priority background work. +The default value is \fB'8000'\fR when metatiles are enabled, or \fB'10000'\fR without metatiles (macro definition \fB'DEFAULT_DIRTY_QUEUE_LIMIT'\fR). + .TP .B pid_file Specify the file path into which the PID will be written by \fBrenderd\fR. @@ -189,6 +202,8 @@ Only used by \fBrenderd\fR. .SH SEE ALSO .BR renderd(1) +.PP +See \fBdocs/renderd-queue-tuning.md\fR in the source tree for queue tuning and rollout guidance. .BR .SH AUTHOR diff --git a/docs/renderd-queue-tuning.md b/docs/renderd-queue-tuning.md new file mode 100644 index 00000000..98001bb5 --- /dev/null +++ b/docs/renderd-queue-tuning.md @@ -0,0 +1,68 @@ +# renderd Queue Tuning + +`renderd` keeps incoming render requests in five queues: + +- priority request queue: missing tiles where a client is waiting +- request queue: stale tiles where a client is waiting +- low priority request queue: less urgent stale/style refresh work +- dirty queue: background work where no client waits for the response +- bulk request queue: explicit bulk rendering work + +Requests are fetched in that priority order. When a time-critical request queue +is full, new requests overflow into the dirty queue. That prevents immediate +drops, but it also means the overflowed request no longer gets client-waiting +priority. + +## Configuration + +Set the queue limits in the active `[renderd]` section of `renderd.conf`: + +```ini +[renderd] +num_threads=4 +request_queue_limit=256 +dirty_queue_limit=8000 +``` + +`request_queue_limit` applies separately to each of the priority, normal, low +priority, and bulk queues. `dirty_queue_limit` applies to the background dirty +queue. If the dirty queue is also full, new overflow work is dropped. + +The defaults keep the previous compiled-in behavior: + +- with metatiles: `request_queue_limit=256`, `dirty_queue_limit=8000` +- without metatiles: `request_queue_limit=512`, `dirty_queue_limit=10000` + +## Operating Guidance + +Increase `dirty_queue_limit` when dirty tiles are dropped during peak load but +the server has quiet periods later where it can catch up. This is useful for +large public tile services where preserving expired work can improve freshness +after the peak passes. + +Do not increase `dirty_queue_limit` blindly on an overloaded server. If the +higher-priority queues are continuously non-empty, dirty work may still starve, +and a larger dirty queue mainly stores more backlog. Watch queue length, queue +time, render throughput, and dropped-tile metrics before and after the change. + +Increase gradually. For example, move from the default to a limit sized for +roughly 10 minutes of dirty work, then one hour, before trying day-scale values. +The right value depends on metatile size, render throughput, expiry volume, and +whether the service has predictable low-load windows. + +## Rollout Checklist + +1. Record current `renderd_queue`, `renderd_queue_time`, and + `renderd_processed` Munin graphs. +2. Set `dirty_queue_limit` in `renderd.conf`. +3. Restart `renderd` during a maintenance window. +4. Confirm startup logs show the intended `request_queue_limit` and + `dirty_queue_limit`. +5. Watch whether dropped dirty work decreases without sustained growth in queue + time. +6. Roll back to the previous values if dirty queue time grows continuously or + missing-tile requests appear to be delayed by old background work. + +This setting only changes queue capacity. It does not implement bounded +overtaking or fairness between queues; those would require a larger queue +scheduler change. diff --git a/etc/renderd/renderd.conf b/etc/renderd/renderd.conf index 412f0f40..f8d19db2 100644 --- a/etc/renderd/renderd.conf +++ b/etc/renderd/renderd.conf @@ -6,6 +6,8 @@ stats_file=/run/renderd/renderd.stats socketname=/run/renderd/renderd.sock num_threads=4 tile_dir=/var/cache/renderd/tiles +;request_queue_limit=256 +;dirty_queue_limit=8000 [mapnik] plugins_dir=/usr/lib/mapnik/3.1/input diff --git a/etc/renderd/renderd.conf.examples b/etc/renderd/renderd.conf.examples index 7e1c2609..84bd763c 100644 --- a/etc/renderd/renderd.conf.examples +++ b/etc/renderd/renderd.conf.examples @@ -6,12 +6,16 @@ stats_file=/run/renderd/renderd.stats socketname=/run/renderd/renderd.sock num_threads=4 tile_dir=/var/cache/renderd/tiles +;request_queue_limit=256 +;dirty_queue_limit=8000 ;[renderd] ;iphostname=::1 ;ipport=7654 ;num_threads=4 ;tile_dir=rados://tiles/etc/ceph/ceph.conf +;request_queue_limit=256 +;dirty_queue_limit=8000 ;pid_file=/run/renderd/renderd_rados.pid ;stats_file=/run/renderd/renderd.stats diff --git a/etc/renderd/renderd.conf.in b/etc/renderd/renderd.conf.in index 36c82b5a..8d96f9c5 100644 --- a/etc/renderd/renderd.conf.in +++ b/etc/renderd/renderd.conf.in @@ -3,6 +3,8 @@ pid_file=@RENDERD_PIDFILE@ socketname=@RENDERD_SOCKET@ stats_file=@RENDERD_RUN_DIR@/renderd.stats tile_dir=@RENDERD_TILE_DIR@ +; request_queue_limit=256 +; dirty_queue_limit=8000 [mapnik] font_dir=@MAPNIK_FONTS_DIR@ diff --git a/includes/render_config.h b/includes/render_config.h index cbadae0f..80f40f7a 100644 --- a/includes/render_config.h +++ b/includes/render_config.h @@ -114,14 +114,13 @@ // Metatiles are much larger in size so we don't need big queues to handle large areas #ifdef METATILE #define QUEUE_MAX (64) -#define REQ_LIMIT (256) -#define DIRTY_LIMIT (8000) +#define DEFAULT_REQUEST_QUEUE_LIMIT (256) +#define DEFAULT_DIRTY_QUEUE_LIMIT (8000) #else #define QUEUE_MAX (1024) -#define REQ_LIMIT (512) -#define DIRTY_LIMIT (10000) -#define HASHIDX_SIZE 22123 +#define DEFAULT_REQUEST_QUEUE_LIMIT (512) +#define DEFAULT_DIRTY_QUEUE_LIMIT (10000) #endif // Penalty for client making an invalid request (in seconds) diff --git a/includes/renderd.h b/includes/renderd.h index d14e6921..26263b12 100644 --- a/includes/renderd.h +++ b/includes/renderd.h @@ -42,9 +42,11 @@ typedef struct { const char *socketname; const char *stats_filename; const char *tile_dir; + int dirty_queue_limit; int ipport; int mapnik_font_dir_recurse; int num_threads; + int request_queue_limit; } renderd_config; typedef struct { diff --git a/includes/request_queue.h b/includes/request_queue.h index ea59d4c5..f223c8f6 100644 --- a/includes/request_queue.h +++ b/includes/request_queue.h @@ -50,7 +50,9 @@ struct item_idx { }; struct request_queue { + int dirtyLimit; int hashidxSize; + int requestLimit; struct item reqHead, reqPrioHead, reqLowHead, reqBulkHead, dirtyHead, renderHead; struct item_idx *item_hashidx; int reqNum, reqPrioNum, reqLowNum, reqBulkNum, dirtyNum; @@ -60,6 +62,7 @@ struct request_queue { }; struct request_queue *request_queue_init(); +struct request_queue *request_queue_init_with_limits(int request_limit, int dirty_limit); void request_queue_close(struct request_queue *queue); struct item *request_queue_fetch_request(struct request_queue *queue); diff --git a/src/renderd.c b/src/renderd.c index 81e0bda1..a0329daf 100644 --- a/src/renderd.c +++ b/src/renderd.c @@ -809,7 +809,7 @@ int main(int argc, char **argv) } g_logger(G_LOG_LEVEL_INFO, "Initialising request queue"); - render_request_queue = request_queue_init(); + render_request_queue = request_queue_init_with_limits(config.request_queue_limit, config.dirty_queue_limit); if (render_request_queue == NULL) { g_logger(G_LOG_LEVEL_CRITICAL, "Failed to initialise request queue"); diff --git a/src/renderd_config.c b/src/renderd_config.c index 51a5e65b..c046dcd4 100644 --- a/src/renderd_config.c +++ b/src/renderd_config.c @@ -425,7 +425,9 @@ void process_renderd_sections(dictionary *ini, const char *config_file_name, ren copy_string(section, &configs_dest[renderd_section_num].name, renderd_strlen + 2); process_config_int(ini, section, "ipport", &configs_dest[renderd_section_num].ipport, 0); + process_config_int(ini, section, "dirty_queue_limit", &configs_dest[renderd_section_num].dirty_queue_limit, DEFAULT_DIRTY_QUEUE_LIMIT); process_config_int(ini, section, "num_threads", &configs_dest[renderd_section_num].num_threads, NUM_THREADS); + process_config_int(ini, section, "request_queue_limit", &configs_dest[renderd_section_num].request_queue_limit, DEFAULT_REQUEST_QUEUE_LIMIT); process_config_string(ini, section, "iphostname", &configs_dest[renderd_section_num].iphostname, "", INILINE_MAX); process_config_string(ini, section, "pid_file", &configs_dest[renderd_section_num].pid_filename, RENDERD_PIDFILE, PATH_MAX); process_config_string(ini, section, "socketname", &configs_dest[renderd_section_num].socketname, RENDERD_SOCKET, PATH_MAX); @@ -436,6 +438,16 @@ void process_renderd_sections(dictionary *ini, const char *config_file_name, ren configs_dest[renderd_section_num].num_threads = sysconf(_SC_NPROCESSORS_ONLN); } + if (configs_dest[renderd_section_num].request_queue_limit < 1) { + g_logger(G_LOG_LEVEL_CRITICAL, "Specified request_queue_limit (%i) is too small, must be greater than or equal to %i.", configs_dest[renderd_section_num].request_queue_limit, 1); + exit(7); + } + + if (configs_dest[renderd_section_num].dirty_queue_limit < 0) { + g_logger(G_LOG_LEVEL_CRITICAL, "Specified dirty_queue_limit (%i) is too small, must be greater than or equal to %i.", configs_dest[renderd_section_num].dirty_queue_limit, 0); + exit(7); + } + if (strnlen(configs_dest[renderd_section_num].socketname, PATH_MAX) >= renderd_socketname_maxlen) { g_logger(G_LOG_LEVEL_CRITICAL, "Specified socketname (%s) exceeds maximum allowed length of %i.", configs_dest[renderd_section_num].socketname, renderd_socketname_maxlen); exit(7); @@ -510,6 +522,8 @@ void process_config_file(const char *config_file_name, int active_renderd_sectio } g_logger(G_LOG_LEVEL_DEBUG, "\trenderd(%i): num_threads = '%i'", i, config_slaves[i].num_threads); + g_logger(G_LOG_LEVEL_DEBUG, "\trenderd(%i): request_queue_limit = '%i'", i, config_slaves[i].request_queue_limit); + g_logger(G_LOG_LEVEL_DEBUG, "\trenderd(%i): dirty_queue_limit = '%i'", i, config_slaves[i].dirty_queue_limit); g_logger(G_LOG_LEVEL_DEBUG, "\trenderd(%i): pid_file = '%s'", i, config_slaves[i].pid_filename); if (strnlen(config_slaves[i].stats_filename, PATH_MAX)) { @@ -526,6 +540,8 @@ void process_config_file(const char *config_file_name, int active_renderd_sectio } g_logger(log_level, "\trenderd: num_threads = '%i'", config.num_threads); + g_logger(log_level, "\trenderd: request_queue_limit = '%i'", config.request_queue_limit); + g_logger(log_level, "\trenderd: dirty_queue_limit = '%i'", config.dirty_queue_limit); if (active_renderd_section_num == 0 && num_slave_threads > 0) { g_logger(log_level, "\trenderd: num_slave_threads = '%i'", num_slave_threads); diff --git a/src/request_queue.c b/src/request_queue.c index 91a1358a..daef3982 100644 --- a/src/request_queue.c +++ b/src/request_queue.c @@ -21,11 +21,30 @@ #include #include #include +#include #include "render_config.h" #include "request_queue.h" #include "g_logger.h" +#define MIN_HASHIDX_SIZE 2213 + +static int request_queue_hash_size(int request_limit, int dirty_limit) +{ + /* + * Keep the de-duplication index roughly proportional to the configured + * queue capacity. Large dirty queues are useful only if duplicate lookup + * remains cheap while the queue is backlogged. + */ + size_t queue_capacity = (4 * (size_t) request_limit) + (size_t) dirty_limit; + + if (queue_capacity > INT_MAX) { + return INT_MAX; + } + + return MAX(MIN_HASHIDX_SIZE, (int) queue_capacity); +} + static int calcHashKey(struct request_queue *queue, struct item *item) { uint64_t xmlnameHash = 0; @@ -306,27 +325,27 @@ enum protoCmd request_queue_add_request(struct request_queue * queue, struct ite } // New request, add it to render or dirty queue - if ((req->cmd == cmdRender) && (queue->reqNum < REQ_LIMIT)) { + if ((req->cmd == cmdRender) && (queue->reqNum < queue->requestLimit)) { list = &(queue->reqHead); item->inQueue = queueRequest; item->originatedQueue = queueRequest; queue->reqNum++; - } else if ((req->cmd == cmdRenderPrio) && (queue->reqPrioNum < REQ_LIMIT)) { + } else if ((req->cmd == cmdRenderPrio) && (queue->reqPrioNum < queue->requestLimit)) { list = &(queue->reqPrioHead); item->inQueue = queueRequestPrio; item->originatedQueue = queueRequestPrio; queue->reqPrioNum++; - } else if ((req->cmd == cmdRenderLow) && (queue->reqLowNum < REQ_LIMIT)) { + } else if ((req->cmd == cmdRenderLow) && (queue->reqLowNum < queue->requestLimit)) { list = &(queue->reqLowHead); item->inQueue = queueRequestLow; item->originatedQueue = queueRequestLow; queue->reqLowNum++; - } else if ((req->cmd == cmdRenderBulk) && (queue->reqBulkNum < REQ_LIMIT)) { + } else if ((req->cmd == cmdRenderBulk) && (queue->reqBulkNum < queue->requestLimit)) { list = &(queue->reqBulkHead); item->inQueue = queueRequestBulk; item->originatedQueue = queueRequestBulk; queue->reqBulkNum++; - } else if (queue->dirtyNum < DIRTY_LIMIT) { + } else if (queue->dirtyNum < queue->dirtyLimit) { list = &(queue->dirtyHead); item->inQueue = queueDirty; item->originatedQueue = queueDirty; @@ -450,7 +469,7 @@ void request_queue_copy_stats(struct request_queue * queue, stats_struct * stats pthread_mutex_unlock(&queue->qLock); } -struct request_queue * request_queue_init() +struct request_queue * request_queue_init_with_limits(int request_limit, int dirty_limit) { int res; struct request_queue * queue = calloc(1, sizeof(struct request_queue)); @@ -459,6 +478,15 @@ struct request_queue * request_queue_init() return NULL; } + if (request_limit < 1 || dirty_limit < 0) { + g_logger(G_LOG_LEVEL_ERROR, "Invalid request queue limits: request_queue_limit=%i dirty_queue_limit=%i", request_limit, dirty_limit); + free(queue); + return NULL; + } + + queue->requestLimit = request_limit; + queue->dirtyLimit = dirty_limit; + res = pthread_mutex_init(&(queue->qLock), NULL); if (res != 0) { @@ -489,13 +517,24 @@ struct request_queue * request_queue_init() queue->reqBulkHead.next = queue->reqBulkHead.prev = &(queue->reqBulkHead); queue->dirtyHead.next = queue->dirtyHead.prev = &(queue->dirtyHead); queue->renderHead.next = queue->renderHead.prev = &(queue->renderHead); - queue->hashidxSize = HASHIDX_SIZE; + queue->hashidxSize = request_queue_hash_size(request_limit, dirty_limit); queue->item_hashidx = (struct item_idx *) malloc(sizeof(struct item_idx) * queue->hashidxSize); + if (queue->item_hashidx == NULL) { + g_logger(G_LOG_LEVEL_ERROR, "Failed to initialise request queue index with %i entries", queue->hashidxSize); + pthread_mutex_destroy(&(queue->qLock)); + free(queue); + return NULL; + } bzero(queue->item_hashidx, sizeof(struct item_idx) * queue->hashidxSize); return queue; } +struct request_queue * request_queue_init() +{ + return request_queue_init_with_limits(DEFAULT_REQUEST_QUEUE_LIMIT, DEFAULT_DIRTY_QUEUE_LIMIT); +} + void request_queue_close(struct request_queue * queue) { //TODO: Free items if the queues are not empty at closing time diff --git a/tests/gen_tile_test.cpp b/tests/gen_tile_test.cpp index eb4291ac..a77a7a1e 100644 --- a/tests/gen_tile_test.cpp +++ b/tests/gen_tile_test.cpp @@ -377,7 +377,7 @@ TEST_CASE("renderd/queueing", "request queueing") struct item *item; request_queue *queue = request_queue_init(); - for (int i = 1; i < (2 * REQ_LIMIT + DIRTY_LIMIT + 2); i++) { + for (int i = 1; i < (2 * DEFAULT_REQUEST_QUEUE_LIMIT + DEFAULT_DIRTY_QUEUE_LIMIT + 2); i++) { item = init_render_request(cmdRenderPrio); res = request_queue_add_request(queue, item); INFO("i: " << i); @@ -386,19 +386,47 @@ TEST_CASE("renderd/queueing", "request queueing") INFO("NoDirt: " << request_queue_no_requests_queued(queue, cmdDirty)); INFO("NoBulk: " << request_queue_no_requests_queued(queue, cmdRenderBulk)); - if (i <= REQ_LIMIT) { + if (i <= DEFAULT_REQUEST_QUEUE_LIMIT) { REQUIRE(res == cmdIgnore); REQUIRE(request_queue_no_requests_queued(queue, cmdRenderPrio) == i); - } else if (i <= (REQ_LIMIT + DIRTY_LIMIT)) { + } else if (i <= (DEFAULT_REQUEST_QUEUE_LIMIT + DEFAULT_DIRTY_QUEUE_LIMIT)) { // Requests should overflow into the dirty queue REQUIRE(res == cmdNotDone); - REQUIRE(request_queue_no_requests_queued(queue, cmdRenderPrio) == REQ_LIMIT); - REQUIRE(request_queue_no_requests_queued(queue, cmdDirty) == (i - REQ_LIMIT)); + REQUIRE(request_queue_no_requests_queued(queue, cmdRenderPrio) == DEFAULT_REQUEST_QUEUE_LIMIT); + REQUIRE(request_queue_no_requests_queued(queue, cmdDirty) == (i - DEFAULT_REQUEST_QUEUE_LIMIT)); } else { // Requests should be dropped altogether REQUIRE(res == cmdNotDone); - REQUIRE(request_queue_no_requests_queued(queue, cmdRenderPrio) == REQ_LIMIT); - REQUIRE(request_queue_no_requests_queued(queue, cmdDirty) == DIRTY_LIMIT); + REQUIRE(request_queue_no_requests_queued(queue, cmdRenderPrio) == DEFAULT_REQUEST_QUEUE_LIMIT); + REQUIRE(request_queue_no_requests_queued(queue, cmdDirty) == DEFAULT_DIRTY_QUEUE_LIMIT); + } + } + + request_queue_close(queue); + } + + SECTION("renderd/queueing/configured overflow limits", "test if configurable queue limits are honoured") { + enum protoCmd res; + struct item *item; + request_queue *queue = request_queue_init_with_limits(2, 3); + + REQUIRE(queue != NULL); + + for (int i = 1; i < 8; i++) { + item = init_render_request(cmdRenderPrio); + res = request_queue_add_request(queue, item); + + if (i <= 2) { + REQUIRE(res == cmdIgnore); + REQUIRE(request_queue_no_requests_queued(queue, cmdRenderPrio) == i); + } else if (i <= 5) { + REQUIRE(res == cmdNotDone); + REQUIRE(request_queue_no_requests_queued(queue, cmdRenderPrio) == 2); + REQUIRE(request_queue_no_requests_queued(queue, cmdDirty) == (i - 2)); + } else { + REQUIRE(res == cmdNotDone); + REQUIRE(request_queue_no_requests_queued(queue, cmdRenderPrio) == 2); + REQUIRE(request_queue_no_requests_queued(queue, cmdDirty) == 3); } } @@ -409,7 +437,7 @@ TEST_CASE("renderd/queueing", "request queueing") pthread_t *addition_threads; request_queue *queue; - REQUIRE((NO_THREADS * NO_QUEUE_REQUESTS) < DIRTY_LIMIT); + REQUIRE((NO_THREADS * NO_QUEUE_REQUESTS) < DEFAULT_DIRTY_QUEUE_LIMIT); for (int j = 0; j < NO_TEST_REPEATS; j++) { // As we are looking for race conditions, repeat this test many times addition_threads = (pthread_t *)calloc(NO_THREADS, sizeof(pthread_t)); diff --git a/tests/renderd_config_test.cpp b/tests/renderd_config_test.cpp index 7f5f2ed1..69bf7b62 100644 --- a/tests/renderd_config_test.cpp +++ b/tests/renderd_config_test.cpp @@ -437,6 +437,38 @@ TEST_CASE("renderd_config config parser", "specific testing") REQUIRE_THAT(err_log_lines, Catch::Matchers::Contains("Specified socketname (" + renderd_socketname + ") exceeds maximum allowed length of " + std::to_string(renderd_socketname_maxlen) + ".")); } + SECTION("renderd.conf renderd section request_queue_limit too small", "should return 7") { + std::string renderd_conf = std::tmpnam(nullptr); + std::ofstream renderd_conf_file; + renderd_conf_file.open(renderd_conf); + renderd_conf_file << "[mapnik]\n[map]\n"; + renderd_conf_file << "[renderd]\nrequest_queue_limit=0\n"; + renderd_conf_file.close(); + + std::vector argv = {"--config", renderd_conf}; + + int status = run_command(test_binary, argv); + std::remove(renderd_conf.c_str()); + REQUIRE(WEXITSTATUS(status) == 7); + REQUIRE_THAT(err_log_lines, Catch::Matchers::Contains("Specified request_queue_limit (0) is too small, must be greater than or equal to 1.")); + } + + SECTION("renderd.conf renderd section dirty_queue_limit too small", "should return 7") { + std::string renderd_conf = std::tmpnam(nullptr); + std::ofstream renderd_conf_file; + renderd_conf_file.open(renderd_conf); + renderd_conf_file << "[mapnik]\n[map]\n"; + renderd_conf_file << "[renderd]\ndirty_queue_limit=-1\n"; + renderd_conf_file.close(); + + std::vector argv = {"--config", renderd_conf}; + + int status = run_command(test_binary, argv); + std::remove(renderd_conf.c_str()); + REQUIRE(WEXITSTATUS(status) == 7); + REQUIRE_THAT(err_log_lines, Catch::Matchers::Contains("Specified dirty_queue_limit (-1) is too small, must be greater than or equal to 0.")); + } + SECTION("renderd.conf duplicate renderd section names", "should return 7") { std::string renderd_conf = std::tmpnam(nullptr); std::ofstream renderd_conf_file; @@ -466,6 +498,21 @@ TEST_CASE("renderd_config_test_helper", "specific testing") REQUIRE(WEXITSTATUS(status) == 0); } + SECTION("valid renderd.conf file with configured queue limits", "should return 0") { + std::string renderd_conf = std::tmpnam(nullptr); + std::ofstream renderd_conf_file; + renderd_conf_file.open(renderd_conf); + renderd_conf_file << "[mapnik]\n[map]\n"; + renderd_conf_file << "[renderd]\nrequest_queue_limit=42\ndirty_queue_limit=4242\n"; + renderd_conf_file.close(); + + std::vector argv = {renderd_conf, std::to_string(0), "assert_queue_limits", "42", "4242"}; + + int status = run_command(test_binary, argv); + std::remove(renderd_conf.c_str()); + REQUIRE(WEXITSTATUS(status) == 0); + } + SECTION("valid renderd.conf file with valid active renderd section (process_renderd_sections)", "should return 0") { std::vector argv = {RENDERD_CONF, std::to_string(0), "process_renderd_sections"}; diff --git a/tests/renderd_config_test_helper.cpp b/tests/renderd_config_test_helper.cpp index 00eea759..0f68ab99 100644 --- a/tests/renderd_config_test_helper.cpp +++ b/tests/renderd_config_test_helper.cpp @@ -3,6 +3,7 @@ #include "renderd_config.h" #include +#include int main(int argc, char **argv) { @@ -28,6 +29,17 @@ int main(int argc, char **argv) process_config_file(config_file_name, active_renderd_section_num, G_LOG_LEVEL_WARNING); } + if (strcmp(process_function, "assert_queue_limits") == 0) { + int expected_request_queue_limit = atoi(argv[4]); + int expected_dirty_queue_limit = atoi(argv[5]); + process_config_file(config_file_name, active_renderd_section_num, G_LOG_LEVEL_WARNING); + + if (config.request_queue_limit != expected_request_queue_limit || config.dirty_queue_limit != expected_dirty_queue_limit) { + fprintf(stderr, "queue limits mismatch: request_queue_limit=%i dirty_queue_limit=%i\n", config.request_queue_limit, config.dirty_queue_limit); + exit(1); + } + } + if (strcmp(process_function, "process_renderd_sections") == 0) { process_renderd_sections(NULL, config_file_name, config_slaves); } From e5b2b734110db3dd72063316c537490edda79473 Mon Sep 17 00:00:00 2001 From: Darafei Praliaskouski Date: Mon, 4 May 2026 04:08:54 +0400 Subject: [PATCH 2/3] feat(renderd): use priority queue for render requests --- docs/renderd-queue-tuning.md | 22 ++- includes/request_queue.h | 4 +- src/request_queue.c | 346 +++++++++++++++++++++++++++-------- tests/gen_tile_test.cpp | 277 +++++++++++++++++++++++++++- 4 files changed, 568 insertions(+), 81 deletions(-) diff --git a/docs/renderd-queue-tuning.md b/docs/renderd-queue-tuning.md index 98001bb5..80e19e01 100644 --- a/docs/renderd-queue-tuning.md +++ b/docs/renderd-queue-tuning.md @@ -1,6 +1,7 @@ # renderd Queue Tuning -`renderd` keeps incoming render requests in five queues: +`renderd` keeps incoming render requests in a priority queue with five priority +classes: - priority request queue: missing tiles where a client is waiting - request queue: stale tiles where a client is waiting @@ -8,10 +9,12 @@ - dirty queue: background work where no client waits for the response - bulk request queue: explicit bulk rendering work -Requests are fetched in that priority order. When a time-critical request queue -is full, new requests overflow into the dirty queue. That prevents immediate -drops, but it also means the overflowed request no longer gets client-waiting -priority. +Requests are fetched by effective priority, preserving FIFO order within the +same priority class. When a time-critical request class is full, new requests +overflow into the dirty class. That prevents immediate drops, but it also means +the overflowed request no longer gets client-waiting priority until a later +client request for the same tile can promote it back into the appropriate +priority class. ## Configuration @@ -44,6 +47,10 @@ Do not increase `dirty_queue_limit` blindly on an overloaded server. If the higher-priority queues are continuously non-empty, dirty work may still starve, and a larger dirty queue mainly stores more backlog. Watch queue length, queue time, render throughput, and dropped-tile metrics before and after the change. +When a client requests a tile that is already queued as lower-priority work, +`renderd` raises that existing item in the priority queue if the target priority +class has capacity. If the target class is still full, the request remains +background work and the client receives the usual not-done response. Increase gradually. For example, move from the default to a limit sized for roughly 10 minutes of dirty work, then one hour, before trying day-scale values. @@ -63,6 +70,7 @@ whether the service has predictable low-load windows. 6. Roll back to the previous values if dirty queue time grows continuously or missing-tile requests appear to be delayed by old background work. -This setting only changes queue capacity. It does not implement bounded -overtaking or fairness between queues; those would require a larger queue +This setting changes per-priority capacity and works with duplicate promotion +for already queued tiles. It does not implement bounded overtaking or broader +fairness between unrelated queued tiles; those would require a larger queue scheduler change. diff --git a/includes/request_queue.h b/includes/request_queue.h index f223c8f6..a7c364f0 100644 --- a/includes/request_queue.h +++ b/includes/request_queue.h @@ -27,6 +27,7 @@ extern "C" { #endif #define HASHIDX_SIZE 2213 +#define PENDING_QUEUE_RANKS 5 typedef struct { long noDirtyRender; @@ -53,7 +54,8 @@ struct request_queue { int dirtyLimit; int hashidxSize; int requestLimit; - struct item reqHead, reqPrioHead, reqLowHead, reqBulkHead, dirtyHead, renderHead; + struct item pendingHead, renderHead; + struct item *pendingTail[PENDING_QUEUE_RANKS]; struct item_idx *item_hashidx; int reqNum, reqPrioNum, reqLowNum, reqBulkNum, dirtyNum; pthread_mutex_t qLock; diff --git a/src/request_queue.c b/src/request_queue.c index daef3982..8d4db530 100644 --- a/src/request_queue.c +++ b/src/request_queue.c @@ -45,16 +45,40 @@ static int request_queue_hash_size(int request_limit, int dirty_limit) return MAX(MIN_HASHIDX_SIZE, (int) queue_capacity); } -static int calcHashKey(struct request_queue *queue, struct item *item) +static uint64_t string_hash(const char *value, size_t max_len) { - uint64_t xmlnameHash = 0; - uint64_t key; + uint64_t hash = 0; - for (int i = 0; (item->req.xmlname[i] != 0) && (i < sizeof(item->req.xmlname)); i++) { - xmlnameHash += item->req.xmlname[i]; + for (size_t i = 0; (i < max_len) && (value[i] != 0); i++) { + hash = (hash * 33) + (unsigned char)value[i]; } - key = ((uint64_t)(xmlnameHash & 0x1FF) << 52) + ((uint64_t)(item->req.z) << 48) + ((uint64_t)(item->mx & 0xFFFFFF) << 24) + (item->my & 0xFFFFFF); + return hash; +} + +static int same_tile_request(struct item *item, struct item *test) +{ + return (item->mx == test->mx) && + (item->my == test->my) && + (item->req.z == test->req.z) && + (strncmp(item->req.xmlname, test->req.xmlname, sizeof(item->req.xmlname)) == 0) && + (strncmp(item->req.mimetype, test->req.mimetype, sizeof(item->req.mimetype)) == 0) && + (strncmp(item->req.options, test->req.options, sizeof(item->req.options)) == 0); +} + +static int calcHashKey(struct request_queue *queue, struct item *item) +{ + uint64_t key; + uint64_t name_hash = string_hash(item->req.xmlname, sizeof(item->req.xmlname)); + uint64_t mimetype_hash = string_hash(item->req.mimetype, sizeof(item->req.mimetype)); + uint64_t options_hash = string_hash(item->req.options, sizeof(item->req.options)); + + key = ((name_hash & 0x1FF) << 52) + + ((mimetype_hash & 0xFF) << 44) + + ((options_hash & 0xFF) << 36) + + ((uint64_t)(item->req.z) << 32) + + ((uint64_t)(item->mx & 0xFFFF) << 16) + + (item->my & 0xFFFF); return key % queue->hashidxSize; } @@ -73,9 +97,7 @@ static struct item * lookup_item_idx(struct request_queue * queue, struct item * while (nextItem != NULL) { test = nextItem->item; - if ((item->mx == test->mx) && (item->my == test->my) - && (item->req.z == test->req.z) && (!strcmp( - item->req.xmlname, test->req.xmlname))) { + if (same_tile_request(item, test)) { return test; } else { nextItem = nextItem->next; @@ -129,9 +151,7 @@ static void remove_item_idx(struct request_queue * queue, struct item * item) while (nextItem != NULL) { test = nextItem->item; - if ((item->mx == test->mx) && (item->my == test->my) && (item->req.z - == test->req.z) && (!strcmp(item->req.xmlname, - test->req.xmlname))) { + if (same_tile_request(item, test)) { /* * Found item, removing it from list */ @@ -162,6 +182,180 @@ static void remove_item_idx(struct request_queue * queue, struct item * item) } } +static int *request_queue_counter(struct request_queue *queue, enum queueEnum queue_type) +{ + switch (queue_type) { + case queueRequestPrio: + return &(queue->reqPrioNum); + + case queueRequest: + return &(queue->reqNum); + + case queueRequestLow: + return &(queue->reqLowNum); + + case queueDirty: + return &(queue->dirtyNum); + + case queueRequestBulk: + return &(queue->reqBulkNum); + + default: + return NULL; + } +} + +static int request_queue_limit(struct request_queue *queue, enum queueEnum queue_type) +{ + return (queue_type == queueDirty) ? queue->dirtyLimit : queue->requestLimit; +} + +static int request_queue_rank(enum queueEnum queue_type) +{ + switch (queue_type) { + case queueRequestPrio: + return 0; + + case queueRequest: + return 1; + + case queueRequestLow: + return 2; + + case queueDirty: + return 3; + + case queueRequestBulk: + return 4; + + default: + return INT_MAX; + } +} + +static int request_queue_for_cmd(enum protoCmd cmd, enum queueEnum *queue_type) +{ + switch (cmd) { + case cmdRenderPrio: + *queue_type = queueRequestPrio; + return 1; + + case cmdRender: + *queue_type = queueRequest; + return 1; + + case cmdRenderLow: + *queue_type = queueRequestLow; + return 1; + + case cmdDirty: + *queue_type = queueDirty; + return 1; + + case cmdRenderBulk: + *queue_type = queueRequestBulk; + return 1; + + default: + return 0; + } +} + +static void request_queue_attach_duplicate(struct item *item, struct item *duplicate) +{ + duplicate->duplicates = item->duplicates; + item->duplicates = duplicate; + duplicate->inQueue = queueDuplicate; +} + +static void request_queue_unlink_item(struct item *item) +{ + item->next->prev = item->prev; + item->prev->next = item->next; +} + +static void request_queue_unlink_pending(struct request_queue *queue, struct item *item) +{ + int rank = request_queue_rank(item->inQueue); + + if (rank < PENDING_QUEUE_RANKS && queue->pendingTail[rank] == item) { + if ((item->prev != &(queue->pendingHead)) && (request_queue_rank(item->prev->inQueue) == rank)) { + queue->pendingTail[rank] = item->prev; + } else { + queue->pendingTail[rank] = &(queue->pendingHead); + } + } + + request_queue_unlink_item(item); +} + +static void request_queue_insert_pending(struct request_queue *queue, struct item *item) +{ + struct item *pos; + int rank = request_queue_rank(item->inQueue); + + if (rank >= PENDING_QUEUE_RANKS) { + return; + } + + pos = &(queue->pendingHead); + + for (int i = rank; i >= 0; i--) { + if (queue->pendingTail[i] != &(queue->pendingHead)) { + pos = queue->pendingTail[i]; + break; + } + } + + item->next = pos->next; + item->prev = pos; + item->prev->next = item; + item->next->prev = item; + queue->pendingTail[rank] = item; +} + +static int request_queue_promote_pending(struct request_queue *queue, struct item *item, struct item *duplicate) +{ + enum queueEnum target_queue; + int *current_count; + int *target_count; + + if (!request_queue_for_cmd(duplicate->req.cmd, &target_queue)) { + return 0; + } + + if (target_queue == queueDirty) { + return 0; + } + + if (request_queue_rank(target_queue) >= request_queue_rank(item->inQueue)) { + return 0; + } + + target_count = request_queue_counter(queue, target_queue); + current_count = request_queue_counter(queue, item->inQueue); + + if (target_count == NULL || current_count == NULL) { + return 0; + } + + if (*target_count >= request_queue_limit(queue, target_queue)) { + return 0; + } + + request_queue_unlink_pending(queue, item); + (*current_count)--; + + item->inQueue = target_queue; + item->originatedQueue = target_queue; + request_queue_insert_pending(queue, item); + (*target_count)++; + request_queue_attach_duplicate(item, duplicate); + pthread_cond_signal(&queue->qCond); + + return 1; +} + static enum protoCmd pending(struct request_queue * queue, struct item *test) { // check all queues and render list to see if this request already queued @@ -172,10 +366,12 @@ static enum protoCmd pending(struct request_queue * queue, struct item *test) item = lookup_item_idx(queue, test); if (item != NULL) { + if (request_queue_promote_pending(queue, item, test)) { + return cmdIgnore; + } + if ((item->inQueue == queueRender) || (item->inQueue == queueRequest) || (item->inQueue == queueRequestPrio) || (item->inQueue == queueRequestLow)) { - test->duplicates = item->duplicates; - item->duplicates = test; - test->inQueue = queueDuplicate; + request_queue_attach_duplicate(item, test); return cmdIgnore; } else if ((item->inQueue == queueDirty) || (item->inQueue == queueRequestBulk)) { return cmdNotDone; @@ -195,31 +391,45 @@ struct item *request_queue_fetch_request(struct request_queue * queue) pthread_cond_wait(&(queue->qCond), &(queue->qLock)); } - if (queue->reqPrioNum) { - item = queue->reqPrioHead.next; - queue->reqPrioNum--; - queue->stats.noReqPrioRender++; - } else if (queue->reqNum) { - item = queue->reqHead.next; - queue->reqNum--; - queue->stats.noReqRender++; - } else if (queue->reqLowNum) { - item = queue->reqLowHead.next; - queue->reqLowNum--; - queue->stats.noReqLowRender++; - } else if (queue->dirtyNum) { - item = queue->dirtyHead.next; - queue->dirtyNum--; - queue->stats.noDirtyRender++; - } else if (queue->reqBulkNum) { - item = queue->reqBulkHead.next; - queue->reqBulkNum--; - queue->stats.noReqBulkRender++; - } + item = queue->pendingHead.next; if (item) { - item->next->prev = item->prev; - item->prev->next = item->next; + switch (item->inQueue) { + case queueRequestPrio: { + queue->reqPrioNum--; + queue->stats.noReqPrioRender++; + break; + } + + case queueRequest: { + queue->reqNum--; + queue->stats.noReqRender++; + break; + } + + case queueRequestLow: { + queue->reqLowNum--; + queue->stats.noReqLowRender++; + break; + } + + case queueDirty: { + queue->dirtyNum--; + queue->stats.noDirtyRender++; + break; + } + + case queueRequestBulk: { + queue->reqBulkNum--; + queue->stats.noReqBulkRender++; + break; + } + + default: + break; + } + + request_queue_unlink_pending(queue, item); //Add item to render queue item->prev = &(queue->renderHead); @@ -241,16 +451,16 @@ void request_queue_clear_requests_by_fd(struct request_queue * queue, int fd) { struct item *item, *dupes, *queueHead; - /**Only need to look up on the shorter request and render queue, - * as the all requests on the dirty queue already have a FD_INVALID - * as a file descriptor, so using the linear list shouldn't be a problem + /**Only need to scan the pending priority queue and render queue. + * Most dirty queue entries have FD_INVALID, but promoted duplicates may + * carry a client fd while the primary item still represents the same tile. */ pthread_mutex_lock(&(queue->qLock)); - for (int i = 0; i < 4; i++) { + for (int i = 0; i < 2; i++) { switch (i) { case 0: { - queueHead = &(queue->reqHead); + queueHead = &(queue->pendingHead); break; } @@ -258,21 +468,18 @@ void request_queue_clear_requests_by_fd(struct request_queue * queue, int fd) queueHead = &(queue->renderHead); break; } - - case 2: { - queueHead = &(queue->reqPrioHead); - break; - } - - case 3: { - queueHead = &(queue->reqBulkHead); - break; - } } item = queueHead->next; while (item != queueHead) { + if (queueHead == &(queue->pendingHead) && item->inQueue == queueDirty) { + int rank = request_queue_rank(queueDirty); + + item = queue->pendingTail[rank]->next; + continue; + } + if (item->fd == fd) { item->fd = FD_INVALID; } @@ -298,7 +505,7 @@ enum protoCmd request_queue_add_request(struct request_queue * queue, struct ite { enum protoCmd status; const struct protocol *req; - struct item *list = NULL; + int item_added = 0; req = &(item->req); if (queue == NULL) { @@ -326,31 +533,31 @@ enum protoCmd request_queue_add_request(struct request_queue * queue, struct ite // New request, add it to render or dirty queue if ((req->cmd == cmdRender) && (queue->reqNum < queue->requestLimit)) { - list = &(queue->reqHead); item->inQueue = queueRequest; item->originatedQueue = queueRequest; queue->reqNum++; + item_added = 1; } else if ((req->cmd == cmdRenderPrio) && (queue->reqPrioNum < queue->requestLimit)) { - list = &(queue->reqPrioHead); item->inQueue = queueRequestPrio; item->originatedQueue = queueRequestPrio; queue->reqPrioNum++; + item_added = 1; } else if ((req->cmd == cmdRenderLow) && (queue->reqLowNum < queue->requestLimit)) { - list = &(queue->reqLowHead); item->inQueue = queueRequestLow; item->originatedQueue = queueRequestLow; queue->reqLowNum++; + item_added = 1; } else if ((req->cmd == cmdRenderBulk) && (queue->reqBulkNum < queue->requestLimit)) { - list = &(queue->reqBulkHead); item->inQueue = queueRequestBulk; item->originatedQueue = queueRequestBulk; queue->reqBulkNum++; + item_added = 1; } else if (queue->dirtyNum < queue->dirtyLimit) { - list = &(queue->dirtyHead); item->inQueue = queueDirty; item->originatedQueue = queueDirty; queue->dirtyNum++; item->fd = FD_INVALID; // No response after render + item_added = 1; } else { // The queue is severely backlogged. Drop request queue->stats.noReqDroped++; @@ -359,11 +566,8 @@ enum protoCmd request_queue_add_request(struct request_queue * queue, struct ite return cmdNotDone; } - if (list) { - item->next = list; - item->prev = list->prev; - item->prev->next = item; - list->prev = item; + if (item_added) { + request_queue_insert_pending(queue, item); /* In addition to the linked list, add item to a hash table index * for faster lookup of pending requests. */ @@ -376,7 +580,7 @@ enum protoCmd request_queue_add_request(struct request_queue * queue, struct ite pthread_mutex_unlock(&queue->qLock); - return (list == &(queue->dirtyHead)) ? cmdNotDone : cmdIgnore; + return (item->inQueue == queueDirty) ? cmdNotDone : cmdIgnore; } void request_queue_remove_request(struct request_queue * queue, struct item * request, int render_time) @@ -422,8 +626,7 @@ void request_queue_remove_request(struct request_queue * queue, struct item * re queue->stats.timeZoomRender[request->req.z] += render_time; } - request->next->prev = request->prev; - request->prev->next = request->next; + request_queue_unlink_item(request); remove_item_idx(queue, request); pthread_mutex_unlock(&(queue->qLock)); } @@ -511,12 +714,11 @@ struct request_queue * request_queue_init_with_limits(int request_limit, int dir queue->stats.noReqLowRender = 0; queue->stats.noReqBulkRender = 0; - queue->reqHead.next = queue->reqHead.prev = &(queue->reqHead); - queue->reqPrioHead.next = queue->reqPrioHead.prev = &(queue->reqPrioHead); - queue->reqLowHead.next = queue->reqLowHead.prev = &(queue->reqLowHead); - queue->reqBulkHead.next = queue->reqBulkHead.prev = &(queue->reqBulkHead); - queue->dirtyHead.next = queue->dirtyHead.prev = &(queue->dirtyHead); + queue->pendingHead.next = queue->pendingHead.prev = &(queue->pendingHead); queue->renderHead.next = queue->renderHead.prev = &(queue->renderHead); + for (int i = 0; i < PENDING_QUEUE_RANKS; i++) { + queue->pendingTail[i] = &(queue->pendingHead); + } queue->hashidxSize = request_queue_hash_size(request_limit, dirty_limit); queue->item_hashidx = (struct item_idx *) malloc(sizeof(struct item_idx) * queue->hashidxSize); if (queue->item_hashidx == NULL) { diff --git a/tests/gen_tile_test.cpp b/tests/gen_tile_test.cpp index a77a7a1e..fa2ec1d9 100644 --- a/tests/gen_tile_test.cpp +++ b/tests/gen_tile_test.cpp @@ -265,7 +265,8 @@ TEST_CASE("renderd/queueing", "request queueing") struct item *itemL = init_render_request(cmdRenderLow); request_queue_add_request(queue, itemL); - // We should be retrieving items in the order RenderPrio, Render, Dirty, Bulk + // We should retrieve items in priority queue order while preserving FIFO + // order within each priority class. item2 = request_queue_fetch_request(queue); INFO("itemRP: " << itemRP); INFO("itemR: " << itemR); @@ -372,6 +373,250 @@ TEST_CASE("renderd/queueing", "request queueing") request_queue_close(queue); } + SECTION("renderd/queueing/promote pending lower priority request", "test if later interactive requests promote already queued background work") { + enum protoCmd res; + struct item *item; + struct item *duplicate; + struct item *fetched; + request_queue *queue = request_queue_init_with_limits(1, 4); + + REQUIRE(queue != NULL); + + item = init_render_request(cmdRenderPrio); + item->mx = 100; + res = request_queue_add_request(queue, item); + REQUIRE(res == cmdIgnore); + REQUIRE(request_queue_no_requests_queued(queue, cmdRenderPrio) == 1); + + item = init_render_request(cmdRenderPrio); + item->mx = 200; + res = request_queue_add_request(queue, item); + REQUIRE(res == cmdNotDone); + REQUIRE(request_queue_no_requests_queued(queue, cmdRenderPrio) == 1); + REQUIRE(request_queue_no_requests_queued(queue, cmdDirty) == 1); + + duplicate = init_render_request(cmdRenderPrio); + duplicate->mx = 200; + duplicate->fd = 42; + res = request_queue_add_request(queue, duplicate); + REQUIRE(res == cmdNotDone); + REQUIRE(request_queue_no_requests_queued(queue, cmdRenderPrio) == 1); + REQUIRE(request_queue_no_requests_queued(queue, cmdDirty) == 1); + + fetched = request_queue_fetch_request(queue); + REQUIRE(fetched->mx == 100); + request_queue_remove_request(queue, fetched, 0); + free(fetched); + + duplicate = init_render_request(cmdRenderPrio); + duplicate->mx = 200; + duplicate->fd = 43; + res = request_queue_add_request(queue, duplicate); + REQUIRE(res == cmdIgnore); + REQUIRE(request_queue_no_requests_queued(queue, cmdRenderPrio) == 1); + REQUIRE(request_queue_no_requests_queued(queue, cmdDirty) == 0); + + fetched = request_queue_fetch_request(queue); + REQUIRE(fetched->mx == 200); + REQUIRE(fetched->req.cmd == cmdRenderPrio); + REQUIRE(fetched->duplicates != NULL); + REQUIRE(fetched->duplicates->fd == 43); + request_queue_remove_request(queue, fetched, 0); + free(fetched->duplicates); + free(fetched); + + request_queue_close(queue); + } + + SECTION("renderd/queueing/promote dirty request", "test if dirty work is requeued when a client starts waiting for the same tile") { + enum protoCmd res; + struct item *dirty; + struct item *duplicate; + struct item *fetched; + request_queue *queue = request_queue_init_with_limits(2, 4); + + REQUIRE(queue != NULL); + + dirty = init_render_request(cmdDirty); + dirty->mx = 300; + res = request_queue_add_request(queue, dirty); + REQUIRE(res == cmdNotDone); + REQUIRE(request_queue_no_requests_queued(queue, cmdDirty) == 1); + + duplicate = init_render_request(cmdRender); + duplicate->mx = 300; + duplicate->fd = 44; + res = request_queue_add_request(queue, duplicate); + REQUIRE(res == cmdIgnore); + REQUIRE(request_queue_no_requests_queued(queue, cmdRender) == 1); + REQUIRE(request_queue_no_requests_queued(queue, cmdDirty) == 0); + + fetched = request_queue_fetch_request(queue); + REQUIRE(fetched == dirty); + REQUIRE(fetched->req.cmd == cmdDirty); + REQUIRE(fetched->duplicates != NULL); + REQUIRE(fetched->duplicates->fd == 44); + request_queue_remove_request(queue, fetched, 0); + free(fetched->duplicates); + free(fetched); + + request_queue_close(queue); + } + + SECTION("renderd/queueing/promotion preserves primary response metadata", "test a later duplicate does not overwrite an already queued request") { + enum protoCmd res; + struct item *low; + struct item *priority; + struct item *fetched; + request_queue *queue = request_queue_init_with_limits(2, 4); + + REQUIRE(queue != NULL); + + low = init_render_request(cmdRenderLow); + low->mx = 375; + low->my = 375; + low->req.x = 376; + low->req.y = 377; + low->fd = 48; + res = request_queue_add_request(queue, low); + REQUIRE(res == cmdIgnore); + + priority = init_render_request(cmdRenderPrio); + priority->mx = 375; + priority->my = 375; + priority->req.x = 378; + priority->req.y = 379; + priority->fd = 49; + res = request_queue_add_request(queue, priority); + REQUIRE(res == cmdIgnore); + + fetched = request_queue_fetch_request(queue); + REQUIRE(fetched == low); + REQUIRE(fetched->req.cmd == cmdRenderLow); + REQUIRE(fetched->req.x == 376); + REQUIRE(fetched->req.y == 377); + REQUIRE(fetched->duplicates != NULL); + REQUIRE(fetched->duplicates->req.cmd == cmdRenderPrio); + REQUIRE(fetched->duplicates->req.x == 378); + REQUIRE(fetched->duplicates->req.y == 379); + request_queue_remove_request(queue, fetched, 0); + free(fetched->duplicates); + free(fetched); + + request_queue_close(queue); + } + + SECTION("renderd/queueing/options keep requests distinct", "test option variants are not deduplicated or promoted together") { + enum protoCmd res; + struct item *dirty; + struct item *duplicate; + struct item *fetched; + request_queue *queue = request_queue_init_with_limits(2, 4); + + REQUIRE(queue != NULL); + + dirty = init_render_request(cmdDirty); + dirty->mx = 325; + strcpy(dirty->req.options, "first"); + res = request_queue_add_request(queue, dirty); + REQUIRE(res == cmdNotDone); + REQUIRE(request_queue_no_requests_queued(queue, cmdDirty) == 1); + + duplicate = init_render_request(cmdRender); + duplicate->mx = 325; + duplicate->fd = 46; + strcpy(duplicate->req.options, "second"); + res = request_queue_add_request(queue, duplicate); + REQUIRE(res == cmdIgnore); + REQUIRE(request_queue_no_requests_queued(queue, cmdRender) == 1); + REQUIRE(request_queue_no_requests_queued(queue, cmdDirty) == 1); + + fetched = request_queue_fetch_request(queue); + REQUIRE(fetched == duplicate); + REQUIRE((std::string)fetched->req.options == "second"); + request_queue_remove_request(queue, fetched, 0); + free(fetched); + + fetched = request_queue_fetch_request(queue); + REQUIRE(fetched == dirty); + REQUIRE((std::string)fetched->req.options == "first"); + request_queue_remove_request(queue, fetched, 0); + free(fetched); + + request_queue_close(queue); + } + + SECTION("renderd/queueing/full-width options keep requests distinct", "test fixed-width option fields do not require NUL termination") { + enum protoCmd res; + struct item *first; + struct item *second; + struct item *fetched; + request_queue *queue = request_queue_init_with_limits(2, 4); + + REQUIRE(queue != NULL); + + first = init_render_request(cmdRender); + first->mx = 326; + memset(first->req.options, 'a', sizeof(first->req.options)); + res = request_queue_add_request(queue, first); + REQUIRE(res == cmdIgnore); + + second = init_render_request(cmdRender); + second->mx = 326; + memset(second->req.options, 'a', sizeof(second->req.options)); + second->req.options[sizeof(second->req.options) - 1] = 'b'; + res = request_queue_add_request(queue, second); + REQUIRE(res == cmdIgnore); + REQUIRE(request_queue_no_requests_queued(queue, cmdRender) == 2); + + fetched = request_queue_fetch_request(queue); + REQUIRE(fetched == first); + REQUIRE(memcmp(fetched->req.options, first->req.options, sizeof(fetched->req.options)) == 0); + request_queue_remove_request(queue, fetched, 0); + free(fetched); + + fetched = request_queue_fetch_request(queue); + REQUIRE(fetched == second); + REQUIRE(memcmp(fetched->req.options, second->req.options, sizeof(fetched->req.options)) == 0); + request_queue_remove_request(queue, fetched, 0); + free(fetched); + + request_queue_close(queue); + } + + SECTION("renderd/queueing/dirty duplicate does not demote bulk request", "test background work does not replace already queued bulk work") { + enum protoCmd res; + struct item *bulk; + struct item *dirty; + struct item *fetched; + request_queue *queue = request_queue_init_with_limits(2, 4); + + REQUIRE(queue != NULL); + + bulk = init_render_request(cmdRenderBulk); + bulk->mx = 350; + bulk->fd = 45; + res = request_queue_add_request(queue, bulk); + REQUIRE(res == cmdIgnore); + REQUIRE(request_queue_no_requests_queued(queue, cmdRenderBulk) == 1); + + dirty = init_render_request(cmdDirty); + dirty->mx = 350; + res = request_queue_add_request(queue, dirty); + REQUIRE(res == cmdNotDone); + REQUIRE(request_queue_no_requests_queued(queue, cmdRenderBulk) == 1); + REQUIRE(request_queue_no_requests_queued(queue, cmdDirty) == 0); + + fetched = request_queue_fetch_request(queue); + REQUIRE(fetched == bulk); + REQUIRE(fetched->req.cmd == cmdRenderBulk); + REQUIRE(fetched->fd == 45); + request_queue_remove_request(queue, fetched, 0); + free(fetched); + + request_queue_close(queue); + } + SECTION("renderd/queueing/overflow requests", "test if requests correctly overflow from one request priority to the next") { enum protoCmd res; struct item *item; @@ -593,6 +838,36 @@ TEST_CASE("renderd/queueing", "request queueing") request_queue_close(queue); } + + SECTION("renderd/queueing/clear fd skips dirty backlog but reaches bulk", "Test fd clearing avoids fd-less dirty entries") { + struct request_queue *queue = request_queue_init_with_limits(4, 4); + struct item *item; + + for (int i = 0; i < 4; i++) { + item = init_render_request(cmdDirty); + request_queue_add_request(queue, item); + } + + item = init_render_request(cmdRenderBulk); + item->fd = 47; + request_queue_add_request(queue, item); + request_queue_clear_requests_by_fd(queue, 47); + + for (int i = 0; i < 4; i++) { + item = request_queue_fetch_request(queue); + REQUIRE(item->req.cmd == cmdDirty); + request_queue_remove_request(queue, item, 0); + free(item); + } + + item = request_queue_fetch_request(queue); + REQUIRE(item->req.cmd == cmdRenderBulk); + REQUIRE(item->fd == FD_INVALID); + request_queue_remove_request(queue, item, 0); + free(item); + + request_queue_close(queue); + } } TEST_CASE("renderd", "tile generation") From 1650afbfcee8c16c0e33bbc021b63460fdcfb019 Mon Sep 17 00:00:00 2001 From: Darafei Praliaskouski Date: Sun, 17 May 2026 18:50:34 +0400 Subject: [PATCH 3/3] fix: repair config templates and queue add status race --- CMakeLists.txt | 6 ++++-- configure.ac | 1 + includes/config.h.cmake.in | 32 ++++++++++++++++++++++++++++++++ includes/config.h.in | 33 +++++++++++++-------------------- src/request_queue.c | 8 +++++++- 5 files changed, 57 insertions(+), 23 deletions(-) create mode 100644 includes/config.h.cmake.in diff --git a/CMakeLists.txt b/CMakeLists.txt index ffbc85e7..30752912 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -84,6 +84,8 @@ include(CheckIncludeFile) check_include_file(paths.h HAVE_PATHS_H) check_include_file(sys/cdefs.h HAVE_SYS_CDEFS_H) check_include_file(sys/loadavg.h HAVE_SYS_LOADAVG_H) +check_include_file(iniparser.h HAVE_INIPARSER_H) +check_include_file(iniparser/iniparser.h HAVE_INIPARSER_INIPARSER_H) # Libraries include(CheckLibraryExists) @@ -181,9 +183,9 @@ set(VERSION "${PROJECT_VERSION}") # #----------------------------------------------------------------------------- -# include/config.h.in +# include/config.h.cmake.in configure_file( - ${PROJECT_SOURCE_DIR}/includes/config.h.in + ${PROJECT_SOURCE_DIR}/includes/config.h.cmake.in ${PROJECT_SOURCE_DIR}/includes/config.h ) diff --git a/configure.ac b/configure.ac index b024890b..88ebcb92 100644 --- a/configure.ac +++ b/configure.ac @@ -9,6 +9,7 @@ AX_CONFIG_NICE AC_INIT([mod_tile], [mod_tile_version], [http://trac.openstreetmap.org]) +AC_DEFINE_UNQUOTED([VERSION], ["$PACKAGE_VERSION"], [Version number of project]) AM_INIT_AUTOMAKE([subdir-objects]) LT_INIT AC_CONFIG_SRCDIR([src/mod_tile.c]) diff --git a/includes/config.h.cmake.in b/includes/config.h.cmake.in new file mode 100644 index 00000000..b5670485 --- /dev/null +++ b/includes/config.h.cmake.in @@ -0,0 +1,32 @@ +#ifndef CONFIG_H +#define CONFIG_H +/* Define to 1 if you have the functions. */ +#cmakedefine HAVE_DAEMON @HAVE_DAEMON@ +#cmakedefine HAVE_GETLOADAVG @HAVE_GETLOADAVG@ + +/* Define to 1 if you have the header files. */ +#cmakedefine HAVE_PATHS_H @HAVE_PATHS_H@ +#cmakedefine HAVE_PTHREAD @HAVE_PTHREAD@ +#cmakedefine HAVE_SYS_CDEFS_H @HAVE_SYS_CDEFS_H@ +#cmakedefine HAVE_SYS_LOADAVG_H @HAVE_SYS_LOADAVG_H@ +#cmakedefine HAVE_INIPARSER_H @HAVE_INIPARSER_H@ +#cmakedefine HAVE_INIPARSER_INIPARSER_H @HAVE_INIPARSER_INIPARSER_H@ + +/* Define to 1 if you have the libraries. */ +#cmakedefine HAVE_CAIRO @HAVE_CAIRO@ +#cmakedefine HAVE_LIBCURL @HAVE_LIBCURL@ +#cmakedefine HAVE_LIBMEMCACHED @HAVE_LIBMEMCACHED@ +#cmakedefine HAVE_LIBRADOS @HAVE_LIBRADOS@ + +/* Define configuration options. */ +#cmakedefine MAPNIK_FONTS_DIR "@MAPNIK_FONTS_DIR@" +#cmakedefine MAPNIK_FONTS_DIR_RECURSE "@MAPNIK_FONTS_DIR_RECURSE@" +#cmakedefine MAPNIK_PLUGINS_DIR "@MAPNIK_PLUGINS_DIR@" +#cmakedefine RENDERD_CONFIG "@RENDERD_CONFIG@" +#cmakedefine RENDERD_PIDFILE "@RENDERD_PIDFILE@" +#cmakedefine RENDERD_SOCKET "@RENDERD_SOCKET@" +#cmakedefine RENDERD_TILE_DIR "@RENDERD_TILE_DIR@" + +/* Version number of project */ +#cmakedefine VERSION "@VERSION@" +#endif diff --git a/includes/config.h.in b/includes/config.h.in index 6e16c6ce..8d05661c 100644 --- a/includes/config.h.in +++ b/includes/config.h.in @@ -1,30 +1,23 @@ #ifndef CONFIG_H #define CONFIG_H /* Define to 1 if you have the functions. */ -#cmakedefine HAVE_DAEMON @HAVE_DAEMON@ -#cmakedefine HAVE_GETLOADAVG @HAVE_GETLOADAVG@ +#undef HAVE_DAEMON +#undef HAVE_GETLOADAVG /* Define to 1 if you have the header files. */ -#cmakedefine HAVE_PATHS_H @HAVE_PATHS_H@ -#cmakedefine HAVE_PTHREAD @HAVE_PTHREAD@ -#cmakedefine HAVE_SYS_CDEFS_H @HAVE_SYS_CDEFS_H@ -#cmakedefine HAVE_SYS_LOADAVG_H @HAVE_SYS_LOADAVG_H@ +#undef HAVE_PATHS_H +#undef HAVE_PTHREAD +#undef HAVE_SYS_CDEFS_H +#undef HAVE_SYS_LOADAVG_H +#undef HAVE_INIPARSER_H +#undef HAVE_INIPARSER_INIPARSER_H /* Define to 1 if you have the libraries. */ -#cmakedefine HAVE_CAIRO @HAVE_CAIRO@ -#cmakedefine HAVE_LIBCURL @HAVE_LIBCURL@ -#cmakedefine HAVE_LIBMEMCACHED @HAVE_LIBMEMCACHED@ -#cmakedefine HAVE_LIBRADOS @HAVE_LIBRADOS@ - -/* Define configuration options. */ -#cmakedefine MAPNIK_FONTS_DIR "@MAPNIK_FONTS_DIR@" -#cmakedefine MAPNIK_FONTS_DIR_RECURSE "@MAPNIK_FONTS_DIR_RECURSE@" -#cmakedefine MAPNIK_PLUGINS_DIR "@MAPNIK_PLUGINS_DIR@" -#cmakedefine RENDERD_CONFIG "@RENDERD_CONFIG@" -#cmakedefine RENDERD_PIDFILE "@RENDERD_PIDFILE@" -#cmakedefine RENDERD_SOCKET "@RENDERD_SOCKET@" -#cmakedefine RENDERD_TILE_DIR "@RENDERD_TILE_DIR@" +#undef HAVE_CAIRO +#undef HAVE_LIBCURL +#undef HAVE_LIBMEMCACHED +#undef HAVE_LIBRADOS /* Version number of project */ -#cmakedefine VERSION "@VERSION@" +#undef VERSION #endif diff --git a/src/request_queue.c b/src/request_queue.c index 8d4db530..970f472c 100644 --- a/src/request_queue.c +++ b/src/request_queue.c @@ -505,6 +505,7 @@ enum protoCmd request_queue_add_request(struct request_queue * queue, struct ite { enum protoCmd status; const struct protocol *req; + enum protoCmd add_status = cmdIgnore; int item_added = 0; req = &(item->req); @@ -557,6 +558,7 @@ enum protoCmd request_queue_add_request(struct request_queue * queue, struct ite item->originatedQueue = queueDirty; queue->dirtyNum++; item->fd = FD_INVALID; // No response after render + add_status = cmdNotDone; item_added = 1; } else { // The queue is severely backlogged. Drop request @@ -580,7 +582,7 @@ enum protoCmd request_queue_add_request(struct request_queue * queue, struct ite pthread_mutex_unlock(&queue->qLock); - return (item->inQueue == queueDirty) ? cmdNotDone : cmdIgnore; + return add_status; } void request_queue_remove_request(struct request_queue * queue, struct item * request, int render_time) @@ -716,17 +718,21 @@ struct request_queue * request_queue_init_with_limits(int request_limit, int dir queue->pendingHead.next = queue->pendingHead.prev = &(queue->pendingHead); queue->renderHead.next = queue->renderHead.prev = &(queue->renderHead); + for (int i = 0; i < PENDING_QUEUE_RANKS; i++) { queue->pendingTail[i] = &(queue->pendingHead); } + queue->hashidxSize = request_queue_hash_size(request_limit, dirty_limit); queue->item_hashidx = (struct item_idx *) malloc(sizeof(struct item_idx) * queue->hashidxSize); + if (queue->item_hashidx == NULL) { g_logger(G_LOG_LEVEL_ERROR, "Failed to initialise request queue index with %i entries", queue->hashidxSize); pthread_mutex_destroy(&(queue->qLock)); free(queue); return NULL; } + bzero(queue->item_hashidx, sizeof(struct item_idx) * queue->hashidxSize); return queue;