From fd66f5936abb6f9271a713e15002be3c4854fb39 Mon Sep 17 00:00:00 2001 From: Alexander Kuzmenkov <36882414+akuzm@users.noreply.github.com> Date: Thu, 9 Feb 2023 12:55:07 +0400 Subject: [PATCH] Warn about mismatched chunk cache sizes Just noticed abysmal INSERT performance when experimenting with one of our customers' data set, and turns out my cache sizes were misconfigured, leading to constant hypertable chunk cache thrashing. Show a warning to detect this misconfiguration. Also use more generous defaults, we're not supposed to run on a microwave (unlike Postgres). --- coccinelle/ereport_pg12.cocci | 10 +++- src/guc.c | 57 +++++++++++++++---- test/expected/insert_single.out | 16 +++++- test/sql/insert_single.sql | 8 +++ tsl/test/expected/dist_copy_available_dns.out | 1 + tsl/test/expected/dist_copy_long.out | 1 + 6 files changed, 80 insertions(+), 13 deletions(-) diff --git a/coccinelle/ereport_pg12.cocci b/coccinelle/ereport_pg12.cocci index 9544f6fe2..69c790136 100644 --- a/coccinelle/ereport_pg12.cocci +++ b/coccinelle/ereport_pg12.cocci @@ -11,6 +11,14 @@ expression E1, E2; // We pass two or more expressions to ereport -+ /* ereport uses PG 12.3+ syntax */ ++ /* ++ * Please enclose the auxiliary ereport arguments into parentheses for ++ * compatibility with PG 12. Example: ++ * ++ * ereport(ERROR, ( errmsg(...), errdetail(...) ) ); ++ * ^-----------add these---------^ ++ * ++ * See https://github.com/postgres/postgres/commit/a86715451653c730d637847b403b0420923956f7 ++ */ ereport(K1, E1, E2, ...); diff --git a/src/guc.c b/src/guc.c index cb8d578e9..dc2c48cd5 100644 --- a/src/guc.c +++ b/src/guc.c @@ -107,6 +107,12 @@ bool ts_shutdown_bgw = false; char *ts_current_timestamp_mock = ""; #endif +/* + * We have to understand if we have finished initializing the GUCs, so that we + * know when it's OK to check their values for mutual consistency. + */ +static bool gucs_are_initialized = false; + /* Hook for plugins to allow additional SSL options */ set_ssl_options_hook_type ts_set_ssl_options_hook = NULL; @@ -117,11 +123,44 @@ ts_assign_ssl_options_hook(void *fn) ts_set_ssl_options_hook = (set_ssl_options_hook_type) fn; } +/* + * Warn about the mismatched cache sizes that can lead to cache thrashing. + */ +static void +validate_chunk_cache_sizes(int hypertable_chunks, int insert_chunks) +{ + /* + * Note that this callback is also called when the individual GUCs are + * initialized, so we are going to see temporary mismatched values here. + * That's why we also have to check that the GUC initialization have + * finished. + */ + if (gucs_are_initialized && insert_chunks > hypertable_chunks) + { + ereport(WARNING, + (errmsg("insert cache size is larger than hypertable chunk cache size"), + errdetail("insert cache size is %d, hypertable chunk cache size is %d", + insert_chunks, + hypertable_chunks), + errhint("This is a configuration problem. Either increase " + "timescaledb.max_cached_chunks_per_hypertable (preferred) or decrease " + "timescaledb.max_open_chunks_per_insert."))); + } +} + static void assign_max_cached_chunks_per_hypertable_hook(int newval, void *extra) { /* invalidate the hypertable cache to reset */ ts_hypertable_cache_invalidate_callback(); + + validate_chunk_cache_sizes(newval, ts_guc_max_open_chunks_per_insert); +} + +static void +assign_max_open_chunks_per_insert_hook(int newval, void *extra) +{ + validate_chunk_cache_sizes(ts_guc_max_cached_chunks_per_hypertable, newval); } void @@ -453,27 +492,20 @@ _guc_init(void) "Maximum open chunks per insert", "Maximum number of open chunk tables per insert", &ts_guc_max_open_chunks_per_insert, - Min(work_mem * INT64CONST(1024) / INT64CONST(25000), - PG_INT16_MAX), /* Measurements via - * `MemoryContextStats(TopMemoryContext)` - * show chunk insert - * state memory context - * takes up ~25K bytes - * (work_mem is in - * kbytes) */ + 1024, 0, PG_INT16_MAX, PGC_USERSET, 0, NULL, - NULL, + assign_max_open_chunks_per_insert_hook, NULL); DefineCustomIntVariable("timescaledb.max_cached_chunks_per_hypertable", "Maximum cached chunks", "Maximum number of chunks stored in the cache", &ts_guc_max_cached_chunks_per_hypertable, - 100, + 1024, 0, 65536, PGC_USERSET, @@ -591,6 +623,11 @@ _guc_init(void) NULL, NULL, NULL); + + gucs_are_initialized = true; + + validate_chunk_cache_sizes(ts_guc_max_cached_chunks_per_hypertable, + ts_guc_max_open_chunks_per_insert); } void diff --git a/test/expected/insert_single.out b/test/expected/insert_single.out index d5467f41c..24edc64ac 100644 --- a/test/expected/insert_single.out +++ b/test/expected/insert_single.out @@ -479,6 +479,15 @@ INSERT INTO "nondefault_mem_settings" VALUES ('2001-03-20T09:00:00', 30.6), ('2002-03-20T09:00:00', 31.9), ('2003-03-20T09:00:00', 32.9); +--warning about mismatched cache sizes +SET timescaledb.max_open_chunks_per_insert = 100; +WARNING: insert cache size is larger than hypertable chunk cache size +SET timescaledb.max_cached_chunks_per_hypertable = 10; +WARNING: insert cache size is larger than hypertable chunk cache size +INSERT INTO "nondefault_mem_settings" VALUES +('2001-05-20T09:00:00', 36.6), +('2002-05-20T09:00:00', 37.9), +('2003-05-20T09:00:00', 38.9); --unlimited SET timescaledb.max_open_chunks_per_insert = 0; SET timescaledb.max_cached_chunks_per_hypertable = 0; @@ -497,10 +506,13 @@ SELECT * FROM "nondefault_mem_settings"; Tue Mar 20 09:00:00 2001 | 30.6 Wed Mar 20 09:00:00 2002 | 31.9 Thu Mar 20 09:00:00 2003 | 32.9 + Sun May 20 09:00:00 2001 | 36.6 + Mon May 20 09:00:00 2002 | 37.9 + Tue May 20 09:00:00 2003 | 38.9 Fri Apr 20 09:00:00 2001 | 33.6 Sat Apr 20 09:00:00 2002 | 34.9 Sun Apr 20 09:00:00 2003 | 35.9 -(11 rows) +(14 rows) --test rollback BEGIN; @@ -527,7 +539,7 @@ SAVEPOINT savepoint_2; SAVEPOINT \set ON_ERROR_STOP 0 INSERT INTO "data_records" ("time", "value") VALUES (3, -1); -ERROR: new row for relation "_hyper_13_34_chunk" violates check constraint "data_records_value_check" +ERROR: new row for relation "_hyper_13_37_chunk" violates check constraint "data_records_value_check" \set ON_ERROR_STOP 1 ROLLBACK TO SAVEPOINT savepoint_2; ROLLBACK diff --git a/test/sql/insert_single.sql b/test/sql/insert_single.sql index 8e50fe91c..d652f65ad 100644 --- a/test/sql/insert_single.sql +++ b/test/sql/insert_single.sql @@ -154,6 +154,14 @@ INSERT INTO "nondefault_mem_settings" VALUES ('2002-03-20T09:00:00', 31.9), ('2003-03-20T09:00:00', 32.9); +--warning about mismatched cache sizes +SET timescaledb.max_open_chunks_per_insert = 100; +SET timescaledb.max_cached_chunks_per_hypertable = 10; +INSERT INTO "nondefault_mem_settings" VALUES +('2001-05-20T09:00:00', 36.6), +('2002-05-20T09:00:00', 37.9), +('2003-05-20T09:00:00', 38.9); + --unlimited SET timescaledb.max_open_chunks_per_insert = 0; SET timescaledb.max_cached_chunks_per_hypertable = 0; diff --git a/tsl/test/expected/dist_copy_available_dns.out b/tsl/test/expected/dist_copy_available_dns.out index 6720e64e0..d86cf82c7 100644 --- a/tsl/test/expected/dist_copy_available_dns.out +++ b/tsl/test/expected/dist_copy_available_dns.out @@ -111,6 +111,7 @@ SELECT * FROM chunk_query_data_node WHERE hypertable_name = 'uk_price_paid' LIMI (5 rows) set timescaledb.max_open_chunks_per_insert = 1117; +WARNING: insert cache size is larger than hypertable chunk cache size SET ROLE :ROLE_CLUSTER_SUPERUSER; SELECT * FROM alter_data_node('data_node_1', available=>true); node_name | host | port | database | available diff --git a/tsl/test/expected/dist_copy_long.out b/tsl/test/expected/dist_copy_long.out index bda05abba..ad814d62d 100644 --- a/tsl/test/expected/dist_copy_long.out +++ b/tsl/test/expected/dist_copy_long.out @@ -116,6 +116,7 @@ select count(*) from uk_price_paid; truncate uk_price_paid; set timescaledb.max_open_chunks_per_insert = 1117; +WARNING: insert cache size is larger than hypertable chunk cache size \copy uk_price_paid from program 'zcat < data/prices-100k-random-1.tsv.gz'; select count(*) from uk_price_paid; count