You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudberry.apache.org by ma...@apache.org on 2025/09/10 08:42:26 UTC
(cloudberry) branch main updated (54fc80faa33 -> caeee214079)
This is an automated email from the ASF dual-hosted git repository.
maxyang pushed a change to branch main
in repository https://gitbox.apache.org/repos/asf/cloudberry.git
from 54fc80faa33 Fix writable CTE on replicated tables EXCEPT partition tables.
new 1d010b4e0b2 Revert "Fix double free issue in alterResgroupCallback during io_limit cleanup"
new e5c8ebb1c5b let resource group io limit testing can be reproduced (#16107)
new 8369cf6dff7 Improve io_limit behaviors (#16351)
new 0735df44c3a Fix resource group io limit flaky case (#16386)
new 60802217fa0 Io limit improve error log (#16394)
new 92bee97d0b9 io limit: fix double free (#16459)
new 723ee2822a0 Add one more hierarchy for resgroup cgroup root (#16732)
new c4e0a80cd9d delete cgroup leaf dir only when use group-v2 (#16830)
new 36b6ddd460a Add guc: gp_resource_group_cgroup_parent (#16738)
new 996d0e40c6a Fix no response when alter io_limit of resource group to '-1' (#17095)
new caeee214079 Fix gpcheckresgroupv2impl to use dynamic cgroup parent from GUC parameter
The 11 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
gpMgmt/bin/gpcheckresgroupv2impl | 59 +++++++++---
gpMgmt/bin/gpconfig | 15 ++++
src/backend/cdb/cdbvars.c | 40 +++++++++
src/backend/commands/resgroupcmds.c | 61 +++++++++----
src/backend/commands/tablespace.c | 5 ++
src/backend/utils/misc/guc_gp.c | 12 +++
src/backend/utils/resgroup/cgroup-ops-dummy.c | 10 ++-
src/backend/utils/resgroup/cgroup-ops-linux-v1.c | 22 +++--
src/backend/utils/resgroup/cgroup-ops-linux-v2.c | 33 ++++---
src/backend/utils/resgroup/cgroup.c | 76 +++++++++++-----
src/backend/utils/resgroup/cgroup_io_limit.c | 100 ++++++++++++++++-----
src/backend/utils/resgroup/io_limit_gram.y | 92 +++++++++++--------
src/backend/utils/resgroup/io_limit_scanner.l | 51 +++++++----
src/backend/utils/resgroup/resgroup.c | 96 ++++++++++++++++++++
src/include/utils/cgroup.h | 15 +++-
src/include/utils/cgroup_io_limit.h | 12 ++-
src/include/utils/guc.h | 1 +
src/include/utils/resgroup.h | 2 +
src/include/utils/unsync_guc_name.h | 1 +
.../resgroup/resgroup_auxiliary_tools_v2.out | 43 +++++++--
.../input/resgroup/resgroup_io_limit.source | 30 ++++++-
.../output/resgroup/resgroup_io_limit.source | 75 ++++++++++++++--
.../sql/resgroup/resgroup_auxiliary_tools_v2.sql | 58 +++++++++++-
23 files changed, 747 insertions(+), 162 deletions(-)
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For additional commands, e-mail: commits-help@cloudberry.apache.org
(cloudberry) 03/11: Improve io_limit behaviors (#16351)
Posted by ma...@apache.org.
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 8369cf6dff784bc0164f58ba98112fcfce8a4412
Author: RMT <d....@outlook.com>
AuthorDate: Wed Sep 6 20:04:05 2023 -0600
Improve io_limit behaviors (#16351)
* add function to clear io.max
This pr has several improvements for io limit:
1. Add a function to clear io.max. This function should be used when
alter io_limit.
2. Check tablespace in io_limit when drop tablespaces. If the
tablespace which will be dropped presents in some io_limit resource
groups, the drop tablespace statement will be aborted.
3. When InitResGroup and AlterResourceGroup, if parseio raises an error,
the error will be demote to WARNING. So the cluster can launch when
some tablespace has been removed.
---
src/backend/commands/resgroupcmds.c | 29 ++++++-
src/backend/commands/tablespace.c | 5 ++
src/backend/utils/resgroup/cgroup-ops-dummy.c | 10 ++-
src/backend/utils/resgroup/cgroup-ops-linux-v1.c | 12 ++-
src/backend/utils/resgroup/cgroup-ops-linux-v2.c | 10 ++-
src/backend/utils/resgroup/cgroup_io_limit.c | 59 ++++++++++++-
src/backend/utils/resgroup/resgroup.c | 96 +++++++++++++++++++++-
src/include/utils/cgroup.h | 2 +
src/include/utils/cgroup_io_limit.h | 6 +-
src/include/utils/resgroup.h | 1 +
.../resgroup/resgroup_auxiliary_tools_v2.out | 8 ++
.../input/resgroup/resgroup_io_limit.source | 5 ++
.../output/resgroup/resgroup_io_limit.source | 11 +++
.../sql/resgroup/resgroup_auxiliary_tools_v2.sql | 19 +++++
14 files changed, 265 insertions(+), 8 deletions(-)
diff --git a/src/backend/commands/resgroupcmds.c b/src/backend/commands/resgroupcmds.c
index 5c319e85b11..ce5ab6d584f 100644
--- a/src/backend/commands/resgroupcmds.c
+++ b/src/backend/commands/resgroupcmds.c
@@ -552,6 +552,8 @@ AlterResourceGroup(AlterResourceGroupStmt *stmt)
/*
* Get all the capabilities of one resource group in pg_resgroupcapability.
+ *
+ * Note: the io_limit in ResGroupCaps will be NIL if parse io_limit string failed.
*/
void
GetResGroupCapabilities(Relation rel, Oid groupId, ResGroupCaps *resgroupCaps)
@@ -634,9 +636,34 @@ GetResGroupCapabilities(Relation rel, Oid groupId, ResGroupCaps *resgroupCaps)
case RESGROUP_LIMIT_TYPE_IO_LIMIT:
if (cgroupOpsRoutine != NULL)
{
+ /* if InterruptHoldoffCount doesn't restore in PG_CATCH,
+ * the catch will failed. */
+ int32 savedholdoffCount = InterruptHoldoffCount;
+
oldContext = CurrentMemoryContext;
MemoryContextSwitchTo(TopMemoryContext);
- resgroupCaps->io_limit = cgroupOpsRoutine->parseio(value);
+ /*
+ * This function will be called in InitResGroups and AlterResourceGroup which
+ * shoud not be abort. In some circumstances, for example, the directory of a
+ * tablespace in io_limit be removed, then parseio will throw error. If
+ * InitResGroups be aborted, the cluster cannot launched. */
+ PG_TRY();
+ {
+ resgroupCaps->io_limit = cgroupOpsRoutine->parseio(value);
+ }
+ PG_CATCH();
+ {
+ resgroupCaps->io_limit = NIL;
+
+ InterruptHoldoffCount = savedholdoffCount;
+
+ if (elog_demote(WARNING))
+ {
+ EmitErrorReport();
+ FlushErrorState();
+ }
+ }
+ PG_END_TRY();
MemoryContextSwitchTo(oldContext);
}
else
diff --git a/src/backend/commands/tablespace.c b/src/backend/commands/tablespace.c
index ef8bd610e0b..9175ebfb5ba 100644
--- a/src/backend/commands/tablespace.c
+++ b/src/backend/commands/tablespace.c
@@ -97,8 +97,10 @@
#include "utils/lsyscache.h"
#include "utils/memutils.h"
#include "utils/rel.h"
+#include "utils/resource_manager.h"
#include "utils/tarrable.h"
#include "utils/varlena.h"
+#include "utils/resgroup.h"
#include "catalog/heap.h"
#include "catalog/oid_dispatch.h"
@@ -738,6 +740,9 @@ DropTableSpace(DropTableSpaceStmt *stmt)
errdetail_internal("%s", detail),
errdetail_log("%s", detail_log)));
+ if (IsResGroupEnabled() && Gp_resource_manager_policy == RESOURCE_MANAGER_POLICY_GROUP_V2)
+ checkTablespaceInIOlimit(tablespaceoid, true);
+
/* DROP hook for the tablespace being removed */
InvokeObjectDropHook(TableSpaceRelationId, tablespaceoid, 0);
diff --git a/src/backend/utils/resgroup/cgroup-ops-dummy.c b/src/backend/utils/resgroup/cgroup-ops-dummy.c
index 9c5c2850aea..91c588f5986 100644
--- a/src/backend/utils/resgroup/cgroup-ops-dummy.c
+++ b/src/backend/utils/resgroup/cgroup-ops-dummy.c
@@ -252,6 +252,12 @@ dumpio_dummy(List *limit_list)
return DefaultIOLimit;
}
+static void
+cleario_dummy(Oid groupid)
+{
+ unsupported_system();
+}
+
static CGroupOpsRoutine cGroupOpsRoutineDummy = {
.getcgroupname = getcgroupname_dummy,
@@ -280,7 +286,9 @@ static CGroupOpsRoutine cGroupOpsRoutineDummy = {
.setio = setio_dummy,
.freeio = freeio_dummy,
.getiostat = getiostat_dummy,
- .dumpio = dumpio_dummy
+ .dumpio = dumpio_dummy,
+ .cleario = cleario_dummy
+
};
CGroupOpsRoutine *get_cgroup_routine_dummy(void)
diff --git a/src/backend/utils/resgroup/cgroup-ops-linux-v1.c b/src/backend/utils/resgroup/cgroup-ops-linux-v1.c
index 96390712be4..fffba070a3f 100644
--- a/src/backend/utils/resgroup/cgroup-ops-linux-v1.c
+++ b/src/backend/utils/resgroup/cgroup-ops-linux-v1.c
@@ -176,6 +176,7 @@ static void setio_v1(Oid group, List *limit_list);
static void freeio_v1(List *limit_list);
static List* getiostat_v1(Oid group, List *io_limit);
static char *dumpio_v1(List *limit_list);
+static void cleario_v1(Oid groupid);
/*
* Detect gpdb cgroup component dirs.
@@ -1158,6 +1159,14 @@ dumpio_v1(List *limit_list)
return DefaultIOLimit;
}
+static void
+cleario_v1(Oid groupid)
+{
+ ereport(WARNING,
+ (errcode(ERRCODE_SYSTEM_ERROR),
+ errmsg("resource group io limit only can be used in cgroup v2.")));
+}
+
static CGroupOpsRoutine cGroupOpsRoutineV1 = {
.getcgroupname = getcgroupname_v1,
.probecgroup = probecgroup_v1,
@@ -1187,7 +1196,8 @@ static CGroupOpsRoutine cGroupOpsRoutineV1 = {
.setio = setio_v1,
.freeio = freeio_v1,
.getiostat = getiostat_v1,
- .dumpio = dumpio_v1
+ .dumpio = dumpio_v1,
+ .cleario = cleario_v1
};
CGroupOpsRoutine *get_group_routine_v1(void)
diff --git a/src/backend/utils/resgroup/cgroup-ops-linux-v2.c b/src/backend/utils/resgroup/cgroup-ops-linux-v2.c
index 546b2272213..933f8b139ef 100644
--- a/src/backend/utils/resgroup/cgroup-ops-linux-v2.c
+++ b/src/backend/utils/resgroup/cgroup-ops-linux-v2.c
@@ -166,6 +166,7 @@ static void setio_v2(Oid group, List *limit_list);
static void freeio_v2(List *limit_list);
static List *getiostat_v2(Oid group, List *io_limit);
static char *dumpio_v2(List *limit_list);
+static void cleario_v2(Oid groupid);
/*
* Dump component dir to the log.
@@ -906,6 +907,12 @@ dumpio_v2(List *limit_list)
return io_limit_dump(limit_list);
}
+static void
+cleario_v2(Oid groupid)
+{
+ clear_io_max(groupid);
+}
+
static CGroupOpsRoutine cGroupOpsRoutineV2 = {
.getcgroupname = getcgroupname_v2,
.probecgroup = probecgroup_v2,
@@ -935,7 +942,8 @@ static CGroupOpsRoutine cGroupOpsRoutineV2 = {
.setio = setio_v2,
.freeio = freeio_v2,
.getiostat = getiostat_v2,
- .dumpio = dumpio_v2
+ .dumpio = dumpio_v2,
+ .cleario = cleario_v2
};
CGroupOpsRoutine *get_group_routine_v2(void)
diff --git a/src/backend/utils/resgroup/cgroup_io_limit.c b/src/backend/utils/resgroup/cgroup_io_limit.c
index f3b1a1e33b2..29737a25ff8 100644
--- a/src/backend/utils/resgroup/cgroup_io_limit.c
+++ b/src/backend/utils/resgroup/cgroup_io_limit.c
@@ -1,5 +1,4 @@
#include "postgres.h"
-#include "utils/cgroup_io_limit.h"
#include "access/genam.h"
#include "access/heapam.h"
#include "access/htup.h"
@@ -9,6 +8,8 @@
#include "catalog/indexing.h"
#include "catalog/pg_resgroup_d.h"
#include "catalog/pg_resgroupcapability_d.h"
+#include "common/string.h"
+#include "commands/tablespace.h"
#include "catalog/pg_tablespace.h"
#include "catalog/pg_tablespace_d.h"
#include "commands/resgroupcmds.h"
@@ -18,11 +19,18 @@
#include "port.h"
#include "storage/fd.h"
#include "utils/cgroup.h"
+#include "utils/builtins.h"
#include "utils/fmgroids.h"
#include "utils/hsearch.h"
#include "utils/palloc.h"
#include "utils/relcache.h"
#include "utils/resgroup.h"
+#include "utils/cgroup_io_limit.h"
+#include <utils/cgroup.h>
+
+#ifndef __linux__
+#error cgroup is only available on linux
+#endif
#include <libgen.h>
#include <limits.h>
@@ -536,7 +544,10 @@ io_limit_dump(List *limit_list)
for(i = 0; i < fields_length; i++)
{
- appendStringInfo(result, "%s=%lu", IOconfigFields[i], *(value + i));
+ if (*(value + i) != IO_LIMIT_MAX)
+ appendStringInfo(result, "%s=%lu", IOconfigFields[i], *(value + i));
+ else
+ appendStringInfo(result, "%s=max", IOconfigFields[i]);
if (i + 1 != fields_length)
appendStringInfo(result, ",");
@@ -548,3 +559,47 @@ io_limit_dump(List *limit_list)
return result->data;
}
+
+void
+clear_io_max(Oid groupid)
+{
+ FILE *f;
+ StringInfo line = makeStringInfo();
+ StringInfo result = makeStringInfo();
+ List *result_lines = NIL;
+ ListCell *cell;
+ char path[MAX_CGROUP_PATHLEN];
+ buildPath(groupid, BASEDIR_GPDB, CGROUP_COMPONENT_PLAIN, "io.max", path, MAX_CGROUP_PATHLEN);
+
+ f = AllocateFile(path, "r");
+ /* pg_get_line_buf will reset line each time */
+ while (pg_get_line_buf(f, line))
+ {
+ uint32 maj, min;
+ int i;
+ char *str = line->data;
+
+ sscanf(str, "%u:%u", &maj, &min);
+
+ appendStringInfo(result, "%u:%u", maj, min);
+ for (i = 0; i < lengthof(IOconfigFields); i++)
+ appendStringInfo(result, " %s=max", IOconfigFields[i]);
+
+ result_lines = lappend(result_lines, (void *) pstrdup(result->data));
+
+ resetStringInfo(result);
+ }
+ FreeFile(f);
+ pfree(line->data);
+ pfree(result->data);
+ pfree(line);
+ pfree(result);
+
+ foreach(cell, result_lines)
+ {
+ char *str = (char *)lfirst(cell);
+ writeStr(groupid, BASEDIR_GPDB, CGROUP_COMPONENT_PLAIN, "io.max", str);
+ }
+
+ list_free_deep(result_lines);
+}
diff --git a/src/backend/utils/resgroup/resgroup.c b/src/backend/utils/resgroup/resgroup.c
index 6dc5f0c8174..4a6c9c14b94 100644
--- a/src/backend/utils/resgroup/resgroup.c
+++ b/src/backend/utils/resgroup/resgroup.c
@@ -33,6 +33,7 @@
#include "cdb/cdbdisp_query.h"
#include "cdb/memquota.h"
#include "commands/resgroupcmds.h"
+#include "commands/tablespace.h"
#include "common/hashfn.h"
#include "funcapi.h"
#include "miscadmin.h"
@@ -59,6 +60,7 @@
#include "utils/cgroup-ops-v1.h"
#include "utils/cgroup-ops-dummy.h"
#include "utils/cgroup-ops-v2.h"
+#include "utils/cgroup_io_limit.h"
#include "access/xact.h"
#define InvalidSlotId (-1)
@@ -805,7 +807,11 @@ ResGroupAlterOnCommit(const ResourceGroupCallbackContext *callbackCtx)
}
else if (callbackCtx->limittype == RESGROUP_LIMIT_TYPE_IO_LIMIT)
{
- cgroupOpsRoutine->setio(callbackCtx->groupid, callbackCtx->caps.io_limit);
+ if (callbackCtx->caps.io_limit != NIL)
+ {
+ cgroupOpsRoutine->cleario(callbackCtx->groupid);
+ cgroupOpsRoutine->setio(callbackCtx->groupid, callbackCtx->caps.io_limit);
+ }
}
/* reset default group if cpuset has changed */
@@ -3701,6 +3707,94 @@ check_and_unassign_from_resgroup(PlannedStmt* stmt)
bypassedGroup->caps.cpuMaxPercent == CPU_MAX_PERCENT_DISABLED);
}
+/*
+ * return ture if there is a resource group which io_limit contains tblspcid.
+ * if errout is true, print warning, message. */
+bool
+checkTablespaceInIOlimit(Oid tblspcid, bool errout)
+{
+ Relation rel_resgroup_caps;
+ SysScanDesc sscan;
+ HeapTuple tuple;
+ bool contain = false;
+ bool print_header = false;
+ StringInfo log = makeStringInfo();
+
+ rel_resgroup_caps = table_open(ResGroupCapabilityRelationId, AccessShareLock);
+ /* get io limit string from catalog */
+ sscan = systable_beginscan(rel_resgroup_caps, InvalidOid, false,
+ NULL, 0, NULL);
+ while (HeapTupleIsValid(tuple = systable_getnext(sscan)))
+ {
+ Oid id;
+ bool isNULL;
+ Datum id_datum;
+ Datum type_datum;
+ Datum value_datum;
+ ResGroupLimitType type;
+ char *io_limit_str;
+ List *limit_list;
+ ListCell *cell;
+
+ type_datum = heap_getattr(tuple, Anum_pg_resgroupcapability_reslimittype,
+ rel_resgroup_caps->rd_att, &isNULL);
+ type = (ResGroupLimitType) DatumGetInt16(type_datum);
+ if (type != RESGROUP_LIMIT_TYPE_IO_LIMIT)
+ continue;
+
+ id_datum = heap_getattr(tuple, Anum_pg_resgroupcapability_resgroupid,
+ rel_resgroup_caps->rd_att, &isNULL);
+ id = DatumGetObjectId(id_datum);
+
+ value_datum = heap_getattr(tuple, Anum_pg_resgroupcapability_value,
+ rel_resgroup_caps->rd_att, &isNULL);
+ io_limit_str = TextDatumGetCString(value_datum);
+
+ if (strcmp(io_limit_str, DefaultIOLimit) == 0)
+ continue;
+
+ limit_list = cgroupOpsRoutine->parseio(io_limit_str);
+ foreach(cell, limit_list)
+ {
+ TblSpcIOLimit *limit = (TblSpcIOLimit *) lfirst(cell);
+
+ if (limit->tablespace_oid == tblspcid)
+ {
+ contain = true;
+
+ if (!errout)
+ break;
+
+ if (!print_header)
+ {
+ print_header = true;
+ appendStringInfo(log, "io limit: following resource groups depend on tablespace %s:", get_tablespace_name(tblspcid));
+ }
+
+ appendStringInfo(log, " %s", GetResGroupNameForId(id));
+ }
+ }
+
+ cgroupOpsRoutine->freeio(limit_list);
+
+ if (contain && !errout)
+ break;
+ }
+ systable_endscan(sscan);
+
+ table_close(rel_resgroup_caps, AccessShareLock);
+
+ if (contain && errout)
+ ereport(ERROR, (errcode(ERRCODE_DEPENDENT_OBJECTS_STILL_EXIST),
+ errmsg("%s", log->data),
+ errhint("you can remove those resource groups or remove tablespace %s from io_limit of those resource groups.", get_tablespace_name(tblspcid))));
+
+ pfree(log->data);
+ pfree(log);
+
+ return contain;
+}
+
/*
* Given a planned statement, check if it is pure catalog query or a very simple query.
* Return true only when:
diff --git a/src/include/utils/cgroup.h b/src/include/utils/cgroup.h
index 42e6468be40..b9c0fa249a2 100644
--- a/src/include/utils/cgroup.h
+++ b/src/include/utils/cgroup.h
@@ -233,6 +233,7 @@ typedef void (*setio_function) (Oid group, List *limit_list);
typedef void (*freeio_function) (List *limit_list);
typedef List* (*getiostat_function) (Oid groupid, List *io_limit);
typedef char* (*dumpio_function) (List *limit_list);
+typedef void (*cleario_function) (Oid groupid);
typedef struct CGroupOpsRoutine
@@ -272,6 +273,7 @@ typedef struct CGroupOpsRoutine
freeio_function freeio;
getiostat_function getiostat;
dumpio_function dumpio;
+ cleario_function cleario;
} CGroupOpsRoutine;
/* The global function handler. */
diff --git a/src/include/utils/cgroup_io_limit.h b/src/include/utils/cgroup_io_limit.h
index e7ea1528899..b10482e16d8 100644
--- a/src/include/utils/cgroup_io_limit.h
+++ b/src/include/utils/cgroup_io_limit.h
@@ -7,11 +7,14 @@
/* type for linux device id, use libc dev_t now.
* bdi means backing device info.
*/
+#ifdef __linux__
typedef dev_t bdi_t;
-
#define make_bdi(major, minor) makedev(major, minor)
#define bdi_major(bdi) major(bdi)
#define bdi_minor(bdi) minor(bdi)
+#else
+typedef uint64 bdi_t;
+#endif
#define IO_LIMIT_MAX (0)
@@ -108,5 +111,6 @@ extern void io_limit_validate(List *limit_list);
extern List *get_iostat(Oid groupid, List *io_limit);
extern int compare_iostat(const ListCell *ca, const ListCell *cb);
extern char *io_limit_dump(List *limit_list);
+extern void clear_io_max(Oid groupid);
#endif
diff --git a/src/include/utils/resgroup.h b/src/include/utils/resgroup.h
index 6db5a10a0e8..035fc180161 100644
--- a/src/include/utils/resgroup.h
+++ b/src/include/utils/resgroup.h
@@ -216,6 +216,7 @@ extern void ResGroupMoveQuery(int sessionId, Oid groupId, const char *groupName)
extern Oid ResGroupGetGroupIdBySessionId(int sessionId);
extern char *getCpuSetByRole(const char *cpuset);
extern void checkCpuSetByRole(const char *cpuset);
+extern bool checkTablespaceInIOlimit(Oid tblspcid, bool errout);
#define LOG_RESGROUP_DEBUG(...) \
do {if (Debug_resource_group) elog(__VA_ARGS__); } while(false);
diff --git a/src/test/isolation2/expected/resgroup/resgroup_auxiliary_tools_v2.out b/src/test/isolation2/expected/resgroup/resgroup_auxiliary_tools_v2.out
index 45bd1cb45e1..61e1e7474c2 100644
--- a/src/test/isolation2/expected/resgroup/resgroup_auxiliary_tools_v2.out
+++ b/src/test/isolation2/expected/resgroup/resgroup_auxiliary_tools_v2.out
@@ -147,3 +147,11 @@ fcntl.flock(f, fcntl.LOCK_EX)
if not os.path.exists(dirname): os.close(f) return True
try: shutil.rmtree(dirname) except Exception as e: plpy.error("cannot remove dir {}".format(e)) else: return True finally: os.close(f) $$ LANGUAGE plpython3u;
CREATE
+
+0: CREATE OR REPLACE FUNCTION check_clear_io_max(groupname text) RETURNS BOOL AS $$ import ctypes import os
+postgres = ctypes.CDLL(None) clear_io_max = postgres['clear_io_max']
+# get group oid sql = "select groupid from gp_toolkit.gp_resgroup_config where groupname = '%s'" % groupname result = plpy.execute(sql) groupid = result[0]['groupid']
+clear_io_max(groupid)
+cgroup_path = "/sys/fs/cgroup/gpdb/%d/io.max" % groupid
+return os.stat(cgroup_path).st_size == 0 $$ LANGUAGE plpython3u;
+CREATE
diff --git a/src/test/isolation2/input/resgroup/resgroup_io_limit.source b/src/test/isolation2/input/resgroup/resgroup_io_limit.source
index d3a0bd485a0..e2e14a7b7a5 100644
--- a/src/test/isolation2/input/resgroup/resgroup_io_limit.source
+++ b/src/test/isolation2/input/resgroup/resgroup_io_limit.source
@@ -44,10 +44,15 @@ CREATE RESOURCE GROUP rg_test_group5 WITH (concurrency=10, cpu_max_percent=10, i
SELECT check_cgroup_io_max('rg_test_group5', 'rg_io_limit_ts_1', 'rbps=1048576000 wbps=1048576000 riops=1000 wiops=1000');
+-- try to drop test tablespace
+DROP TABLESPACE rg_io_limit_ts_1;
+
ALTER RESOURCE GROUP rg_test_group5 SET io_limit 'rg_io_limit_ts_1:rbps=1000,wbps=1000';
SELECT check_cgroup_io_max('rg_test_group5', 'rg_io_limit_ts_1', 'rbps=1048576000 wbps=1048576000 riops=max wiops=max');
+SELECT check_clear_io_max('rg_test_group5');
+
-- fault injector
SELECT gp_inject_fault('create_resource_group_fail', 'error', 1);
diff --git a/src/test/isolation2/output/resgroup/resgroup_io_limit.source b/src/test/isolation2/output/resgroup/resgroup_io_limit.source
index 9e40b8c34d9..4c2d90c163c 100644
--- a/src/test/isolation2/output/resgroup/resgroup_io_limit.source
+++ b/src/test/isolation2/output/resgroup/resgroup_io_limit.source
@@ -80,6 +80,11 @@ SELECT check_cgroup_io_max('rg_test_group5', 'rg_io_limit_ts_1', 'rbps=104857600
t
(1 row)
+-- try to drop test tablespace
+DROP TABLESPACE rg_io_limit_ts_1;
+ERROR: io limit: following resource groups depend on tablespace rg_io_limit_ts_1: rg_test_group5
+HINT: you can remove those resource groups or remove tablespace rg_io_limit_ts_1 from io_limit of those resource groups.
+
ALTER RESOURCE GROUP rg_test_group5 SET io_limit 'rg_io_limit_ts_1:rbps=1000,wbps=1000';
ALTER
@@ -89,6 +94,12 @@ SELECT check_cgroup_io_max('rg_test_group5', 'rg_io_limit_ts_1', 'rbps=104857600
t
(1 row)
+SELECT check_clear_io_max('rg_test_group5');
+ check_clear_io_max
+--------------------
+ t
+(1 row)
+
-- fault injector
SELECT gp_inject_fault('create_resource_group_fail', 'error', 1);
gp_inject_fault
diff --git a/src/test/isolation2/sql/resgroup/resgroup_auxiliary_tools_v2.sql b/src/test/isolation2/sql/resgroup/resgroup_auxiliary_tools_v2.sql
index 966d121a5dc..6a60a32e3f7 100644
--- a/src/test/isolation2/sql/resgroup/resgroup_auxiliary_tools_v2.sql
+++ b/src/test/isolation2/sql/resgroup/resgroup_auxiliary_tools_v2.sql
@@ -351,3 +351,22 @@ $$ LANGUAGE plpython3u;
finally:
os.close(f)
$$ LANGUAGE plpython3u;
+
+0: CREATE OR REPLACE FUNCTION check_clear_io_max(groupname text) RETURNS BOOL AS $$
+ import ctypes
+ import os
+
+ postgres = ctypes.CDLL(None)
+ clear_io_max = postgres['clear_io_max']
+
+ # get group oid
+ sql = "select groupid from gp_toolkit.gp_resgroup_config where groupname = '%s'" % groupname
+ result = plpy.execute(sql)
+ groupid = result[0]['groupid']
+
+ clear_io_max(groupid)
+
+ cgroup_path = "/sys/fs/cgroup/gpdb/%d/io.max" % groupid
+
+ return os.stat(cgroup_path).st_size == 0
+$$ LANGUAGE plpython3u;
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For additional commands, e-mail: commits-help@cloudberry.apache.org
(cloudberry) 02/11: let resource group io limit testing can be reproduced (#16107)
Posted by ma...@apache.org.
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 e5c8ebb1c5b27cf349fde0dc266840e24709f1a5
Author: RMT <d....@outlook.com>
AuthorDate: Tue Aug 1 18:04:38 2023 +0800
let resource group io limit testing can be reproduced (#16107)
let resource group io limit testing can be reproduced.
If we retain the objects created in the testing, we must clear those
objects before we re-run the testing on local, it's not convenient for
developers.
---
.../expected/resgroup/resgroup_auxiliary_tools_v2.out | 8 +++++---
src/test/isolation2/input/resgroup/resgroup_io_limit.source | 4 ++++
src/test/isolation2/output/resgroup/resgroup_io_limit.source | 9 +++++++++
.../isolation2/sql/resgroup/resgroup_auxiliary_tools_v2.sql | 11 +++++++++++
4 files changed, 29 insertions(+), 3 deletions(-)
diff --git a/src/test/isolation2/expected/resgroup/resgroup_auxiliary_tools_v2.out b/src/test/isolation2/expected/resgroup/resgroup_auxiliary_tools_v2.out
index 779e4dd1bcf..45bd1cb45e1 100644
--- a/src/test/isolation2/expected/resgroup/resgroup_auxiliary_tools_v2.out
+++ b/src/test/isolation2/expected/resgroup/resgroup_auxiliary_tools_v2.out
@@ -141,7 +141,9 @@ if os.path.exists(dirname): return True
try: os.makedirs(dirname) except Exception as e: plpy.error("cannot create dir {}".format(e)) else: return True $$ LANGUAGE plpython3u;
CREATE
-0: CREATE OR REPLACE FUNCTION rmdir(dirname text) RETURNS BOOL AS $$ import shutil import os
-if not os.path.exists(dirname): return True
-try: shutil.rmtree(dirname) except Exception as e: plpy.error("cannot remove dir {}".format(e)) else: return True $$ LANGUAGE plpython3u;
+0: CREATE OR REPLACE FUNCTION rmdir(dirname text) RETURNS BOOL AS $$ import shutil import fcntl import os
+try: f = os.open(dirname, os.O_RDONLY) except FileNotFoundError: return True
+fcntl.flock(f, fcntl.LOCK_EX)
+if not os.path.exists(dirname): os.close(f) return True
+try: shutil.rmtree(dirname) except Exception as e: plpy.error("cannot remove dir {}".format(e)) else: return True finally: os.close(f) $$ LANGUAGE plpython3u;
CREATE
diff --git a/src/test/isolation2/input/resgroup/resgroup_io_limit.source b/src/test/isolation2/input/resgroup/resgroup_io_limit.source
index 935964c5163..d3a0bd485a0 100644
--- a/src/test/isolation2/input/resgroup/resgroup_io_limit.source
+++ b/src/test/isolation2/input/resgroup/resgroup_io_limit.source
@@ -68,3 +68,7 @@ DROP RESOURCE GROUP rg_test_group2;
DROP RESOURCE GROUP rg_test_group3;
DROP RESOURCE GROUP rg_test_group4;
DROP RESOURCE GROUP rg_test_group5;
+
+DROP TABLESPACE rg_io_limit_ts_1;
+
+select rmdir('@testtablespace@/rg_io_limit_ts_1') from gp_dist_random('gp_id') intersect select rmdir('@testtablespace@/rg_io_limit_ts_1') from gp_id;
diff --git a/src/test/isolation2/output/resgroup/resgroup_io_limit.source b/src/test/isolation2/output/resgroup/resgroup_io_limit.source
index 610c5b29b13..9e40b8c34d9 100644
--- a/src/test/isolation2/output/resgroup/resgroup_io_limit.source
+++ b/src/test/isolation2/output/resgroup/resgroup_io_limit.source
@@ -134,3 +134,12 @@ DROP RESOURCE GROUP rg_test_group4;
DROP
DROP RESOURCE GROUP rg_test_group5;
DROP
+
+DROP TABLESPACE rg_io_limit_ts_1;
+DROP
+
+select rmdir('@testtablespace@/rg_io_limit_ts_1') from gp_dist_random('gp_id') intersect select rmdir('@testtablespace@/rg_io_limit_ts_1') from gp_id;
+ rmdir
+-------
+ t
+(1 row)
diff --git a/src/test/isolation2/sql/resgroup/resgroup_auxiliary_tools_v2.sql b/src/test/isolation2/sql/resgroup/resgroup_auxiliary_tools_v2.sql
index b791c1c31af..966d121a5dc 100644
--- a/src/test/isolation2/sql/resgroup/resgroup_auxiliary_tools_v2.sql
+++ b/src/test/isolation2/sql/resgroup/resgroup_auxiliary_tools_v2.sql
@@ -328,9 +328,18 @@ $$ LANGUAGE plpython3u;
0: CREATE OR REPLACE FUNCTION rmdir(dirname text) RETURNS BOOL AS $$
import shutil
+ import fcntl
import os
+ try:
+ f = os.open(dirname, os.O_RDONLY)
+ except FileNotFoundError:
+ return True
+
+ fcntl.flock(f, fcntl.LOCK_EX)
+
if not os.path.exists(dirname):
+ os.close(f)
return True
try:
@@ -339,4 +348,6 @@ $$ LANGUAGE plpython3u;
plpy.error("cannot remove dir {}".format(e))
else:
return True
+ finally:
+ os.close(f)
$$ LANGUAGE plpython3u;
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For additional commands, e-mail: commits-help@cloudberry.apache.org
(cloudberry) 11/11: Fix gpcheckresgroupv2impl to use dynamic cgroup parent from GUC parameter
Posted by ma...@apache.org.
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 caeee2140794295b00c06c0a83a1d0cc40acdcee
Author: zhangyue <zh...@hashdata.cn>
AuthorDate: Sun Aug 31 07:02:33 2025 +0000
Fix gpcheckresgroupv2impl to use dynamic cgroup parent from GUC parameter
This commit fixes issues introduced in
"Add guc: gp_resource_group_cgroup_parent (#16738)" where the
gp_resource_group_cgroup_parent GUC parameter was added but the
gpcheckresgroupv2impl script still used hardcoded "gpdb" paths.
Changes:
- Implement get_cgroup_parent() method to dynamically retrieve the
gp_resource_group_cgroup_parent value from database
- Replace all hardcoded "gpdb" paths with dynamic cgroup parent value
- Improve error handling in cgroup.c with more descriptive error messages
- Fix test configuration order: set gp_resource_group_cgroup_parent before
enabling gp_resource_manager=group-v2 to avoid validation failures
This ensures the cgroup validation script works correctly with custom
cgroup parent directories configured via the GUC parameter, making the
resource group feature more flexible for different deployment scenarios.
---
gpMgmt/bin/gpcheckresgroupv2impl | 59 +++++++++++++++++-----
src/backend/utils/resgroup/cgroup.c | 12 +++++
src/include/utils/cgroup.h | 9 ++++
.../resgroup/resgroup_auxiliary_tools_v2.out | 19 ++++++-
.../sql/resgroup/resgroup_auxiliary_tools_v2.sql | 12 ++++-
5 files changed, 95 insertions(+), 16 deletions(-)
diff --git a/gpMgmt/bin/gpcheckresgroupv2impl b/gpMgmt/bin/gpcheckresgroupv2impl
index 4e15d048eb1..b71d2562628 100755
--- a/gpMgmt/bin/gpcheckresgroupv2impl
+++ b/gpMgmt/bin/gpcheckresgroupv2impl
@@ -2,8 +2,17 @@
# Copyright (c) 2017, VMware, Inc. or its affiliates.
import os
+import sys
from functools import reduce
+# Add the gppylib path to sys.path to import database connection modules
+try:
+ from gppylib.db import dbconn
+ from pg import DatabaseError
+except ImportError as err:
+ sys.exit('Cannot import modules. Please check that you have sourced '
+ 'cloudberry-env.sh. Detail: ' + str(err))
+
class ValidationException(Exception):
def __init__(self, message):
@@ -29,6 +38,7 @@ class CgroupValidationVersionTwo(CgroupValidation):
def __init__(self):
self.mount_point = self.detect_cgroup_mount_point()
self.tab = {"r": os.R_OK, "w": os.W_OK, "x": os.X_OK, "f": os.F_OK}
+ self.cgroup_parent = self.get_cgroup_parent()
def validate_all(self):
"""
@@ -43,23 +53,46 @@ class CgroupValidationVersionTwo(CgroupValidation):
self.validate_permission("cgroup.procs", "rw")
- self.validate_permission("gpdb/", "rwx")
- self.validate_permission("gpdb/cgroup.procs", "rw")
+ self.validate_permission(self.cgroup_parent + "/", "rwx")
+ self.validate_permission(self.cgroup_parent + "/cgroup.procs", "rw")
+
+ self.validate_permission(self.cgroup_parent + "/cpu.max", "rw")
+ self.validate_permission(self.cgroup_parent + "/cpu.weight", "rw")
+ self.validate_permission(self.cgroup_parent + "/cpu.weight.nice", "rw")
+ self.validate_permission(self.cgroup_parent + "/cpu.stat", "r")
- self.validate_permission("gpdb/cpu.max", "rw")
- self.validate_permission("gpdb/cpu.weight", "rw")
- self.validate_permission("gpdb/cpu.weight.nice", "rw")
- self.validate_permission("gpdb/cpu.stat", "r")
+ self.validate_permission(self.cgroup_parent + "/cpuset.cpus", "rw")
+ self.validate_permission(self.cgroup_parent + "/cpuset.cpus.partition", "rw")
+ self.validate_permission(self.cgroup_parent + "/cpuset.mems", "rw")
+ self.validate_permission(self.cgroup_parent + "/cpuset.cpus.effective", "r")
+ self.validate_permission(self.cgroup_parent + "/cpuset.mems.effective", "r")
- self.validate_permission("gpdb/cpuset.cpus", "rw")
- self.validate_permission("gpdb/cpuset.cpus.partition", "rw")
- self.validate_permission("gpdb/cpuset.mems", "rw")
- self.validate_permission("gpdb/cpuset.cpus.effective", "r")
- self.validate_permission("gpdb/cpuset.mems.effective", "r")
+ self.validate_permission(self.cgroup_parent + "/memory.current", "r")
- self.validate_permission("gpdb/memory.current", "r")
+ self.validate_permission(self.cgroup_parent + "/io.max", "rw")
- self.validate_permission("gpdb/io.max", "rw")
+ def get_cgroup_parent(self):
+ """
+ Get the cgroup parent directory from the database GUC parameter
+ gp_resource_group_cgroup_parent. If unable to connect to database
+ or retrieve the parameter, report error using die function.
+ """
+ try:
+ dburl = dbconn.DbURL()
+
+ with dbconn.connect(dburl, utility=True) as conn:
+ # Query the GUC parameter value
+ sql = "SHOW gp_resource_group_cgroup_parent"
+ cursor = dbconn.query(conn, sql)
+ result = cursor.fetchone()
+
+ if result and result[0]:
+ return result[0]
+ else:
+ self.die("failed to retrieve gp_resource_group_cgroup_parent parameter from database")
+
+ except Exception as e:
+ self.die("failed to retrieve gp_resource_group_cgroup_parent parameter: {}".format(str(e)))
def die(self, msg):
raise ValidationException("cgroup is not properly configured: {}".format(msg))
diff --git a/src/backend/utils/resgroup/cgroup.c b/src/backend/utils/resgroup/cgroup.c
index 7c783fbbf6d..983d247dc96 100644
--- a/src/backend/utils/resgroup/cgroup.c
+++ b/src/backend/utils/resgroup/cgroup.c
@@ -665,6 +665,12 @@ permListCheck(const PermList *permlist, Oid group, bool report)
prop[0] ? "file" : "directory",
path);
}
+ else
+ {
+ CGROUP_CONFIG_WARNING("invalid %s name '%s': %m",
+ prop[0] ? "file" : "directory",
+ path);
+ }
return false;
}
@@ -678,6 +684,12 @@ permListCheck(const PermList *permlist, Oid group, bool report)
prop[0] ? "file" : "directory",
path);
}
+ else
+ {
+ CGROUP_CONFIG_WARNING("can't access %s '%s': %m",
+ prop[0] ? "file" : "directory",
+ path);
+ }
return false;
}
}
diff --git a/src/include/utils/cgroup.h b/src/include/utils/cgroup.h
index 39ef82aeb52..e43e1a88412 100644
--- a/src/include/utils/cgroup.h
+++ b/src/include/utils/cgroup.h
@@ -18,9 +18,18 @@
#include "postgres.h"
#include "nodes/pg_list.h"
+extern char *gp_resource_group_cgroup_parent;
+
#define MAX_CGROUP_PATHLEN 256
#define MAX_CGROUP_CONTENTLEN 1024
+#define CGROUP_CONFIG_WARNING(msg, ...) \
+ ereport(WARNING, \
+ (errmsg("cgroup is not properly configured: " msg, ##__VA_ARGS__), \
+ errhint("Please check whether the directory '/sys/fs/cgroup/%s' exists when gp_resource_manager = 'group-v2' and gp_resource_group_cgroup_parent = '%s'.", \
+ gp_resource_group_cgroup_parent ? gp_resource_group_cgroup_parent : "", \
+ gp_resource_group_cgroup_parent ? gp_resource_group_cgroup_parent : "")))
+
#define CGROUP_ERROR(...) elog(ERROR, __VA_ARGS__)
#define CGROUP_CONFIG_ERROR(...) \
CGROUP_ERROR("cgroup is not properly configured: " __VA_ARGS__)
diff --git a/src/test/isolation2/expected/resgroup/resgroup_auxiliary_tools_v2.out b/src/test/isolation2/expected/resgroup/resgroup_auxiliary_tools_v2.out
index de159f4910e..473c2fff185 100644
--- a/src/test/isolation2/expected/resgroup/resgroup_auxiliary_tools_v2.out
+++ b/src/test/isolation2/expected/resgroup/resgroup_auxiliary_tools_v2.out
@@ -20,10 +20,20 @@ ERROR: language "plpython3u" already exists
-- end_ignore
-- enable resource group and restart cluster.
+-- prerequisites:
+-- 1. '/sys/fs/cgroup/gpdb' must exist,
+-- otherwise create it before run installcheck-resgroup-v2;
+-- 2. 'gpconfig -c gp_resource_group_cgroup_parent -v "gpdb" && gpstop -rai'
+-- must run before 'gpconfig -c gp_resource_manager -v group-v2', because
+-- during the process of setting gp_resource_manager to group-v2, the
+-- system will check whether the directory
+-- '/sys/fs/cgroup/$gp_resource_group_cgroup_parent' exists.
-- start_ignore
-! gpconfig -c gp_resource_manager -v group-v2;
+! gpconfig -c gp_resource_group_cgroup_parent -v "gpdb";
-! gpconfig -c gp_resource_group_cgroup_parent -v "gpdb"
+! gpstop -rai;
+
+! gpconfig -c gp_resource_manager -v group-v2;
! gpconfig -c max_connections -v 250 -m 25;
@@ -39,6 +49,11 @@ ERROR: language "plpython3u" already exists
---------------------
group-v2
(1 row)
+0: SHOW gp_resource_group_cgroup_parent;
+ gp_resource_group_cgroup_parent
+---------------------------------
+ gpdb
+(1 row)
-- resource queue statistics should not crash
0: SELECT * FROM pg_resqueue_status;
diff --git a/src/test/isolation2/sql/resgroup/resgroup_auxiliary_tools_v2.sql b/src/test/isolation2/sql/resgroup/resgroup_auxiliary_tools_v2.sql
index e274acbb8f3..cf708526ad3 100644
--- a/src/test/isolation2/sql/resgroup/resgroup_auxiliary_tools_v2.sql
+++ b/src/test/isolation2/sql/resgroup/resgroup_auxiliary_tools_v2.sql
@@ -19,9 +19,18 @@ CREATE LANGUAGE plpython3u;
-- end_ignore
-- enable resource group and restart cluster.
+-- prerequisites:
+-- 1. '/sys/fs/cgroup/gpdb' must exist,
+-- otherwise create it before run installcheck-resgroup-v2;
+-- 2. 'gpconfig -c gp_resource_group_cgroup_parent -v "gpdb" && gpstop -rai'
+-- must run before 'gpconfig -c gp_resource_manager -v group-v2', because
+-- during the process of setting gp_resource_manager to group-v2, the
+-- system will check whether the directory
+-- '/sys/fs/cgroup/$gp_resource_group_cgroup_parent' exists.
-- start_ignore
+! gpconfig -c gp_resource_group_cgroup_parent -v "gpdb";
+! gpstop -rai;
! gpconfig -c gp_resource_manager -v group-v2;
-! gpconfig -c gp_resource_group_cgroup_parent -v "gpdb"
! gpconfig -c max_connections -v 250 -m 25;
! gpconfig -c runaway_detector_activation_percent -v 100;
! gpstop -rai;
@@ -30,6 +39,7 @@ CREATE LANGUAGE plpython3u;
-- after the restart we need a new connection to run the queries
0: SHOW gp_resource_manager;
+0: SHOW gp_resource_group_cgroup_parent;
-- resource queue statistics should not crash
0: SELECT * FROM pg_resqueue_status;
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For additional commands, e-mail: commits-help@cloudberry.apache.org
(cloudberry) 09/11: Add guc: gp_resource_group_cgroup_parent (#16738)
Posted by ma...@apache.org.
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 36b6ddd460a3db6cf3438d71641530ab2e21968e
Author: RMT <d....@outlook.com>
AuthorDate: Tue Dec 19 14:07:51 2023 +0800
Add guc: gp_resource_group_cgroup_parent (#16738)
Add guc: gp_resource_group_cgroup_parent (only for cgroup v2).
Current gpdb doesn't support change root cgroup path of resource group.
For some situations, it's better if gpdb can change the root cgroup path
of resource group.
For example, on the OS with systemd, user maybe want to create a
delegated cgroup to gpdb via systemd, but the delegated cgroup must end
with .service which typically is /sys/fs/cgroup/gpdb.service. And in
other OS without systemd, user maybe want to use /sys/fs/cgroup/gpdb or
other locations directly. So add the gp_resource_group_cgroup_parent can
make the resource group more flexible.
---
gpMgmt/bin/gpconfig | 15 ++++++++
src/backend/cdb/cdbvars.c | 40 ++++++++++++++++++++++
src/backend/utils/misc/guc_gp.c | 12 +++++++
src/backend/utils/resgroup/cgroup.c | 2 +-
src/include/utils/guc.h | 1 +
src/include/utils/resgroup.h | 1 +
src/include/utils/unsync_guc_name.h | 1 +
.../resgroup/resgroup_auxiliary_tools_v2.out | 2 ++
.../sql/resgroup/resgroup_auxiliary_tools_v2.sql | 1 +
9 files changed, 74 insertions(+), 1 deletion(-)
diff --git a/gpMgmt/bin/gpconfig b/gpMgmt/bin/gpconfig
index 3d5495bd01b..f4e3ce7c62d 100755
--- a/gpMgmt/bin/gpconfig
+++ b/gpMgmt/bin/gpconfig
@@ -15,6 +15,7 @@
import os
import sys
import re
+import psutil
try:
from gppylib.gpparseopts import OptParser, OptChecker
@@ -170,6 +171,20 @@ class Guc:
return msg
elif newval != "'queue'":
return "the value of gp_resource_manager must be 'group' or 'group-v2' or 'queue'"
+ elif self.name == "gp_resource_group_cgroup_parent":
+ base = newval.strip("'")
+ if not re.match("^[0-9a-zA-Z][-._0-9a-zA-Z]*$", base):
+ return "resource group cgroup parent can only contains alphabet, number, and non-leading . _ -"
+
+ path = None
+ for partition in psutil.disk_partitions(all=True):
+ if partition.fstype == "cgroup2":
+ path = partition.mountpoint + "/" + base
+ break
+ if path is None:
+ return "cannot find cgroup v2 mountpoint"
+ if not os.path.isdir(path):
+ return "'%s' doesn't exists or is not a directory" % path
elif self.name == 'unix_socket_permissions':
if newval[0] != '0':
diff --git a/src/backend/cdb/cdbvars.c b/src/backend/cdb/cdbvars.c
index 9db3389e0bf..04c81848b05 100644
--- a/src/backend/cdb/cdbvars.c
+++ b/src/backend/cdb/cdbvars.c
@@ -17,9 +17,11 @@
*
*-------------------------------------------------------------------------
*/
+#include "catalog/pg_collation_d.h"
#include "postgres.h"
#include "miscadmin.h"
+#include "regex/regex.h"
#include "utils/guc.h"
#include "cdb/cdbvars.h"
#include "libpq-fe.h"
@@ -567,6 +569,44 @@ gpvars_show_gp_resource_manager_policy(void)
}
}
+bool gpvars_check_gp_resource_group_cgroup_parent(char **newval, void **extra, GucSource source)
+{
+ int regres;
+ char err[128];
+ regex_t re;
+ char *pattern = "^[0-9a-zA-Z][-._0-9a-zA-Z]*$";
+ pg_wchar *wpattern = palloc((strlen(pattern) + 1) * sizeof(pg_wchar));
+ int wlen = pg_mb2wchar_with_len(pattern, wpattern, strlen(pattern));
+ pg_wchar *data = palloc((strlen(*newval) + 1) * sizeof(pg_wchar));
+ int data_len = pg_mb2wchar_with_len(*newval, data, sizeof(*newval));
+ bool match = true;
+
+ regres = pg_regcomp(&re, wpattern, wlen, REG_ADVANCED, DEFAULT_COLLATION_OID);
+ if (regres != REG_OKAY)
+ {
+ pg_regerror(regres, &re, err, sizeof(err));
+ GUC_check_errmsg("compile regex failed: %s", err);
+
+ pfree(wpattern);
+ pfree(data);
+
+ return false;
+ }
+
+ regres = pg_regexec(&re, data, data_len, 0, NULL, 0, NULL, 0);
+ if (regres != REG_OKAY)
+ {
+ match = false;
+ GUC_check_errmsg("gp_resource_group_cgroup_parent can only contains alphabet, number and non-leading . _ -");
+ }
+
+ pg_regfree(&re);
+ pfree(wpattern);
+ pfree(data);
+
+ return match;
+}
+
/*
* gpvars_assign_statement_mem
*/
diff --git a/src/backend/utils/misc/guc_gp.c b/src/backend/utils/misc/guc_gp.c
index 2c373e5a582..cc98e66c089 100644
--- a/src/backend/utils/misc/guc_gp.c
+++ b/src/backend/utils/misc/guc_gp.c
@@ -239,6 +239,7 @@ double gp_resource_group_cpu_limit;
bool gp_resource_group_bypass;
bool gp_resource_group_bypass_catalog_query;
bool gp_resource_group_bypass_direct_dispatch;
+char *gp_resource_group_cgroup_parent;
/* Metrics collector debug GUC */
bool vmem_process_interrupt = false;
@@ -4900,6 +4901,17 @@ struct config_string ConfigureNamesString_gp[] =
gpvars_show_gp_resource_manager_policy,
},
+ {
+ {"gp_resource_group_cgroup_parent", PGC_POSTMASTER, RESOURCES,
+ gettext_noop("The root of gpdb cgroup hierarchy."),
+ NULL,
+ GUC_SUPERUSER_ONLY
+ },
+ &gp_resource_group_cgroup_parent,
+ "gpdb.service",
+ gpvars_check_gp_resource_group_cgroup_parent, NULL, NULL
+ },
+
/* for pljava */
{
{"pljava_vmoptions", PGC_SUSET, CUSTOM_OPTIONS,
diff --git a/src/backend/utils/resgroup/cgroup.c b/src/backend/utils/resgroup/cgroup.c
index 504c8c2fcba..7c783fbbf6d 100644
--- a/src/backend/utils/resgroup/cgroup.c
+++ b/src/backend/utils/resgroup/cgroup.c
@@ -174,7 +174,7 @@ buildPathSafe(Oid group,
* for cgroup v2, we just have the top level and child level,
* don't need to care about the component.
*/
- base_dir = base == BASEDIR_GPDB ? "gpdb" : "";
+ base_dir = base == BASEDIR_GPDB ? gp_resource_group_cgroup_parent : "";
len = snprintf(path, path_size, "%s/%s%s/%s",
cgroupSystemInfo->cgroup_dir, base_dir, group_dir, filename);
}
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index b12d29f1e16..13d7e79e262 100644
--- a/src/include/utils/guc.h
+++ b/src/include/utils/guc.h
@@ -860,6 +860,7 @@ extern void gpvars_assign_gp_resource_manager_policy(const char *newval, void *e
extern const char *gpvars_show_gp_resource_manager_policy(void);
extern const char *gpvars_assign_gp_resqueue_memory_policy(const char *newval, bool doit, GucSource source);
extern const char *gpvars_show_gp_resqueue_memory_policy(void);
+extern bool gpvars_check_gp_resource_group_cgroup_parent(char **newval, void **extra, GucSource source);
extern bool gpvars_check_statement_mem(int *newval, void **extra, GucSource source);
extern bool gpvars_check_rg_query_fixed_mem(int *newval, void **extra, GucSource source);
extern int guc_name_compare(const char *namea, const char *nameb);
diff --git a/src/include/utils/resgroup.h b/src/include/utils/resgroup.h
index 035fc180161..dd03b842fd2 100644
--- a/src/include/utils/resgroup.h
+++ b/src/include/utils/resgroup.h
@@ -109,6 +109,7 @@ extern int gp_resource_group_queuing_timeout;
extern bool gp_resource_group_bypass_catalog_query;
extern int gp_resource_group_move_timeout;
extern bool gp_resource_group_bypass_direct_dispatch;
+extern char *gp_resource_group_cgroup_parent;
/*
* Non-GUC global variables.
diff --git a/src/include/utils/unsync_guc_name.h b/src/include/utils/unsync_guc_name.h
index 37f629e6e97..97b9df29814 100644
--- a/src/include/utils/unsync_guc_name.h
+++ b/src/include/utils/unsync_guc_name.h
@@ -260,6 +260,7 @@
"gp_resource_group_bypass_direct_dispatch",
"gp_resource_group_queuing_timeout",
"gp_resource_group_move_timeout",
+ "gp_resource_group_cgroup_parent",
"gp_resource_manager",
"gp_resqueue_memory_policy",
"gp_resqueue_priority",
diff --git a/src/test/isolation2/expected/resgroup/resgroup_auxiliary_tools_v2.out b/src/test/isolation2/expected/resgroup/resgroup_auxiliary_tools_v2.out
index 93c1d1aa4df..bc5e8764091 100644
--- a/src/test/isolation2/expected/resgroup/resgroup_auxiliary_tools_v2.out
+++ b/src/test/isolation2/expected/resgroup/resgroup_auxiliary_tools_v2.out
@@ -23,6 +23,8 @@ ERROR: language "plpython3u" already exists
-- start_ignore
! gpconfig -c gp_resource_manager -v group-v2;
+! gpconfig -c gp_resource_group_cgroup_parent -v "gpdb"
+
! gpconfig -c max_connections -v 250 -m 25;
! gpconfig -c runaway_detector_activation_percent -v 100;
diff --git a/src/test/isolation2/sql/resgroup/resgroup_auxiliary_tools_v2.sql b/src/test/isolation2/sql/resgroup/resgroup_auxiliary_tools_v2.sql
index e2d8db51888..b762850468c 100644
--- a/src/test/isolation2/sql/resgroup/resgroup_auxiliary_tools_v2.sql
+++ b/src/test/isolation2/sql/resgroup/resgroup_auxiliary_tools_v2.sql
@@ -21,6 +21,7 @@ CREATE LANGUAGE plpython3u;
-- enable resource group and restart cluster.
-- start_ignore
! gpconfig -c gp_resource_manager -v group-v2;
+! gpconfig -c gp_resource_group_cgroup_parent -v "gpdb"
! gpconfig -c max_connections -v 250 -m 25;
! gpconfig -c runaway_detector_activation_percent -v 100;
! gpstop -rai;
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For additional commands, e-mail: commits-help@cloudberry.apache.org
(cloudberry) 06/11: io limit: fix double free (#16459)
Posted by ma...@apache.org.
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 92bee97d0b943e049c5eb900bf5e93c7e7623d06
Author: RMT <d....@outlook.com>
AuthorDate: Wed Sep 20 13:36:26 2023 +0800
io limit: fix double free (#16459)
io limit: fix double free.
In 'alterResgroupCallback', the io_limit pointer of 'caps' and 'oldCaps'
maybe point to the same location, so there is a double free potentially.
In 'alterResgroupCallback', the 'oldCaps' will be filled in
'GetResGroupCapabilities', and the assign it to 'caps' via:
caps = oldCaps
To resolve this problem, the code should free the oldCaps.io_limit, and
set it to NIL, when the io_limit has not been altered.
So, if the io_limit has not been altered, caps.io_limit =
oldCaps.io_limit = NIL. If io_limit has been altered, caps.io_limit !=
oldCaps.io_limit.
---
src/backend/commands/resgroupcmds.c | 10 +++++++++-
src/test/isolation2/input/resgroup/resgroup_io_limit.source | 6 +++++-
src/test/isolation2/output/resgroup/resgroup_io_limit.source | 11 ++++++++++-
3 files changed, 24 insertions(+), 3 deletions(-)
diff --git a/src/backend/commands/resgroupcmds.c b/src/backend/commands/resgroupcmds.c
index ce5ab6d584f..6ebe1ac243e 100644
--- a/src/backend/commands/resgroupcmds.c
+++ b/src/backend/commands/resgroupcmds.c
@@ -438,6 +438,12 @@ AlterResourceGroup(AlterResourceGroupStmt *stmt)
/* Load current resource group capabilities */
GetResGroupCapabilities(pg_resgroupcapability_rel, groupid, &oldCaps);
+ /* If io_limit not been altered, reset io_limit field to NIL */
+ if (limitType != RESGROUP_LIMIT_TYPE_IO_LIMIT && oldCaps.io_limit != NIL)
+ {
+ cgroupOpsRoutine->freeio(oldCaps.io_limit);
+ oldCaps.io_limit = NIL;
+ }
caps = oldCaps;
switch (limitType)
@@ -479,6 +485,8 @@ AlterResourceGroup(AlterResourceGroupStmt *stmt)
}
validateCapabilities(pg_resgroupcapability_rel, groupid, &caps, false);
+ AssertImply(limitType != RESGROUP_LIMIT_TYPE_IO_LIMIT, caps.io_limit == NIL);
+ AssertImply(limitType == RESGROUP_LIMIT_TYPE_IO_LIMIT, caps.io_limit != NIL);
/* cpuset & cpu_max_percent can not coexist.
* if cpuset is active, then cpu_max_percent must set to CPU_RATE_LIMIT_DISABLED,
@@ -1123,7 +1131,7 @@ alterResgroupCallback(XactEvent event, void *arg)
if (callbackCtx->caps.io_limit != NIL)
cgroupOpsRoutine->freeio(callbackCtx->caps.io_limit);
- if (callbackCtx->caps.io_limit != NIL)
+ if (callbackCtx->oldCaps.io_limit != NIL)
cgroupOpsRoutine->freeio(callbackCtx->oldCaps.io_limit);
pfree(callbackCtx);
diff --git a/src/test/isolation2/input/resgroup/resgroup_io_limit.source b/src/test/isolation2/input/resgroup/resgroup_io_limit.source
index 8b0c4d69dc6..a09414f3f99 100644
--- a/src/test/isolation2/input/resgroup/resgroup_io_limit.source
+++ b/src/test/isolation2/input/resgroup/resgroup_io_limit.source
@@ -56,6 +56,8 @@ ALTER RESOURCE GROUP rg_test_group5 SET io_limit 'rg_io_limit_ts_1:rbps=1000,wbp
SELECT check_cgroup_io_max('rg_test_group5', 'rg_io_limit_ts_1', 'rbps=1048576000 wbps=1048576000 riops=max wiops=max');
+ALTER RESOURCE GROUP rg_test_group5 SET concurrency 1;
+
SELECT check_clear_io_max('rg_test_group5');
-- fault injector
@@ -67,11 +69,13 @@ SELECT groupid, groupname, cpuset FROM gp_toolkit.gp_resgroup_config WHERE group
SELECT gp_inject_fault('create_resource_group_fail', 'reset', 1);
--- start_ignore
-- view
+-- start_ignore
SELECT * from gp_toolkit.gp_resgroup_iostats_per_host;
-- end_ignore
+SELECT count(*) > 0 as r from gp_toolkit.gp_resgroup_iostats_per_host;
+
-- clean
DROP RESOURCE GROUP rg_test_group1;
DROP RESOURCE GROUP rg_test_group2;
diff --git a/src/test/isolation2/output/resgroup/resgroup_io_limit.source b/src/test/isolation2/output/resgroup/resgroup_io_limit.source
index 114522afa75..3e7bd7f2134 100644
--- a/src/test/isolation2/output/resgroup/resgroup_io_limit.source
+++ b/src/test/isolation2/output/resgroup/resgroup_io_limit.source
@@ -107,6 +107,9 @@ SELECT check_cgroup_io_max('rg_test_group5', 'rg_io_limit_ts_1', 'rbps=104857600
t
(1 row)
+ALTER RESOURCE GROUP rg_test_group5 SET concurrency 1;
+ALTER
+
SELECT check_clear_io_max('rg_test_group5');
check_clear_io_max
--------------------
@@ -134,8 +137,8 @@ SELECT gp_inject_fault('create_resource_group_fail', 'reset', 1);
Success:
(1 row)
--- start_ignore
-- view
+-- start_ignore
SELECT * from gp_toolkit.gp_resgroup_iostats_per_host;
rsgname | hostname | tablespace | rbps | wbps | riops | wiops
----------------+----------+------------------+------+------+-------+-------
@@ -147,6 +150,12 @@ SELECT * from gp_toolkit.gp_resgroup_iostats_per_host;
(5 rows)
-- end_ignore
+SELECT count(*) > 0 as r from gp_toolkit.gp_resgroup_iostats_per_host;
+ r
+------
+ t
+(1 row)
+
-- clean
DROP RESOURCE GROUP rg_test_group1;
DROP
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For additional commands, e-mail: commits-help@cloudberry.apache.org
(cloudberry) 07/11: Add one more hierarchy for resgroup cgroup root (#16732)
Posted by ma...@apache.org.
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 723ee2822a0fc6841a0b7f4c9e0ebdfd8df99b74
Author: RMT <d....@outlook.com>
AuthorDate: Tue Dec 5 14:20:10 2023 +0800
Add one more hierarchy for resgroup cgroup root (#16732)
Add one more hierarchy for resource group when use cgroup v2.
Current leaf node in the gpdb cgroup hierarchy is:
/sys/fs/cgroup/gpdb/<oid>, it's ok for gpdb workflow. But for some
extensions which want to use gpdb cgroup hierarchy, it's not convenient.
Extensions like plcontainer want create sub-cgroup under
/sys/fs/cgroup/<oid> as new leaf node, it's not possible in current
hierarchy, because of no internal processes constraint of cgroup v2.
This commit use a new hierarchy to adopt extensions which want to use
gpdb cgroup hierarchy, and the modification is tiny: move processes from
/sys/fs/cgroup/<oid>/cgroup.procs to
/sys/fs/cgroup/gpdb/<oid>/queries/cgroup.procs, and keep limitations in
/sys/fs/cgroup/<oid>.
With this modification, extensions which want to use gpdb cgroup
hierarchy can create sub cgroup under /sys/fs/cgroup/gpdb/<oid>.
For example, plcontainer will create a cgroup
/sys/fs/cgroup/gpdb/<oid>/docker-12345 and put processes into it.
---
src/backend/utils/resgroup/cgroup-ops-linux-v1.c | 10 +++++-----
src/backend/utils/resgroup/cgroup-ops-linux-v2.c | 15 ++++++++++-----
src/backend/utils/resgroup/cgroup.c | 8 +++++---
src/include/utils/cgroup.h | 4 +++-
.../expected/resgroup/resgroup_auxiliary_tools_v2.out | 2 +-
.../sql/resgroup/resgroup_auxiliary_tools_v2.sql | 2 +-
6 files changed, 25 insertions(+), 16 deletions(-)
diff --git a/src/backend/utils/resgroup/cgroup-ops-linux-v1.c b/src/backend/utils/resgroup/cgroup-ops-linux-v1.c
index fffba070a3f..0ad53ee35ab 100644
--- a/src/backend/utils/resgroup/cgroup-ops-linux-v1.c
+++ b/src/backend/utils/resgroup/cgroup-ops-linux-v1.c
@@ -652,11 +652,11 @@ createcgroup_v1(Oid group)
{
int retry = 0;
- if (!createDir(group, CGROUP_COMPONENT_CPU) ||
- !createDir(group, CGROUP_COMPONENT_CPUACCT) ||
- !createDir(group, CGROUP_COMPONENT_MEMORY) ||
+ if (!createDir(group, CGROUP_COMPONENT_CPU, "") ||
+ !createDir(group, CGROUP_COMPONENT_CPUACCT, "") ||
+ !createDir(group, CGROUP_COMPONENT_MEMORY, "") ||
(gp_resource_group_enable_cgroup_cpuset &&
- !createDir(group, CGROUP_COMPONENT_CPUSET)))
+ !createDir(group, CGROUP_COMPONENT_CPUSET, "")))
{
CGROUP_ERROR("can't create cgroup for resource group '%d': %m", group);
}
@@ -705,7 +705,7 @@ create_default_cpuset_group_v1(void)
CGroupComponentType component = CGROUP_COMPONENT_CPUSET;
int retry = 0;
- if (!createDir(DEFAULT_CPUSET_GROUP_ID, component))
+ if (!createDir(DEFAULT_CPUSET_GROUP_ID, component, ""))
{
CGROUP_ERROR("can't create cpuset cgroup for resgroup '%d': %m",
DEFAULT_CPUSET_GROUP_ID);
diff --git a/src/backend/utils/resgroup/cgroup-ops-linux-v2.c b/src/backend/utils/resgroup/cgroup-ops-linux-v2.c
index 69d2b1b0461..6b5f667fe7f 100644
--- a/src/backend/utils/resgroup/cgroup-ops-linux-v2.c
+++ b/src/backend/utils/resgroup/cgroup-ops-linux-v2.c
@@ -386,7 +386,8 @@ createcgroup_v2(Oid group)
{
int retry = 0;
- if (!createDir(group, CGROUP_COMPONENT_PLAIN))
+ if (!createDir(group, CGROUP_COMPONENT_PLAIN, "") ||
+ !createDir(group, CGROUP_COMPONENT_PLAIN, CGROUPV2_LEAF_INDENTIFIER))
{
CGROUP_ERROR("can't create cgroup for resource group '%d': %m", group);
}
@@ -418,7 +419,7 @@ create_default_cpuset_group_v2(void)
CGroupComponentType component = CGROUP_COMPONENT_PLAIN;
int retry = 0;
- if (!createDir(DEFAULT_CPUSET_GROUP_ID, component))
+ if (!createDir(DEFAULT_CPUSET_GROUP_ID, component, ""))
{
CGROUP_ERROR("can't create cpuset cgroup for resgroup '%d': %m",
DEFAULT_CPUSET_GROUP_ID);
@@ -466,6 +467,7 @@ create_default_cpuset_group_v2(void)
static void
attachcgroup_v2(Oid group, int pid, bool is_cpuset_enabled)
{
+ char path_of_leaf[MAXPATHLEN];
/*
* needn't write to file if the pid has already been written in.
* Unless it has not been written or the group has changed or
@@ -474,8 +476,9 @@ attachcgroup_v2(Oid group, int pid, bool is_cpuset_enabled)
if (IsUnderPostmaster && group == currentGroupIdInCGroup)
return;
+ pg_sprintf(path_of_leaf, "%s/cgroup.procs", CGROUPV2_LEAF_INDENTIFIER);
writeInt64(group, BASEDIR_GPDB, CGROUP_COMPONENT_PLAIN,
- "cgroup.procs", pid);
+ path_of_leaf, pid);
/*
* Do not assign the process to cgroup/memory for now.
@@ -499,6 +502,7 @@ detachcgroup_v2(Oid group, CGroupComponentType component, int fd_dir)
{
char path[MAX_CGROUP_PATHLEN];
size_t path_size = sizeof(path);
+ char path_of_leaf[MAXPATHLEN];
char *buf;
size_t buf_size;
@@ -533,7 +537,8 @@ detachcgroup_v2(Oid group, CGroupComponentType component, int fd_dir)
} \
} while (0)
- buildPath(group, BASEDIR_GPDB, component, "cgroup.procs", path, path_size);
+ pg_sprintf(path_of_leaf, "%s/cgroup.procs", CGROUPV2_LEAF_INDENTIFIER);
+ buildPath(group, BASEDIR_GPDB, component, path_of_leaf, path, path_size);
fdr = open(path, O_RDONLY);
@@ -561,7 +566,7 @@ detachcgroup_v2(Oid group, CGroupComponentType component, int fd_dir)
if (buf_len == 0)
return;
- buildPath(DEFAULTRESGROUP_OID, BASEDIR_GPDB, component, "cgroup.procs",
+ buildPath(DEFAULTRESGROUP_OID, BASEDIR_GPDB, component, path_of_leaf,
path, path_size);
fdw = open(path, O_WRONLY);
diff --git a/src/backend/utils/resgroup/cgroup.c b/src/backend/utils/resgroup/cgroup.c
index a996e31aa51..839090f39e4 100644
--- a/src/backend/utils/resgroup/cgroup.c
+++ b/src/backend/utils/resgroup/cgroup.c
@@ -285,12 +285,12 @@ lockDir(const char *path, bool block)
* Create cgroup dir
*/
bool
-createDir(Oid group, CGroupComponentType component)
+createDir(Oid group, CGroupComponentType component, char *filename)
{
char path[MAX_CGROUP_PATHLEN];
size_t path_size = sizeof(path);
- buildPath(group, BASEDIR_GPDB, component, "", path, path_size);
+ buildPath(group, BASEDIR_GPDB, component, filename, path, path_size);
if (mkdir(path, 0755) && errno != EEXIST)
return false;
@@ -468,12 +468,14 @@ deleteDir(Oid group, CGroupComponentType component, const char *filename, bool u
{
char path[MAX_CGROUP_PATHLEN];
+ char leaf_path[MAX_CGROUP_PATHLEN];
size_t path_size = sizeof(path);
int retry = unassign ? 0 : MAX_RETRY - 1;
int fd_dir;
buildPath(group, BASEDIR_GPDB, component, "", path, path_size);
+ buildPath(group, BASEDIR_GPDB, component, CGROUPV2_LEAF_INDENTIFIER, leaf_path, path_size);
/*
* To prevent race condition between multiple processes we require a dir
@@ -497,7 +499,7 @@ deleteDir(Oid group, CGroupComponentType component, const char *filename, bool u
if (unassign)
detachcgroup(group, component, fd_dir);
- if (rmdir(path))
+ if (rmdir(leaf_path) || rmdir(path))
{
int err = errno;
diff --git a/src/include/utils/cgroup.h b/src/include/utils/cgroup.h
index b9c0fa249a2..39ef82aeb52 100644
--- a/src/include/utils/cgroup.h
+++ b/src/include/utils/cgroup.h
@@ -47,6 +47,8 @@
/* This is the default value about Linux Control Group */
#define DEFAULT_CPU_PERIOD_US 100000LL
+/* The name of leaf cgroup when use cgroup v2 */
+#define CGROUPV2_LEAF_INDENTIFIER "queries"
/*
* Resource Group underlying component types.
@@ -168,7 +170,7 @@ extern void setComponentDir(CGroupComponentType component, const char *dir);
extern int lockDir(const char *path, bool block);
/* Create cgroup dir. */
-extern bool createDir(Oid group, CGroupComponentType comp);
+extern bool createDir(Oid group, CGroupComponentType comp, char *filename);
/* Delete cgroup dir. */
extern bool deleteDir(Oid group, CGroupComponentType component, const char *filename, bool unassign,
void (*detachcgroup) (Oid group, CGroupComponentType component, int fd_dir));
diff --git a/src/test/isolation2/expected/resgroup/resgroup_auxiliary_tools_v2.out b/src/test/isolation2/expected/resgroup/resgroup_auxiliary_tools_v2.out
index e2e959374b2..93c1d1aa4df 100644
--- a/src/test/isolation2/expected/resgroup/resgroup_auxiliary_tools_v2.out
+++ b/src/test/isolation2/expected/resgroup/resgroup_auxiliary_tools_v2.out
@@ -118,7 +118,7 @@ sql = "select sess_id from pg_stat_activity where pid = '%d'" % pid result = plp
sql = "select groupid from gp_toolkit.gp_resgroup_config where groupname='%s'" % groupname result = plpy.execute(sql) groupid = result[0]['groupid']
sql = "select hostname from gp_segment_configuration group by hostname" result = plpy.execute(sql) hosts = [_['hostname'] for _ in result]
def get_result(host): stdout = subprocess.run(["ssh", "{}".format(host), "ps -ef | grep postgres | grep con{} | grep -v grep | awk '{{print $2}}'".format(session_id)], stdout=subprocess.PIPE, check=True).stdout session_pids = stdout.splitlines()
-path = "/sys/fs/cgroup/gpdb/{}/cgroup.procs".format(groupid) stdout = subprocess.run(["ssh", "{}".format(host), "cat {}".format(path)], stdout=subprocess.PIPE, check=True).stdout cgroups_pids = stdout.splitlines()
+path = "/sys/fs/cgroup/gpdb/{}/queries/cgroup.procs".format(groupid) stdout = subprocess.run(["ssh", "{}".format(host), "cat {}".format(path)], stdout=subprocess.PIPE, check=True).stdout cgroups_pids = stdout.splitlines()
return set(session_pids).issubset(set(cgroups_pids))
for host in hosts: if not get_result(host): return False return True
$$ LANGUAGE plpython3u;
diff --git a/src/test/isolation2/sql/resgroup/resgroup_auxiliary_tools_v2.sql b/src/test/isolation2/sql/resgroup/resgroup_auxiliary_tools_v2.sql
index 88f66c4f18d..e2d8db51888 100644
--- a/src/test/isolation2/sql/resgroup/resgroup_auxiliary_tools_v2.sql
+++ b/src/test/isolation2/sql/resgroup/resgroup_auxiliary_tools_v2.sql
@@ -259,7 +259,7 @@ $$ LANGUAGE plpython3u;
stdout=subprocess.PIPE, check=True).stdout
session_pids = stdout.splitlines()
- path = "/sys/fs/cgroup/gpdb/{}/cgroup.procs".format(groupid)
+ path = "/sys/fs/cgroup/gpdb/{}/queries/cgroup.procs".format(groupid)
stdout = subprocess.run(["ssh", "{}".format(host), "cat {}".format(path)], stdout=subprocess.PIPE, check=True).stdout
cgroups_pids = stdout.splitlines()
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For additional commands, e-mail: commits-help@cloudberry.apache.org
(cloudberry) 01/11: Revert "Fix double free issue in alterResgroupCallback during io_limit cleanup"
Posted by ma...@apache.org.
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 1d010b4e0b22a6ee1c34396ac70cbac59c33c6b1
Author: zhangyue <zh...@hashdata.cn>
AuthorDate: Fri Aug 29 12:40:09 2025 +0800
Revert "Fix double free issue in alterResgroupCallback during io_limit cleanup"
This reverts commit 65cd966d5a144dfe861d5107ac2db0be0ad18798.
---
src/backend/commands/resgroupcmds.c | 15 +--------------
1 file changed, 1 insertion(+), 14 deletions(-)
diff --git a/src/backend/commands/resgroupcmds.c b/src/backend/commands/resgroupcmds.c
index f6ee4fefc34..5c319e85b11 100644
--- a/src/backend/commands/resgroupcmds.c
+++ b/src/backend/commands/resgroupcmds.c
@@ -1093,23 +1093,10 @@ alterResgroupCallback(XactEvent event, void *arg)
if (event == XACT_EVENT_COMMIT)
ResGroupAlterOnCommit(callbackCtx);
- /*
- * Free io_limit resources allocated in AlterResourceGroup().
- *
- * We need to handle two cases:
- * 1. caps.io_limit != oldCaps.io_limit: case RESGROUP_LIMIT_TYPE_IO_LIMIT
- * 2. caps.io_limit == oldCaps.io_limit: other cases
- *
- * The pointer comparison (oldCaps.io_limit != caps.io_limit) is crucial to
- * avoid double free errors. When "other cases", both pointers might
- * reference the same memory location, so we only free oldCaps.io_limit if
- * it's different from caps.io_limit.
- */
if (callbackCtx->caps.io_limit != NIL)
cgroupOpsRoutine->freeio(callbackCtx->caps.io_limit);
- if (callbackCtx->oldCaps.io_limit != NIL &&
- callbackCtx->oldCaps.io_limit != callbackCtx->caps.io_limit)
+ if (callbackCtx->caps.io_limit != NIL)
cgroupOpsRoutine->freeio(callbackCtx->oldCaps.io_limit);
pfree(callbackCtx);
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For additional commands, e-mail: commits-help@cloudberry.apache.org
(cloudberry) 05/11: Io limit improve error log (#16394)
Posted by ma...@apache.org.
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 <d....@outlook.com>
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: commits-unsubscribe@cloudberry.apache.org
For additional commands, e-mail: commits-help@cloudberry.apache.org
(cloudberry) 10/11: Fix no response when alter io_limit of resource group to '-1' (#17095)
Posted by ma...@apache.org.
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 996d0e40c6a957f887ca0a4d82520a2de4b8697c
Author: RMT <d....@outlook.com>
AuthorDate: Tue Feb 27 15:57:19 2024 +0800
Fix no response when alter io_limit of resource group to '-1' (#17095)
Fix no response when alter io_limit of resource group to '-1'.
There is no action when ALTER RESOURCE GROUP xxx SET IO_LIMIT '-1'
before.
Now the action is that clear the content of io.max and update relation
pg_resgroupcapability.
---
src/backend/commands/resgroupcmds.c | 11 ++++++++++-
src/backend/utils/resgroup/resgroup.c | 12 +++++++-----
.../resgroup/resgroup_auxiliary_tools_v2.out | 6 ++++++
.../input/resgroup/resgroup_io_limit.source | 10 ++++++++++
.../output/resgroup/resgroup_io_limit.source | 21 +++++++++++++++++++++
.../sql/resgroup/resgroup_auxiliary_tools_v2.sql | 13 +++++++++++++
6 files changed, 67 insertions(+), 6 deletions(-)
diff --git a/src/backend/commands/resgroupcmds.c b/src/backend/commands/resgroupcmds.c
index 6ebe1ac243e..9be40eba69f 100644
--- a/src/backend/commands/resgroupcmds.c
+++ b/src/backend/commands/resgroupcmds.c
@@ -486,7 +486,6 @@ AlterResourceGroup(AlterResourceGroupStmt *stmt)
validateCapabilities(pg_resgroupcapability_rel, groupid, &caps, false);
AssertImply(limitType != RESGROUP_LIMIT_TYPE_IO_LIMIT, caps.io_limit == NIL);
- AssertImply(limitType == RESGROUP_LIMIT_TYPE_IO_LIMIT, caps.io_limit != NIL);
/* cpuset & cpu_max_percent can not coexist.
* if cpuset is active, then cpu_max_percent must set to CPU_RATE_LIMIT_DISABLED,
@@ -519,6 +518,16 @@ AlterResourceGroup(AlterResourceGroupStmt *stmt)
updateResgroupCapabilityEntry(pg_resgroupcapability_rel,
groupid, RESGROUP_LIMIT_TYPE_IO_LIMIT,
0, cgroupOpsRoutine->dumpio(caps.io_limit));
+ else
+ {
+ /*
+ * When alter io_limit to -1 , the caps.io_limit will be nil.
+ * So we should update the io_limit in capability relation to -1.
+ */
+ updateResgroupCapabilityEntry(pg_resgroupcapability_rel,
+ groupid, RESGROUP_LIMIT_TYPE_IO_LIMIT,
+ 0, DefaultIOLimit);
+ }
}
else
{
diff --git a/src/backend/utils/resgroup/resgroup.c b/src/backend/utils/resgroup/resgroup.c
index 4a6c9c14b94..72f1ac75d41 100644
--- a/src/backend/utils/resgroup/resgroup.c
+++ b/src/backend/utils/resgroup/resgroup.c
@@ -807,11 +807,13 @@ ResGroupAlterOnCommit(const ResourceGroupCallbackContext *callbackCtx)
}
else if (callbackCtx->limittype == RESGROUP_LIMIT_TYPE_IO_LIMIT)
{
- if (callbackCtx->caps.io_limit != NIL)
- {
- cgroupOpsRoutine->cleario(callbackCtx->groupid);
- cgroupOpsRoutine->setio(callbackCtx->groupid, callbackCtx->caps.io_limit);
- }
+ /*
+ * When alter io_limit to -1 , the caps.io_limit will be nil.
+ * There are no errors in io_limit string when caps.io_limit is nil.
+ * When alter io_limit, caps.io_limit is nil means this resource group's io_limit should be clear.
+ */
+ cgroupOpsRoutine->cleario(callbackCtx->groupid);
+ cgroupOpsRoutine->setio(callbackCtx->groupid, callbackCtx->caps.io_limit);
}
/* reset default group if cpuset has changed */
diff --git a/src/test/isolation2/expected/resgroup/resgroup_auxiliary_tools_v2.out b/src/test/isolation2/expected/resgroup/resgroup_auxiliary_tools_v2.out
index bc5e8764091..de159f4910e 100644
--- a/src/test/isolation2/expected/resgroup/resgroup_auxiliary_tools_v2.out
+++ b/src/test/isolation2/expected/resgroup/resgroup_auxiliary_tools_v2.out
@@ -157,3 +157,9 @@ clear_io_max(groupid)
cgroup_path = "/sys/fs/cgroup/gpdb/%d/io.max" % groupid
return os.stat(cgroup_path).st_size == 0 $$ LANGUAGE plpython3u;
CREATE
+
+0: CREATE OR REPLACE FUNCTION check_io_max_empty(groupname text) RETURNS BOOL AS $$ import os
+# get group oid sql = "select groupid from gp_toolkit.gp_resgroup_config where groupname = '%s'" % groupname result = plpy.execute(sql) groupid = result[0]['groupid']
+cgroup_path = "/sys/fs/cgroup/gpdb/%d/io.max" % groupid
+return os.stat(cgroup_path).st_size == 0 $$ LANGUAGE plpython3u;
+CREATE
diff --git a/src/test/isolation2/input/resgroup/resgroup_io_limit.source b/src/test/isolation2/input/resgroup/resgroup_io_limit.source
index a09414f3f99..9a5fe2dc2f7 100644
--- a/src/test/isolation2/input/resgroup/resgroup_io_limit.source
+++ b/src/test/isolation2/input/resgroup/resgroup_io_limit.source
@@ -69,6 +69,15 @@ SELECT groupid, groupname, cpuset FROM gp_toolkit.gp_resgroup_config WHERE group
SELECT gp_inject_fault('create_resource_group_fail', 'reset', 1);
+-- clear limitations
+CREATE RESOURCE GROUP rg_test_group7 WITH (concurrency=10, cpu_max_percent=10, io_limit='rg_io_limit_ts_1:rbps=1000,wbps=1000');
+
+SELECT check_cgroup_io_max('rg_test_group7', 'rg_io_limit_ts_1', 'rbps=1048576000 wbps=1048576000 riops=max wiops=max');
+
+ALTER RESOURCE GROUP rg_test_group7 SET IO_LIMIT '-1';
+
+SELECT check_io_max_empty('rg_test_group7');
+
-- view
-- start_ignore
SELECT * from gp_toolkit.gp_resgroup_iostats_per_host;
@@ -82,6 +91,7 @@ DROP RESOURCE GROUP rg_test_group2;
DROP RESOURCE GROUP rg_test_group3;
DROP RESOURCE GROUP rg_test_group4;
DROP RESOURCE GROUP rg_test_group5;
+DROP RESOURCE GROUP rg_test_group7;
DROP TABLESPACE rg_io_limit_ts_1;
diff --git a/src/test/isolation2/output/resgroup/resgroup_io_limit.source b/src/test/isolation2/output/resgroup/resgroup_io_limit.source
index 3e7bd7f2134..ea7957bc1cf 100644
--- a/src/test/isolation2/output/resgroup/resgroup_io_limit.source
+++ b/src/test/isolation2/output/resgroup/resgroup_io_limit.source
@@ -137,6 +137,25 @@ SELECT gp_inject_fault('create_resource_group_fail', 'reset', 1);
Success:
(1 row)
+-- clear limitations
+CREATE RESOURCE GROUP rg_test_group7 WITH (concurrency=10, cpu_max_percent=10, io_limit='rg_io_limit_ts_1:rbps=1000,wbps=1000');
+CREATE
+
+SELECT check_cgroup_io_max('rg_test_group7', 'rg_io_limit_ts_1', 'rbps=1048576000 wbps=1048576000 riops=max wiops=max');
+ check_cgroup_io_max
+---------------------
+ t
+(1 row)
+
+ALTER RESOURCE GROUP rg_test_group7 SET IO_LIMIT '-1';
+ALTER
+
+SELECT check_io_max_empty('rg_test_group7');
+ check_io_max_empty
+--------------------
+ t
+(1 row)
+
-- view
-- start_ignore
SELECT * from gp_toolkit.gp_resgroup_iostats_per_host;
@@ -167,6 +186,8 @@ DROP RESOURCE GROUP rg_test_group4;
DROP
DROP RESOURCE GROUP rg_test_group5;
DROP
+DROP RESOURCE GROUP rg_test_group7;
+DROP
DROP TABLESPACE rg_io_limit_ts_1;
DROP
diff --git a/src/test/isolation2/sql/resgroup/resgroup_auxiliary_tools_v2.sql b/src/test/isolation2/sql/resgroup/resgroup_auxiliary_tools_v2.sql
index b762850468c..e274acbb8f3 100644
--- a/src/test/isolation2/sql/resgroup/resgroup_auxiliary_tools_v2.sql
+++ b/src/test/isolation2/sql/resgroup/resgroup_auxiliary_tools_v2.sql
@@ -373,3 +373,16 @@ $$ LANGUAGE plpython3u;
return os.stat(cgroup_path).st_size == 0
$$ LANGUAGE plpython3u;
+
+0: CREATE OR REPLACE FUNCTION check_io_max_empty(groupname text) RETURNS BOOL AS $$
+ import os
+
+ # get group oid
+ sql = "select groupid from gp_toolkit.gp_resgroup_config where groupname = '%s'" % groupname
+ result = plpy.execute(sql)
+ groupid = result[0]['groupid']
+
+ cgroup_path = "/sys/fs/cgroup/gpdb/%d/io.max" % groupid
+
+ return os.stat(cgroup_path).st_size == 0
+$$ LANGUAGE plpython3u;
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For additional commands, e-mail: commits-help@cloudberry.apache.org
(cloudberry) 08/11: delete cgroup leaf dir only when use group-v2 (#16830)
Posted by ma...@apache.org.
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 c4e0a80cd9d7ce9cc2a3d4f7e2da9438219cb354
Author: RMT <d....@outlook.com>
AuthorDate: Tue Dec 12 22:49:00 2023 +0800
delete cgroup leaf dir only when use group-v2 (#16830)
delete cgroup leaf dir only when use group-v2.
There is no leaf directory in gpdb cgroup when use cgroup v1, so the
rmdir(leaf_path) will always return non-zero values, then the rmdir(path)
will be ignored.
When drop some resource groups, when corresponding cgroup dir cannot be
removed because the rmdire(path) is not executed, this behavior will
cause the failure of CI.
This commit add some logic to check resource group version in deleteDir,
when use group-v1, rmdir(leaf_path) will be ignored.
---
src/backend/utils/resgroup/cgroup.c | 58 +++++++++++++++++++++++--------------
1 file changed, 36 insertions(+), 22 deletions(-)
diff --git a/src/backend/utils/resgroup/cgroup.c b/src/backend/utils/resgroup/cgroup.c
index 839090f39e4..504c8c2fcba 100644
--- a/src/backend/utils/resgroup/cgroup.c
+++ b/src/backend/utils/resgroup/cgroup.c
@@ -466,16 +466,20 @@ bool
deleteDir(Oid group, CGroupComponentType component, const char *filename, bool unassign,
void (*detachcgroup) (Oid group, CGroupComponentType component, int fd_dir))
{
-
char path[MAX_CGROUP_PATHLEN];
char leaf_path[MAX_CGROUP_PATHLEN];
size_t path_size = sizeof(path);
+ bool is_v2 = Gp_resource_manager_policy == RESOURCE_MANAGER_POLICY_GROUP_V2;
+ int path_cnt = 2;
+ char *paths[2] = {leaf_path, path};
int retry = unassign ? 0 : MAX_RETRY - 1;
int fd_dir;
+ int i;
buildPath(group, BASEDIR_GPDB, component, "", path, path_size);
- buildPath(group, BASEDIR_GPDB, component, CGROUPV2_LEAF_INDENTIFIER, leaf_path, path_size);
+ if (is_v2)
+ buildPath(group, BASEDIR_GPDB, component, CGROUPV2_LEAF_INDENTIFIER, leaf_path, path_size);
/*
* To prevent race condition between multiple processes we require a dir
@@ -494,37 +498,47 @@ deleteDir(Oid group, CGroupComponentType component, const char *filename, bool u
if (filename)
writeInt64(group, BASEDIR_GPDB, component, filename, 0);
+ if (!unassign)
+ detachcgroup = NULL;
+
+ i = is_v2 ? 0 : 1;
while (++retry <= MAX_RETRY)
{
- if (unassign)
+ if (detachcgroup)
detachcgroup(group, component, fd_dir);
- if (rmdir(leaf_path) || rmdir(path))
+ for (; i < path_cnt; ++i)
{
- int err = errno;
-
- if (err == EBUSY && unassign && retry < MAX_RETRY)
+ if (rmdir(paths[i]))
{
- elog(DEBUG1, "can't remove dir, will retry: %s: %s",
- path, strerror(err));
- pg_usleep(1000);
- continue;
+ int err = errno;
+
+ if (err == EBUSY && unassign && retry < MAX_RETRY)
+ {
+ elog(DEBUG1, "can't remove dir, will retry: %s: %s",
+ paths[i], strerror(err));
+ pg_usleep(1000);
+ break;
+ }
+
+ if (err != ENOENT)
+ {
+ elog(DEBUG1, "can't remove dir, ignore the error: %s: %s",
+ paths[i], strerror(err));
+ goto error;
+ }
}
- /*
- * we don't check for ENOENT again as we already acquired the lock
- * on this dir and the dir still exist at that time, so if then
- * it's removed by other processes then it's a bug.
- */
- elog(DEBUG1, "can't remove dir, ignore the error: %s: %s",
- path, strerror(err));
+ detachcgroup = NULL;
+
+ elog(DEBUG1, "cgroup dir '%s' removed", paths[i]);
}
- break;
- }
- if (retry <= MAX_RETRY)
- elog(DEBUG1, "cgroup dir '%s' removed", path);
+ if (i >= path_cnt)
+ break;
+ }
+error:
/* close() also releases the lock */
close(fd_dir);
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For additional commands, e-mail: commits-help@cloudberry.apache.org
(cloudberry) 04/11: Fix resource group io limit flaky case (#16386)
Posted by ma...@apache.org.
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 0735df44c3a1a142a8e9b51c9adb8536181ccfca
Author: RMT <d....@outlook.com>
AuthorDate: Wed Sep 6 21:41:14 2023 -0600
Fix resource group io limit flaky case (#16386)
Fix resource group io limit flaky case.
The flaky case caused by running mkdir on multi segments at the same
host.
Just catch FileExistsError and ignore it is ok, the mkdir function just
need the dir exists.
---
src/test/isolation2/expected/resgroup/resgroup_auxiliary_tools_v2.out | 2 +-
src/test/isolation2/sql/resgroup/resgroup_auxiliary_tools_v2.sql | 2 ++
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/test/isolation2/expected/resgroup/resgroup_auxiliary_tools_v2.out b/src/test/isolation2/expected/resgroup/resgroup_auxiliary_tools_v2.out
index 61e1e7474c2..e2e959374b2 100644
--- a/src/test/isolation2/expected/resgroup/resgroup_auxiliary_tools_v2.out
+++ b/src/test/isolation2/expected/resgroup/resgroup_auxiliary_tools_v2.out
@@ -138,7 +138,7 @@ CREATE
0: CREATE OR REPLACE FUNCTION mkdir(dirname text) RETURNS BOOL AS $$ import os
if os.path.exists(dirname): return True
-try: os.makedirs(dirname) except Exception as e: plpy.error("cannot create dir {}".format(e)) else: return True $$ LANGUAGE plpython3u;
+try: os.makedirs(dirname) except FileExistsError: return True except Exception as e: plpy.error("cannot create dir {}".format(e)) else: return True $$ LANGUAGE plpython3u;
CREATE
0: CREATE OR REPLACE FUNCTION rmdir(dirname text) RETURNS BOOL AS $$ import shutil import fcntl import os
diff --git a/src/test/isolation2/sql/resgroup/resgroup_auxiliary_tools_v2.sql b/src/test/isolation2/sql/resgroup/resgroup_auxiliary_tools_v2.sql
index 6a60a32e3f7..88f66c4f18d 100644
--- a/src/test/isolation2/sql/resgroup/resgroup_auxiliary_tools_v2.sql
+++ b/src/test/isolation2/sql/resgroup/resgroup_auxiliary_tools_v2.sql
@@ -320,6 +320,8 @@ $$ LANGUAGE plpython3u;
try:
os.makedirs(dirname)
+ except FileExistsError:
+ return True
except Exception as e:
plpy.error("cannot create dir {}".format(e))
else:
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For additional commands, e-mail: commits-help@cloudberry.apache.org