This is an automated email from the ASF dual-hosted git repository. maxyang pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/cloudberry.git
commit 60802217fa0d211d5d77a3a4a809c2cca5934420 Author: RMT <[email protected]> AuthorDate: Tue Sep 12 07:32:19 2023 -0600 Io limit improve error log (#16394) When io_limit encountered syntax error, previous log is just "Error: Syntax error". Now, the io_limit has comprehensive log for syntax error: ``` demo=# create resource group rg1 WITH (cpu_max_percent=10, io_limit='pg_defaultrbps=100, wbps=550,riops=1000,wiops=1000'); ERROR: io limit: syntax error, unexpected '=', expecting ':' HINT: pg_defaultrbps=100, wbps=550,riops=1000,wiops=1000 ^ ``` ``` demo=# create resource group rg1 WITH (cpu_max_percent=10, io_limit='pg_default: rbps=100wbps=550,riops=1000, wiops=1000'); ERROR: io limit: syntax error, unexpected IO_KEY, expecting end of file or ';' HINT: pg_default:rbps=100wbps=550,riops=1000,wiops=1000 ^ ``` --- src/backend/utils/resgroup/cgroup-ops-linux-v2.c | 8 +- src/backend/utils/resgroup/cgroup_io_limit.c | 43 +++++----- src/backend/utils/resgroup/io_limit_gram.y | 92 +++++++++++++--------- src/backend/utils/resgroup/io_limit_scanner.l | 51 ++++++++---- src/include/utils/cgroup_io_limit.h | 6 +- .../input/resgroup/resgroup_io_limit.source | 5 ++ .../output/resgroup/resgroup_io_limit.source | 23 ++++-- 7 files changed, 142 insertions(+), 86 deletions(-) diff --git a/src/backend/utils/resgroup/cgroup-ops-linux-v2.c b/src/backend/utils/resgroup/cgroup-ops-linux-v2.c index 933f8b139ef..69d2b1b0461 100644 --- a/src/backend/utils/resgroup/cgroup-ops-linux-v2.c +++ b/src/backend/utils/resgroup/cgroup-ops-linux-v2.c @@ -857,22 +857,22 @@ setio_v2(Oid group, List *limit_list) { TblSpcIOLimit *limit = (TblSpcIOLimit *)lfirst(tblspc_cell); - if (limit->ioconfig->rbps == IO_LIMIT_MAX ) + if (limit->ioconfig->rbps == IO_LIMIT_MAX || limit->ioconfig->rbps == IO_LIMIT_EMPTY) sprintf(rbps_str, "rbps=max"); else sprintf(rbps_str, "rbps=%lu", limit->ioconfig->rbps * 1024 * 1024); - if (limit->ioconfig->wbps == IO_LIMIT_MAX) + if (limit->ioconfig->wbps == IO_LIMIT_MAX || limit->ioconfig->wbps == IO_LIMIT_EMPTY) sprintf(wbps_str, "wbps=max"); else sprintf(wbps_str, "wbps=%lu", limit->ioconfig->wbps * 1024 * 1024); - if (limit->ioconfig->riops == IO_LIMIT_MAX) + if (limit->ioconfig->riops == IO_LIMIT_MAX || limit->ioconfig->riops == IO_LIMIT_EMPTY) sprintf(riops_str, "riops=max"); else sprintf(riops_str, "riops=%u", (uint32)limit->ioconfig->riops); - if (limit->ioconfig->wiops == IO_LIMIT_MAX) + if (limit->ioconfig->wiops == IO_LIMIT_MAX || limit->ioconfig->wiops == IO_LIMIT_EMPTY) sprintf(wiops_str, "wiops=max"); else sprintf(wiops_str, "wiops=%u", (uint32)limit->ioconfig->wiops); diff --git a/src/backend/utils/resgroup/cgroup_io_limit.c b/src/backend/utils/resgroup/cgroup_io_limit.c index 29737a25ff8..43e096301d2 100644 --- a/src/backend/utils/resgroup/cgroup_io_limit.c +++ b/src/backend/utils/resgroup/cgroup_io_limit.c @@ -45,7 +45,6 @@ const char *IOconfigFields[4] = {"rbps", "wbps", "riops", "wiops"}; const char *IOStatFields[4] = {"rbytes", "wbytes", "rios", "wios"}; static int bdi_cmp(const void *a, const void *b); -static void ioconfig_validate(IOconfig *config); typedef struct BDICmp { @@ -103,8 +102,6 @@ io_limit_validate(List *limit_list) TblSpcIOLimit *limit = (TblSpcIOLimit *)lfirst(limit_cell); ListCell *bdi_cell; - ioconfig_validate(limit->ioconfig); - foreach (bdi_cell, limit->bdi_list) { bdi_array[i].bdi = *(bdi_t *)lfirst(bdi_cell); @@ -259,6 +256,10 @@ get_bdi_of_path(const char *ori_path) errmsg("cannot find disk of %s, details: %m", path), errhint("mount point of %s is: %s", path, match_mnt.mnt_fsname))); } + pfree(match_mnt.mnt_fsname); + pfree(match_mnt.mnt_dir); + pfree(match_mnt.mnt_type); + pfree(match_mnt.mnt_opts); maj = major(sb.st_rdev); min = minor(sb.st_rdev); @@ -305,31 +306,27 @@ get_bdi_of_path(const char *ori_path) return make_bdi(parent_maj, parent_min); } -static void -ioconfig_validate(IOconfig *config) + +bool +io_limit_value_validate(const char *field, const uint64 value, uint64 *max) { const uint64 ULMAX = ULLONG_MAX / 1024 / 1024; const uint32 UMAX = UINT_MAX; - if (config->rbps != IO_LIMIT_MAX && (config->rbps > ULMAX || config->rbps < 2)) - ereport(ERROR, - (errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg("io limit: rbps must in range [2, %lu] or equal 0", ULMAX))); - - if (config->wbps != IO_LIMIT_MAX && (config->wbps > ULMAX || config->wbps < 2)) - ereport(ERROR, - (errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg("io limit: wbps must in range [2, %lu] or equal 0", ULMAX))); + *max = 0; - if (config->wiops != IO_LIMIT_MAX && (config->wiops > UMAX || config->wiops < 2)) - ereport(ERROR, - (errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg("io limit: wiops must in range [2, %u] or equal 0", UMAX))); + if (strcmp(field, "rbps") == 0 || strcmp(field, "wbps") == 0) + { + *max = ULMAX; + return value == IO_LIMIT_MAX || !(value > ULMAX || value < 2); + } + else if (strcmp(field, "riops") == 0 || strcmp(field, "wiops") == 0) + { + *max = (uint64) UMAX; + return value == IO_LIMIT_MAX || !(value > UMAX || value < 2); + } - if (config->riops != IO_LIMIT_MAX && (config->riops > UMAX || config->riops < 2)) - ereport(ERROR, - (errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg("io limit: riops must in range [2, %u] or equal 0", UMAX))); + return false; } char * @@ -544,7 +541,7 @@ io_limit_dump(List *limit_list) for(i = 0; i < fields_length; i++) { - if (*(value + i) != IO_LIMIT_MAX) + if ((*(value + i) != IO_LIMIT_MAX) && (*(value + i) != IO_LIMIT_EMPTY)) appendStringInfo(result, "%s=%lu", IOconfigFields[i], *(value + i)); else appendStringInfo(result, "%s=max", IOconfigFields[i]); diff --git a/src/backend/utils/resgroup/io_limit_gram.y b/src/backend/utils/resgroup/io_limit_gram.y index e79b35b6f21..6663821c856 100644 --- a/src/backend/utils/resgroup/io_limit_gram.y +++ b/src/backend/utils/resgroup/io_limit_gram.y @@ -1,5 +1,6 @@ %define api.pure true %define api.prefix {io_limit_yy} +%error-verbose %code top { #include "postgres.h" @@ -10,6 +11,9 @@ #define YYMALLOC palloc #define YYFREE pfree + + extern int io_limit_yycolumn; + static char *line; } %code requires { @@ -35,7 +39,7 @@ void io_limit_yyerror(IOLimitParserContext *parser_context, void *scanner, const char *message); } -%token IOLIMIT_CONFIG_DELIM TABLESPACE_IO_CONFIG_START STAR IOCONFIG_DELIM VALUE_MAX +%token STAR VALUE_MAX %token <str> ID IO_KEY %token <integer> NUMBER @@ -43,7 +47,7 @@ %type <integer> io_value %type <ioconfig> ioconfigs %type <tblspciolimit> tablespace_io_config -%type <list> iolimit_config_string start +%type <list> iolimit_config_string %type <ioconfigitem> ioconfig %destructor { pfree($$); } <str> <ioconfig> <ioconfigitem> @@ -55,34 +59,31 @@ %% -start: iolimit_config_string - { - context->result = $$ = $1; - return 0; - } - iolimit_config_string: tablespace_io_config { List *l = NIL; $$ = lappend(l, $1); + + context->result = $$; } - | iolimit_config_string IOLIMIT_CONFIG_DELIM tablespace_io_config + | iolimit_config_string ';' tablespace_io_config { $$ = lappend($1, $3); + + context->result = $$; } +; -tablespace_name: ID { $$ = $1; } +tablespace_name: ID { $$ = $1; } ; -tablespace_io_config: tablespace_name TABLESPACE_IO_CONFIG_START ioconfigs +tablespace_io_config: tablespace_name ':' ioconfigs { TblSpcIOLimit *tblspciolimit = (TblSpcIOLimit *)palloc0(sizeof(TblSpcIOLimit)); if (context->star_tablespace_cnt > 0) - ereport(ERROR, - (errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg("io limit: tablespace '*' cannot be used with other tablespaces"))); + yyerror(NULL, NULL, "tablespace '*' cannot be used with other tablespaces"); tblspciolimit->tablespace_oid = get_tablespace_oid($1, false); context->normal_tablespce_cnt++; @@ -90,14 +91,13 @@ tablespace_io_config: tablespace_name TABLESPACE_IO_CONFIG_START ioconfigs $$ = tblspciolimit; } - | STAR TABLESPACE_IO_CONFIG_START ioconfigs + | STAR ':' ioconfigs { TblSpcIOLimit *tblspciolimit = (TblSpcIOLimit *)palloc0(sizeof(TblSpcIOLimit)); if (context->normal_tablespce_cnt > 0 || context->star_tablespace_cnt > 0) - ereport(ERROR, - (errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg("io limit: tablespace '*' cannot be used with other tablespaces"))); + yyerror(NULL, NULL, "tablespace '*' cannot be used with other tablespaces"); + tblspciolimit->tablespace_oid = InvalidOid; context->star_tablespace_cnt++; @@ -105,18 +105,14 @@ tablespace_io_config: tablespace_name TABLESPACE_IO_CONFIG_START ioconfigs $$ = tblspciolimit; } - | NUMBER TABLESPACE_IO_CONFIG_START ioconfigs + | NUMBER ':' ioconfigs { TblSpcIOLimit *tblspciolimit = (TblSpcIOLimit *)palloc0(sizeof(TblSpcIOLimit)); if (context->star_tablespace_cnt > 0) - ereport(ERROR, - (errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg("io limit: tablespace '*' cannot be used with other tablespaces"))); - if ($1 <= 0 || $1 > UINT_MAX) - ereport(ERROR, - (errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg("io limit: tablespace oid exceeds the limit"))); + yyerror(NULL, NULL, "tablespace '*' cannot be used with other tablespaces"); + if (!OidIsValid($1) || $1 > OID_MAX) + yyerror(NULL, NULL, "tablespace oid exceeds the limit"); tblspciolimit->tablespace_oid = $1; context->normal_tablespce_cnt++; @@ -125,49 +121,69 @@ tablespace_io_config: tablespace_name TABLESPACE_IO_CONFIG_START ioconfigs $$ = tblspciolimit; } +; ioconfigs: ioconfig { IOconfig *config = (IOconfig *)palloc0(sizeof(IOconfig)); uint64 *config_var = (uint64 *)config; if (config == NULL) - io_limit_yyerror(NULL, NULL, "io limit: cannot allocate memory"); + yyerror(NULL, NULL, "cannot allocate memory"); *(config_var + $1->offset) = $1->value; $$ = config; } - | ioconfigs IOCONFIG_DELIM ioconfig + | ioconfigs ',' ioconfig { uint64 *config_var = (uint64 *)$1; + + if (*(config_var + $3->offset) != IO_LIMIT_EMPTY) + yyerror(NULL, NULL, psprintf("duplicated IO_KEY: %s", IOconfigFields[$3->offset])); + *(config_var + $3->offset) = $3->value; $$ = $1; } +; ioconfig: IO_KEY '=' io_value { - IOconfigItem *item = (IOconfigItem *)palloc0(sizeof(IOconfigItem)); + uint64 max; + IOconfigItem *item = (IOconfigItem *)palloc(sizeof(IOconfigItem)); + item->value = IO_LIMIT_MAX; + if (item == NULL) - io_limit_yyerror(NULL, NULL, "io limit: cannot allocate memory"); + yyerror(NULL, NULL, "cannot allocate memory"); item->value = $3; - for (int i = 0;i < lengthof(IOconfigFields); ++i) + for (int i = 0; i < lengthof(IOconfigFields); ++i) if (strcmp($1, IOconfigFields[i]) == 0) + { item->offset = i; + if (!io_limit_value_validate(IOconfigFields[i], item->value, &max)) + yyerror(NULL, NULL, + psprintf("value of '%s' must in range [2, %lu] or equal 'max'", IOconfigFields[i], max)); + } $$ = item; } +; -io_value: NUMBER { $$ = $1; } - | VALUE_MAX { $$ = 0; } +io_value: NUMBER + { + $$ = $1; + } + | VALUE_MAX { $$ = IO_LIMIT_MAX; } +; %% void io_limit_yyerror(IOLimitParserContext *parser_context, void *scanner, const char *message) { - ereport(ERROR, \ - (errcode(ERRCODE_SYNTAX_ERROR), \ - errmsg("%s", message))); \ + ereport(ERROR, + (errcode(ERRCODE_SYNTAX_ERROR), + errmsg("io limit: %s", message), + errhint(" %s\n %*c", line, io_limit_yycolumn, '^'))); } /* @@ -180,12 +196,14 @@ io_limit_parse(const char *limit_str) IOLimitParserContext context; IOLimitScannerState state; + line = (char *) limit_str; + context.result = NIL; context.normal_tablespce_cnt = 0; context.star_tablespace_cnt = 0; io_limit_scanner_begin(&state, limit_str); if (yyparse(&context, state.scanner) != 0) - yyerror(&context, state.scanner, "io limit: parse error"); + yyerror(&context, state.scanner, "parse error"); io_limit_scanner_finish(&state); diff --git a/src/backend/utils/resgroup/io_limit_scanner.l b/src/backend/utils/resgroup/io_limit_scanner.l index 429e682d4c9..92754f64e79 100644 --- a/src/backend/utils/resgroup/io_limit_scanner.l +++ b/src/backend/utils/resgroup/io_limit_scanner.l @@ -3,6 +3,11 @@ #include "io_limit_gram.h" #define YYSTYPE IO_LIMIT_YYSTYPE + +#define SAVE_COLUMN io_limit_yycolumn = next_col; next_col += strlen(yytext) + +static int next_col = 1; +int io_limit_yycolumn = 1; %} %option noinput nounput noyywrap @@ -14,43 +19,56 @@ id [a-zA-Z_][a-zA-Z0-9_]* wildcard \* +%s ts_param + %% -[wr](b|io)ps { - yylval->str = pstrdup(yytext); - return IO_KEY; - } +: { + SAVE_COLUMN; + BEGIN ts_param; + return ':'; + } + +<ts_param>[wr](b|io)ps { + SAVE_COLUMN; + yylval->str = pstrdup(yytext); + return IO_KEY; + } -max { +<ts_param>max { + SAVE_COLUMN; yylval->str = pstrdup("max"); return VALUE_MAX; + } + +<ts_param>; { + SAVE_COLUMN; + BEGIN INITIAL; + return ';'; } {id} { + SAVE_COLUMN; yylval->str = pstrdup(yytext); return ID; } -\; { return IOLIMIT_CONFIG_DELIM; } - -: { return TABLESPACE_IO_CONFIG_START; }; - -, { return IOCONFIG_DELIM; } - [[:digit:]]+ { + SAVE_COLUMN; yylval->integer = strtoull(yytext, NULL, 10); return NUMBER; } {wildcard} { + SAVE_COLUMN; return STAR; } -[[:space:]] ; +[[:space:]] { SAVE_COLUMN; } . { + SAVE_COLUMN; return *yytext; } - %% @@ -91,7 +109,10 @@ io_limit_scanner_begin(IOLimitScannerState *state, const char *limit_str) yyscan_t scanner; yylex_init(&scanner); - state->state = (void *)yy_scan_string(limit_str, scanner); + next_col = 1; + io_limit_yycolumn = 1; + + state->buffer = (void *)yy_scan_string(limit_str, scanner); state->scanner = scanner; } @@ -99,5 +120,5 @@ io_limit_scanner_begin(IOLimitScannerState *state, const char *limit_str) void io_limit_scanner_finish(IOLimitScannerState *state) { - yy_delete_buffer((YY_BUFFER_STATE)state->state, (yyscan_t)state->scanner); + yy_delete_buffer(state->buffer, state->scanner); } diff --git a/src/include/utils/cgroup_io_limit.h b/src/include/utils/cgroup_io_limit.h index b10482e16d8..5f774822435 100644 --- a/src/include/utils/cgroup_io_limit.h +++ b/src/include/utils/cgroup_io_limit.h @@ -17,7 +17,8 @@ typedef uint64 bdi_t; #endif -#define IO_LIMIT_MAX (0) +#define IO_LIMIT_MAX (PG_UINT64_MAX) +#define IO_LIMIT_EMPTY (0) /* * IOconfig represents the io.max of cgroup v2 io controller. @@ -72,7 +73,7 @@ typedef struct IOLimitParserContext typedef struct IOLimitScannerState { - void *state; + void *buffer; void *scanner; } IOLimitScannerState; @@ -107,6 +108,7 @@ extern int fill_bdi_list(TblSpcIOLimit *io_limit); extern List *io_limit_parse(const char *limit_str); extern void io_limit_free(List *limit_list); extern void io_limit_validate(List *limit_list); +bool io_limit_value_validate(const char *field, const uint64 value, uint64 *max); extern List *get_iostat(Oid groupid, List *io_limit); extern int compare_iostat(const ListCell *ca, const ListCell *cb); diff --git a/src/test/isolation2/input/resgroup/resgroup_io_limit.source b/src/test/isolation2/input/resgroup/resgroup_io_limit.source index e2e14a7b7a5..8b0c4d69dc6 100644 --- a/src/test/isolation2/input/resgroup/resgroup_io_limit.source +++ b/src/test/isolation2/input/resgroup/resgroup_io_limit.source @@ -1,3 +1,8 @@ +-- different bison version may have different log format +-- start_matchignore +-- m/^ERROR: io limit: syntax error.*\n/ +-- end_matchignore + -- positive: io limit with correct syntax CREATE RESOURCE GROUP rg_test_group1 WITH (concurrency=10, cpu_max_percent=10, io_limit='pg_default:rbps=1000,wbps=1000,riops=1000,wiops=1000'); SELECT io_limit FROM gp_toolkit.gp_resgroup_config WHERE groupname='rg_test_group1'; diff --git a/src/test/isolation2/output/resgroup/resgroup_io_limit.source b/src/test/isolation2/output/resgroup/resgroup_io_limit.source index 4c2d90c163c..114522afa75 100644 --- a/src/test/isolation2/output/resgroup/resgroup_io_limit.source +++ b/src/test/isolation2/output/resgroup/resgroup_io_limit.source @@ -1,3 +1,8 @@ +-- different bison version may have different log format +-- start_matchignore +-- m/^ERROR: io limit: syntax error.*\n/ +-- end_matchignore + -- positive: io limit with correct syntax CREATE RESOURCE GROUP rg_test_group1 WITH (concurrency=10, cpu_max_percent=10, io_limit='pg_default:rbps=1000,wbps=1000,riops=1000,wiops=1000'); CREATE @@ -43,23 +48,31 @@ SELECT check_cgroup_io_max('rg_test_group4', 'pg_default', 'rbps=1048576000 wbps -- negative: io limit with incorrect syntax -- * must be unique tablespace CREATE RESOURCE GROUP rg_test_group WITH (concurrency=10, cpu_max_percent=10, io_limit='pg_default:rbps=1000,wbps=1000,riops=1000,wiops=1000;*:wbps=1000'); -ERROR: io limit: tablespace '*' cannot be used with other tablespaces (seg0 127.0.0.2:6000 pid=62791) +ERROR: io limit: tablespace '*' cannot be used with other tablespaces +HINT: pg_default:rbps=1000,wbps=1000,riops=1000,wiops=1000;*:wbps=1000 + ^ -- tail ; CREATE RESOURCE GROUP rg_test_group WITH (concurrency=10, cpu_max_percent=10, io_limit='pg_default:rbps=1000,wbps=1000,riops=1000,wiops=1000;'); -ERROR: syntax error (seg0 127.0.0.2:6000 pid=62791) +ERROR: io limit: syntax error, unexpected end of file, expecting STAR or ID or NUMBER +HINT: pg_default:rbps=1000,wbps=1000,riops=1000,wiops=1000; + ^ -- tail , CREATE RESOURCE GROUP rg_test_group WITH (concurrency=10, cpu_max_percent=10, io_limit='pg_default:rbps=1000,wbps=1000,riops=1000,'); -ERROR: syntax error (seg1 127.0.0.2:6001 pid=62792) +ERROR: io limit: syntax error, unexpected end of file, expecting IO_KEY +HINT: pg_default:rbps=1000,wbps=1000,riops=1000, + ^ -- wrong key CREATE RESOURCE GROUP rg_test_group WITH (concurrency=10, cpu_max_percent=10, io_limit='pg_default:rrbps=1000'); -ERROR: syntax error (seg0 127.0.0.2:6000 pid=62791) +ERROR: io limit: syntax error, unexpected ID, expecting IO_KEY +HINT: pg_default:rrbps=1000 + ^ -- wrong tablespace name CREATE RESOURCE GROUP rg_test_group WITH (concurrency=10, cpu_max_percent=10, io_limit='pgdefault:rbps=1000,wbps=1000,riops=1000'); -ERROR: tablespace "pgdefault" does not exist (seg0 127.0.0.2:6000 pid=62791) +ERROR: tablespace "pgdefault" does not exist -- use another tablespace select mkdir('@testtablespace@/rg_io_limit_ts_1') from gp_dist_random('gp_id') intersect select mkdir('@testtablespace@/rg_io_limit_ts_1') from gp_id; --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
