From 6af358988832266d2fd3b6752696f676fe1cb02c Mon Sep 17 00:00:00 2001 From: Mats Kindahl Date: Mon, 8 Apr 2024 20:19:00 +0200 Subject: [PATCH] Add telemetry for access methods Add telemetry for tracking access methods used, number of pages for each access method, and number of instances using each access method. Also introduces a type-based function `ts_jsonb_set_value_by_type` that can generate correct JSONB based on the PostgreSQL type. It will generate "bare" values for numerics, and strings for anything else using the output function for the type. To test this for string values, we update `ts_jsonb_add_interval` to use this new function, which is calling the output function for the type, just like `ts_jsonb_set_value_by_type`. --- .github/workflows/code_style.yaml | 2 +- .unreleased/pr_6810 | 1 + src/jsonb_utils.c | 52 +++++++++++++++-- src/jsonb_utils.h | 1 + src/telemetry/telemetry.c | 96 +++++++++++++++++++++++++++++++ test/expected/telemetry.out | 14 ++++- test/sql/telemetry.sql | 7 +++ 7 files changed, 167 insertions(+), 6 deletions(-) create mode 100644 .unreleased/pr_6810 diff --git a/.github/workflows/code_style.yaml b/.github/workflows/code_style.yaml index 0511270d18e..b1b349a9021 100644 --- a/.github/workflows/code_style.yaml +++ b/.github/workflows/code_style.yaml @@ -66,7 +66,7 @@ jobs: - name: Run codespell run: | find . -type f \( -name "*.c" -or -name "*.h" -or -name "*.yaml" -or -name "*.sh" \) \ - -exec codespell -L "inh,larg,inout" {} \+ + -exec codespell -L "brin,inh,larg,inout" {} \+ cc_checks: name: Check code formatting diff --git a/.unreleased/pr_6810 b/.unreleased/pr_6810 new file mode 100644 index 00000000000..821d29c97fa --- /dev/null +++ b/.unreleased/pr_6810 @@ -0,0 +1 @@ +Implements: #6810 Add telemetry for access methods diff --git a/src/jsonb_utils.c b/src/jsonb_utils.c index 9da4602991a..685ce6a7275 100644 --- a/src/jsonb_utils.c +++ b/src/jsonb_utils.c @@ -57,6 +57,51 @@ ts_jsonb_add_str(JsonbParseState *state, const char *key, const char *value) ts_jsonb_add_value(state, key, &json_value); } +static PGFunction +get_convert_func(Oid typeid) +{ + switch (typeid) + { + case INT2OID: + return int2_numeric; + case INT4OID: + return int4_numeric; + case INT8OID: + return int8_numeric; + default: + return NULL; + } +} + +void +ts_jsonb_set_value_by_type(JsonbValue *value, Oid typeid, Datum datum) +{ + switch (typeid) + { + Oid typeOut; + bool isvarlena; + char *str; + PGFunction func; + + case INT2OID: + case INT4OID: + case INT8OID: + case NUMERICOID: + func = get_convert_func(typeid); + value->type = jbvNumeric; + value->val.numeric = DatumGetNumeric(func ? DirectFunctionCall1(func, datum) : datum); + break; + + default: + getTypeOutputInfo(typeid, &typeOut, &isvarlena); + str = OidOutputFunctionCall(typeOut, datum); + value->type = jbvString; + value->val.string.val = str; + value->val.string.len = strlen(str); + break; + } +} + void ts_jsonb_add_int32(JsonbParseState *state, const char *key, const int32 int_value) { @@ -80,11 +125,10 @@ ts_jsonb_add_int64(JsonbParseState *state, const char *key, const int64 int_valu void ts_jsonb_add_interval(JsonbParseState *state, const char *key, Interval *interval) { - char *value; - - value = DatumGetCString(DirectFunctionCall1(interval_out, IntervalPGetDatum(interval))); + JsonbValue json_value; - ts_jsonb_add_str(state, key, value); + ts_jsonb_set_value_by_type(&json_value, INTERVALOID, IntervalPGetDatum(interval)); + ts_jsonb_add_value(state, key, &json_value); } void diff --git a/src/jsonb_utils.h b/src/jsonb_utils.h index e9082cbd90c..98ecc6f7347 100644 --- a/src/jsonb_utils.h +++ b/src/jsonb_utils.h @@ -23,6 +23,7 @@ extern TSDLLEXPORT void ts_jsonb_add_int64(JsonbParseState *state, const char *k const int64 value); extern TSDLLEXPORT void ts_jsonb_add_numeric(JsonbParseState *state, const char *key, const Numeric value); +extern TSDLLEXPORT void ts_jsonb_set_value_by_type(JsonbValue *value, Oid typeid, Datum datum); extern void ts_jsonb_add_value(JsonbParseState *state, const char *key, JsonbValue *value); diff --git a/src/telemetry/telemetry.c b/src/telemetry/telemetry.c index 7a155e585cf..fcba309ff42 100644 --- a/src/telemetry/telemetry.c +++ b/src/telemetry/telemetry.c @@ -757,6 +757,93 @@ add_replication_telemetry(JsonbParseState *state) #define REQ_RELS_CONTINUOUS_AGGS "continuous_aggregates" #define REQ_FUNCTIONS_USED "functions_used" #define REQ_REPLICATION "replication" +#define REQ_ACCESS_METHODS "access_methods" + +/* + * Add the result of a query as a sub-object to the JSONB. + * + * Each row from the query generates a separate object keyed by one of the + * columns. Each row will be represented as an object and stored under the + * "key" column. For example, with this query: + * + * select amname as name, + * sum(relpages) as pages, + * count(*) as instances + * from pg_class join pg_am on relam = pg_am.oid + * group by pg_am.oid; + * + * might generate the object + * + * { + * "brin" : { + * "instances" : 44, + * "pages" : 432 + * }, + * "btree" : { + * "instances" : 99, + * "pages" : 1234 + * } + * } + */ +static void +add_query_result_dict(JsonbParseState *state, const char *query) +{ + MemoryContext orig_context = CurrentMemoryContext; + + int res; + if (SPI_connect() != SPI_OK_CONNECT) + elog(ERROR, "could not connect to SPI"); + + /* Lock down search_path */ + res = SPI_execute("SET LOCAL search_path TO pg_catalog, pg_temp", false, 0); + Ensure(res >= 0, "could not set search path"); + + res = SPI_execute(query, true, 0); + Ensure(res >= 0, "could not execute query"); + + MemoryContext spi_context = MemoryContextSwitchTo(orig_context); + + (void) pushJsonbValue(&state, WJB_BEGIN_OBJECT, NULL); + for (uint64 r = 0; r < SPI_processed; r++) + { + char *key_string = SPI_getvalue(SPI_tuptable->vals[r], SPI_tuptable->tupdesc, 1); + JsonbValue key = { + .type = jbvString, + .val.string.val = pstrdup(key_string), + .val.string.len = strlen(key_string), + }; + + (void) pushJsonbValue(&state, WJB_KEY, &key); + + (void) pushJsonbValue(&state, WJB_BEGIN_OBJECT, NULL); + for (int c = 1; c < SPI_tuptable->tupdesc->natts; ++c) + { + bool isnull; + Datum val_datum = + SPI_getbinval(SPI_tuptable->vals[r], SPI_tuptable->tupdesc, c + 1, &isnull); + if (!isnull) + { + char *key_string = SPI_fname(SPI_tuptable->tupdesc, c + 1); + JsonbValue key = { + .type = jbvString, + .val.string.val = pstrdup(key_string), + .val.string.len = strlen(key_string), + }; + JsonbValue value; + ts_jsonb_set_value_by_type(&value, + SPI_gettypeid(SPI_tuptable->tupdesc, c + 1), + val_datum); + (void) pushJsonbValue(&state, WJB_KEY, &key); + (void) pushJsonbValue(&state, WJB_VALUE, &value); + } + } + pushJsonbValue(&state, WJB_END_OBJECT, NULL); + } + MemoryContextSwitchTo(spi_context); + res = SPI_finish(); + Assert(res == SPI_OK_FINISH); + (void) pushJsonbValue(&state, WJB_END_OBJECT, NULL); +} static Jsonb * build_telemetry_report() @@ -937,6 +1024,15 @@ build_telemetry_report() add_replication_telemetry(parse_state); pushJsonbValue(&parse_state, WJB_END_OBJECT, NULL); + key.type = jbvString; + key.val.string.val = REQ_ACCESS_METHODS; + key.val.string.len = strlen(REQ_ACCESS_METHODS); + (void) pushJsonbValue(&parse_state, WJB_KEY, &key); + add_query_result_dict(parse_state, + "SELECT amname AS name, sum(relpages) AS pages, count(*) AS " + "instances FROM pg_class JOIN pg_am ON relam = pg_am.oid " + "GROUP BY amname"); + /* end of telemetry object */ result = pushJsonbValue(&parse_state, WJB_END_OBJECT, NULL); diff --git a/test/expected/telemetry.out b/test/expected/telemetry.out index a8a5de53d64..d287e22d68b 100644 --- a/test/expected/telemetry.out +++ b/test/expected/telemetry.out @@ -350,6 +350,7 @@ WHERE key != 'os_name_pretty'; db_metadata replication build_os_name + access_methods functions_used install_method installed_time @@ -377,7 +378,7 @@ WHERE key != 'os_name_pretty'; num_compression_policies_fixed num_user_defined_actions_fixed num_continuous_aggs_policies_fixed -(37 rows) +(38 rows) CREATE MATERIALIZED VIEW telemetry_report AS SELECT t FROM get_telemetry_report() t; @@ -395,6 +396,17 @@ SELECT t -> 'instance_metadata' FROM telemetry_report; {"cloud": "ci"} (1 row) +-- Check access methods +SELECT t->'access_methods' ? 'btree', + t->'access_methods' ? 'heap', + CAST(t->'access_methods'->'btree'->'pages' AS int) > 0, + CAST(t->'access_methods'->'btree'->'instances' AS int) > 0 + FROM telemetry_report; + ?column? | ?column? | ?column? | ?column? +----------+----------+----------+---------- + t | t | t | t +(1 row) + WITH t AS ( SELECT t -> 'relations' AS rels FROM telemetry_report diff --git a/test/sql/telemetry.sql b/test/sql/telemetry.sql index db8773656b1..a18ebc3ace8 100644 --- a/test/sql/telemetry.sql +++ b/test/sql/telemetry.sql @@ -166,6 +166,13 @@ SELECT t -> 'db_metadata' FROM telemetry_report; -- check timescaledb_telemetry.cloud SELECT t -> 'instance_metadata' FROM telemetry_report; +-- Check access methods +SELECT t->'access_methods' ? 'btree', + t->'access_methods' ? 'heap', + CAST(t->'access_methods'->'btree'->'pages' AS int) > 0, + CAST(t->'access_methods'->'btree'->'instances' AS int) > 0 + FROM telemetry_report; + WITH t AS ( SELECT t -> 'relations' AS rels FROM telemetry_report