diff --git a/src/common/datatypes/HostAddr.h b/src/common/datatypes/HostAddr.h index 1af8ddcfb19..b326d216d68 100644 --- a/src/common/datatypes/HostAddr.h +++ b/src/common/datatypes/HostAddr.h @@ -40,7 +40,13 @@ struct HostAddr { std::string toString() const { std::stringstream os; - os << "\"" << host << ":" << port << "\""; + os << "\"" << host << "\":" << port; + return os.str(); + } + + std::string toRawString() const { + std::stringstream os; + os << host << ":" << port; return os.str(); } diff --git a/src/graph/util/Utils.cpp b/src/graph/util/Utils.cpp index 864145e2944..6442a336bc8 100644 --- a/src/graph/util/Utils.cpp +++ b/src/graph/util/Utils.cpp @@ -21,7 +21,7 @@ folly::dynamic collectRespProfileData(const storage::cpp2::ResponseCommon& resp, size_t numVertices, size_t totalRpcTime) { folly::dynamic stat = folly::dynamic::object(); - stat.insert("address", std::get<0>(info).toString()); + stat.insert("host", std::get<0>(info).toRawString()); stat.insert("exec", folly::sformat("{}(us)", std::get<1>(info))); stat.insert("total", folly::sformat("{}(us)", std::get<2>(info))); if (numVertices > 0) { diff --git a/tests/common/plan_differ.py b/tests/common/plan_differ.py index d496979dc2f..19cae12dc81 100644 --- a/tests/common/plan_differ.py +++ b/tests/common/plan_differ.py @@ -11,6 +11,7 @@ class PlanDiffer: NAME = "name" DEPENDS = "dependencies" OP_INFO = "operator info" + PROFILING_DATA = "profiling data" PATTERN = re.compile(r"\{\"loopBody\": \"(\d+)\"\}") def __init__(self, resp, expect): @@ -48,6 +49,7 @@ def _diff_plan_node(self, plan_desc, line_num, rows, column_names) -> bool: op = expect_node[column_names.index(self.OP_INFO)] res = self.PATTERN.match(op) if not res: + self._err_msg = "Could not find 'loopBody' info in operator info of the Loop" return False body_id = int(res.group(1)) loop_body_idx = self._loop_body(plan_desc, @@ -60,17 +62,24 @@ def _diff_plan_node(self, plan_desc, line_num, rows, column_names) -> bool: elif self._is_same_node(name, "Select"): # TODO(yee): check select node pass - elif self.OP_INFO in column_names: + + if self.OP_INFO in column_names: op = expect_node[column_names.index(self.OP_INFO)] # Parse expected operator info jsonStr to dict - expect_op_dict = {} - if op: - expect_op_dict = json.loads(op) + expect_op_dict = json.loads(op) if op else {} self._err_msg = self._check_op_info( expect_op_dict, plan_node_desc.description) if self._err_msg: return False + if self.PROFILING_DATA in column_names: + profiling_data = expect_node[column_names.index(self.PROFILING_DATA)] + expect_profiling_data = json.loads(profiling_data) if profiling_data else {} + self._err_msg = self._check_profiling_data( + expect_profiling_data, plan_node_desc.profiles) + if self._err_msg: + return False + if plan_node_desc.dependencies is None: return True @@ -89,8 +98,7 @@ def _diff_plan_node(self, plan_desc, line_num, rows, column_names) -> bool: def _check_op_info(self, exp, resp): if resp is None: - if exp: - return f"expect: {exp} but resp plan node is None" + return f"expect: {exp} but resp plan node is None" if exp else None if exp: resp_dict = { f"{bytes.decode(pair.key)}": f"{bytes.decode(pair.value)}" @@ -101,6 +109,56 @@ def _check_op_info(self, exp, resp): json.dumps(exp), json.dumps(resp_dict)) return None + def _check_profiling_other_stats(self, exp, resp): + if type(exp) != type(resp): + return False + if isinstance(exp, dict): + return self._is_subdict_nested(exp, resp) + return exp == resp + + def _check_profiling_stats(self, exp, resp, version): + if not isinstance(exp, dict): + return False + other_stats = resp.other_stats if resp.other_stats else {} + for k,v in exp.items(): + if k == "version": + if int(v) != version : + return False + elif hasattr(resp, k): + if getattr(resp, k) != v: + return False + else: + if isinstance(k, str): + k = k.encode() + if k not in other_stats: + return False + val = other_stats[k] + try: + val = json.loads(val) + except: + try: + val = val.decode() + except: + pass + if not self._check_profiling_other_stats(v, val): + return False + return True + + def _check_profiling_data(self, exp, resp): + if resp is None: + return f"expect: {exp} but resp profiling data is None" if exp else None + if not exp: + return None + if isinstance(resp, list) and len(resp) > 1: + if (not isinstance(exp, list)) or len(exp) != len(resp): + return f"Expected profiling data has invalid length: {len(exp)} vs. {len(resp)}" + for i, r in enumerate(resp): + if not self._check_profiling_stats(exp[i], r, i): + return f"Fail to diff {json.dumps(exp[i])} and {r}, i: {i}" + elif not self._check_profiling_stats(exp, resp[0], 0): + return f"Fail to diff {json.dumps(exp)} and {resp[0]}" + return None + def _is_same_node(self, lhs: str, rhs: str) -> bool: return lhs.lower().startswith(rhs.lower()) @@ -142,7 +200,7 @@ def _try_convert_json(j): extracted_resp_dict[k] = _try_convert_json(resp[k]) else: extracted_resp_dict = self._convert_jsonStr_to_dict(resp, key_list) - + for k in extracted_expected_dict: extracted_expected_dict[k] = _try_convert_json(extracted_expected_dict[k]) @@ -156,7 +214,7 @@ def _is_subdict(small, big): return not bool(diff) return _is_subdict(extracted_expected_dict, extracted_resp_dict) - + # resp: pair(key, jsonStr) def _convert_jsonStr_to_dict(self, resp, key_list): resp_json_str = '' diff --git a/tests/tck/conftest.py b/tests/tck/conftest.py index 8162bdfc7b8..4a88ea03273 100644 --- a/tests/tck/conftest.py +++ b/tests/tck/conftest.py @@ -822,7 +822,6 @@ def check_plan(request, plan, exec_ctx): f"Location: {location}", differ.err_msg(), ] - assert res, "\n".join(msg) diff --git a/tests/tck/features/explain/ExplainAndProfile.feature b/tests/tck/features/explain/ExplainAndProfile.feature index 4057bed2592..926094493e1 100644 --- a/tests/tck/features/explain/ExplainAndProfile.feature +++ b/tests/tck/features/explain/ExplainAndProfile.feature @@ -70,3 +70,22 @@ Feature: Explain and Profile | explain | | EXPLAIN | | PROFILE | + + Scenario: Test profiling data format + When profiling query: + """ + GO 4 STEPS FROM 'Tim Duncan' OVER like YIELD like._dst AS dst | YIELD count(*) + """ + Then the result should be, in any order: + | count(*) | + | 6 | + And the execution plan should be: + | id | name | dependencies | profiling data | operator info | + | 7 | Aggregate | 6 | {"version":0, "rows": 1} | | + | 6 | Project | 5 | {"version":0, "rows": 6} | | + | 5 | GetNeighbors | 4 | {"version":0, "rows": 6, "resp[0]": {"vertices": 3}} | | + | 4 | Loop | 0 | [{"version":0, "rows": 1},{"version":1, "rows": 1},{"version":2, "rows": 1},{"version":3, "rows": 1}] | {"loopBody": "3"} | + | 3 | Dedup | 2 | [{"version":0, "rows": 2},{"version":1, "rows": 3},{"version":2, "rows": 3}] | | + | 2 | GetDstBySrc | 1 | [{"version":0, "rows": 2, "resp[0]": {"vertices": 2}},{"version":1, "rows": 3, "resp[0]":{"vertices": 3}}, {"version":2, "rows": 3, "resp[0]":{"vertices": 3}}] | | + | 1 | Start | | [{"version":0, "rows": 0},{"version":1, "rows": 0},{"version":2, "rows": 0}] | | + | 0 | Start | | {"version":0, "rows": 0} | |