From 3fb2e8739da2e4e05642fb4e82bca280b7163344 Mon Sep 17 00:00:00 2001 From: Xiaoli Zhou Date: Fri, 14 Jun 2024 14:15:49 +0800 Subject: [PATCH 1/3] fix(interactive): Fix Result Parser Issue of Gremlin `select('a', 'b').by(valueMap())` (#3919) --- .../rex/operator/SqlMapValueConstructor.java | 1 + .../common/ir/type/GraphTypeFactoryImpl.java | 50 +++++++++++++++++++ .../gremlin/antlr4x/GraphBuilderTest.java | 13 +++++ 3 files changed, 64 insertions(+) diff --git a/interactive_engine/compiler/src/main/java/com/alibaba/graphscope/common/ir/rex/operator/SqlMapValueConstructor.java b/interactive_engine/compiler/src/main/java/com/alibaba/graphscope/common/ir/rex/operator/SqlMapValueConstructor.java index 8cb3f114ddf6..e6f81796694d 100644 --- a/interactive_engine/compiler/src/main/java/com/alibaba/graphscope/common/ir/rex/operator/SqlMapValueConstructor.java +++ b/interactive_engine/compiler/src/main/java/com/alibaba/graphscope/common/ir/rex/operator/SqlMapValueConstructor.java @@ -42,6 +42,7 @@ public SqlMapValueConstructor() { super("MAP", SqlKind.MAP_VALUE_CONSTRUCTOR); } + @Override public RelDataType inferReturnType(SqlOperatorBinding opBinding) { RelDataTypeFactory typeFactory = opBinding.getTypeFactory(); List argTypes = opBinding.collectOperandTypes(); diff --git a/interactive_engine/compiler/src/main/java/com/alibaba/graphscope/common/ir/type/GraphTypeFactoryImpl.java b/interactive_engine/compiler/src/main/java/com/alibaba/graphscope/common/ir/type/GraphTypeFactoryImpl.java index 91a13333fefb..f31c408545ec 100644 --- a/interactive_engine/compiler/src/main/java/com/alibaba/graphscope/common/ir/type/GraphTypeFactoryImpl.java +++ b/interactive_engine/compiler/src/main/java/com/alibaba/graphscope/common/ir/type/GraphTypeFactoryImpl.java @@ -18,6 +18,7 @@ import com.alibaba.graphscope.common.config.Configs; import com.alibaba.graphscope.common.config.FrontendConfig; +import com.google.common.collect.Lists; import org.apache.calcite.jdbc.JavaTypeFactoryImpl; import org.apache.calcite.rel.type.RelDataType; @@ -86,6 +87,55 @@ public RelDataType createArbitraryMapType( } return types.get(0); } + if (types.stream().anyMatch(t -> t instanceof ArbitraryMapType)) { + return leastRestrictiveForArbitraryMapType(types); + } return super.leastRestrictive(types); } + + // re-implement lease-restrictive type inference for arbitrary map types + // for each key type and value type, check if they have a least-restrictive type, otherwise + // return null + private @Nullable RelDataType leastRestrictiveForArbitraryMapType(List types) { + boolean isNullable = false; + List> leastKeyTypes = Lists.newArrayList(); + List> leastValueTypes = Lists.newArrayList(); + for (RelDataType type : types) { + if (!(type instanceof ArbitraryMapType)) return null; + ArbitraryMapType mapType = (ArbitraryMapType) type; + if (mapType.isNullable()) isNullable = true; + if (leastKeyTypes.isEmpty() || leastValueTypes.isEmpty()) { + for (RelDataType keyType : mapType.getKeyTypes()) { + leastKeyTypes.add(Lists.newArrayList(keyType)); + } + for (RelDataType valueType : mapType.getValueTypes()) { + leastValueTypes.add(Lists.newArrayList(valueType)); + } + } else { + if (leastKeyTypes.size() != mapType.getKeyTypes().size() + || leastValueTypes.size() != mapType.getValueTypes().size()) { + return null; + } + for (int i = 0; i < leastKeyTypes.size(); i++) { + leastKeyTypes.get(i).add(mapType.getKeyTypes().get(i)); + } + for (int i = 0; i < leastValueTypes.size(); i++) { + leastValueTypes.get(i).add(mapType.getValueTypes().get(i)); + } + } + } + List mapKeyTypes = Lists.newArrayList(); + for (List leastKeyType : leastKeyTypes) { + RelDataType type = leastRestrictive(leastKeyType); + if (type == null) return null; + mapKeyTypes.add(type); + } + List mapValueTypes = Lists.newArrayList(); + for (List leastValueType : leastValueTypes) { + RelDataType type = leastRestrictive(leastValueType); + if (type == null) return null; + mapValueTypes.add(type); + } + return createArbitraryMapType(mapKeyTypes, mapValueTypes, isNullable); + } } diff --git a/interactive_engine/compiler/src/test/java/com/alibaba/graphscope/gremlin/antlr4x/GraphBuilderTest.java b/interactive_engine/compiler/src/test/java/com/alibaba/graphscope/gremlin/antlr4x/GraphBuilderTest.java index 26f5a7d6bb1e..0fe8105ad9da 100644 --- a/interactive_engine/compiler/src/test/java/com/alibaba/graphscope/gremlin/antlr4x/GraphBuilderTest.java +++ b/interactive_engine/compiler/src/test/java/com/alibaba/graphscope/gremlin/antlr4x/GraphBuilderTest.java @@ -1701,4 +1701,17 @@ public void g_V_path_expand_until_age_gt_30_values_age() { + " person]}], alias=[_], opt=[VERTEX])", node.explain().trim()); } + + @Test + public void g_V_select_a_b_valueMap() { + RelNode rel = + eval( + "g.V().hasLabel('person').as('a').out('knows').as('b').select('a'," + + " 'b').by(valueMap())"); + Assert.assertEquals( + "([CHAR(1), CHAR(1)], [([CHAR(2), CHAR(4), CHAR(3)], [BIGINT, CHAR(1), INTEGER])" + + " MAP, ([CHAR(2), CHAR(4), CHAR(4), CHAR(12), CHAR(3)], [BIGINT, CHAR(1)," + + " CHAR(1), DATE, INTEGER]) MAP]) MAP", + rel.getRowType().getFieldList().get(0).getType().toString()); + } } From 674a6aa1252ae2b3eb92560e6d840e64e162a462 Mon Sep 17 00:00:00 2001 From: Xiaoli Zhou Date: Fri, 14 Jun 2024 14:16:03 +0800 Subject: [PATCH 2/3] fix(interactive): Refine Query Timeout Configuration in GIE Doc (#3917) --- charts/gie-standalone/templates/frontend/statefulset.yaml | 4 ++-- charts/gie-standalone/values.yaml | 4 ++-- docs/interactive_engine/dev_and_test.md | 4 +++- docs/interactive_engine/getting_started.md | 4 ++-- docs/storage_engine/groot.md | 1 + interactive_engine/compiler/set_properties.sh | 4 ++-- .../com/alibaba/graphscope/common/config/PegasusConfig.java | 3 --- 7 files changed, 12 insertions(+), 12 deletions(-) diff --git a/charts/gie-standalone/templates/frontend/statefulset.yaml b/charts/gie-standalone/templates/frontend/statefulset.yaml index 96c3d220bee5..e6fbb25386cb 100644 --- a/charts/gie-standalone/templates/frontend/statefulset.yaml +++ b/charts/gie-standalone/templates/frontend/statefulset.yaml @@ -117,8 +117,8 @@ spec: value: {{ .Values.extraConfig | quote }} - name: WORKER_NUM value: {{ .Values.pegasusWorkerNum | quote }} - - name: TIMEOUT - value: {{ .Values.pegasusTimeout | quote }} + - name: QUERY_TIMEOUT + value: {{ .Values.queryExecutionTimeoutMS | quote }} - name: BATCH_SIZE value: {{ .Values.pegasusBatchSize | quote }} - name: OUTPUT_CAPACITY diff --git a/charts/gie-standalone/values.yaml b/charts/gie-standalone/values.yaml index 79c121eb4574..524b6930af8b 100644 --- a/charts/gie-standalone/values.yaml +++ b/charts/gie-standalone/values.yaml @@ -36,8 +36,6 @@ htapLoaderConfig: "v6d_modern_loader.json" ## Pegasus Config pegasusWorkerNum: 2 -pegasusTimeout: 240000 - pegasusBatchSize: 1024 pegasusOutputCapacity: 16 @@ -46,6 +44,8 @@ frontendQueryPerSecondLimit: 2147483647 gremlinScriptLanguageName: "antlr_gremlin_traversal" +queryExecutionTimeoutMS: 3000000 + ## need by vineyard in distributed env etcdEndpoint: "etcd-for-vineyard.default.svc.cluster.local:2379" diff --git a/docs/interactive_engine/dev_and_test.md b/docs/interactive_engine/dev_and_test.md index d6f74f9e90df..1b4568c03421 100644 --- a/docs/interactive_engine/dev_and_test.md +++ b/docs/interactive_engine/dev_and_test.md @@ -105,7 +105,6 @@ $GIE_TEST_HOME/bin/gaia_executor $GIE_TEST_HOME/conf/log4rs.yml $GIE_TEST_HOME/c ## Pegasus service config # a.k.a. thread num pegasus.worker.num = 1 -pegasus.timeout = 240000 pegasus.batch.size = 1024 pegasus.output.capacity = 16 @@ -126,6 +125,9 @@ neo4j.bolt.server.port = 7687 # disable authentication if username or password is not set # auth.username = default # auth.password = default + +# set total execution time for a query +query.execution.timeout.ms: 3000000 ``` 6. Start the `frontend`: diff --git a/docs/interactive_engine/getting_started.md b/docs/interactive_engine/getting_started.md index 664614a7fdf6..7aa43c0c253b 100644 --- a/docs/interactive_engine/getting_started.md +++ b/docs/interactive_engine/getting_started.md @@ -98,8 +98,8 @@ The number 6 is printed, which is the number of vertices in modern graph. You could pass additional key-value pairs to customize the startup configuration of GIE, for example: ```python -# Set the timeout value to 10 min -g = gs.interactive(graph, params={'query.execution.timeout.ms': 600000}) +# set total execution time for a query to 3000s +g = gs.interactive(graph, params={'query.execution.timeout.ms': 3000000}) ``` ## What's the Next diff --git a/docs/storage_engine/groot.md b/docs/storage_engine/groot.md index 0f87adc7851f..727bedffd2fb 100644 --- a/docs/storage_engine/groot.md +++ b/docs/storage_engine/groot.md @@ -75,6 +75,7 @@ helm status demo | frontend.replicaCount | Number of Frontend | 1 | | frontend.service.type | Kubernetes Service type of frontend | NodePort | | frontend.query.per.second.limit | the maximum qps can be handled by frontend service | 2147483647 (without limitation) | +| query.execution.timeout.ms | the total execution time for a query | 3000000 | If Groot is launched with the default configuration, then two Store Pods, one Frontend Pod, and one Coordinator Pod will be started. The number of Coordinator nodes is fixed to 1. diff --git a/interactive_engine/compiler/set_properties.sh b/interactive_engine/compiler/set_properties.sh index 20fe83649343..2e9daa46f796 100755 --- a/interactive_engine/compiler/set_properties.sh +++ b/interactive_engine/compiler/set_properties.sh @@ -16,7 +16,7 @@ worker_num="pegasus.worker.num: $WORKER_NUM"; -timeout="pegasus.timeout: $TIMEOUT" +query_timeout="query.execution.timeout.ms: $QUERY_TIMEOUT" batch_size="pegasus.batch.size: $BATCH_SIZE"; @@ -47,6 +47,6 @@ done graph_schema="graph.schema: $GRAPH_SCHEMA" -properties="$worker_num\n$timeout\n$batch_size\n$output_capacity\n$hosts\n$server_num\n$graph_schema\n$gremlin_server_port\n$cypher_server_port\n$frontend_query_per_second_limit\n$gremlin_script_language_name\n$graph_physical_opt" +properties="$worker_num\n$query_timeout\n$batch_size\n$output_capacity\n$hosts\n$server_num\n$graph_schema\n$gremlin_server_port\n$cypher_server_port\n$frontend_query_per_second_limit\n$gremlin_script_language_name\n$graph_physical_opt" echo -e $properties > ./conf/ir.compiler.properties diff --git a/interactive_engine/compiler/src/main/java/com/alibaba/graphscope/common/config/PegasusConfig.java b/interactive_engine/compiler/src/main/java/com/alibaba/graphscope/common/config/PegasusConfig.java index f06765a7ddf5..b82ed8eeccaa 100644 --- a/interactive_engine/compiler/src/main/java/com/alibaba/graphscope/common/config/PegasusConfig.java +++ b/interactive_engine/compiler/src/main/java/com/alibaba/graphscope/common/config/PegasusConfig.java @@ -31,7 +31,4 @@ public class PegasusConfig { public static final Config PEGASUS_HOSTS = Config.stringConfig("pegasus.hosts", "localhost:8080"); - - public static final Config PEGASUS_GRPC_TIMEOUT = - Config.longConfig("pegasus.grpc.timeout", 6000000L); } From 87054c4a20df40952480a485b5ab6d6c9d52c044 Mon Sep 17 00:00:00 2001 From: Xiaoli Zhou Date: Fri, 14 Jun 2024 14:18:41 +0800 Subject: [PATCH 3/3] fix(interactive): Fix Gremlin Alias Issue in GraphBuilder (#3918) Co-authored-by: BingqingLyu --- .../graphscope/common/ir/tools/GraphBuilder.java | 6 ++++++ .../graphscope/gremlin/antlr4x/GraphBuilderTest.java | 11 +++++++++++ 2 files changed, 17 insertions(+) diff --git a/interactive_engine/compiler/src/main/java/com/alibaba/graphscope/common/ir/tools/GraphBuilder.java b/interactive_engine/compiler/src/main/java/com/alibaba/graphscope/common/ir/tools/GraphBuilder.java index 8c7485178992..579881448618 100644 --- a/interactive_engine/compiler/src/main/java/com/alibaba/graphscope/common/ir/tools/GraphBuilder.java +++ b/interactive_engine/compiler/src/main/java/com/alibaba/graphscope/common/ir/tools/GraphBuilder.java @@ -1749,7 +1749,9 @@ public GraphBuilder as(String alias) { RelNode top = requireNonNull(peek(), "frame stack is empty"); // skip intermediate operations which make no changes to the row type, i.e. // filter/limit/dedup... + RelNode parent = null; while (!top.getInputs().isEmpty() && top.getInput(0).getRowType() == top.getRowType()) { + parent = top; top = top.getInput(0); } if (top instanceof AbstractBindableTableScan @@ -1832,6 +1834,10 @@ public GraphBuilder as(String alias) { aggregate(aggregate.getGroupKey(), ImmutableList.of(aggCall.as(alias))); } } + if (parent != null && peek() != top) { + parent.replaceInput(0, build()); + push(parent); + } } return this; } diff --git a/interactive_engine/compiler/src/test/java/com/alibaba/graphscope/gremlin/antlr4x/GraphBuilderTest.java b/interactive_engine/compiler/src/test/java/com/alibaba/graphscope/gremlin/antlr4x/GraphBuilderTest.java index 0fe8105ad9da..4af926289233 100644 --- a/interactive_engine/compiler/src/test/java/com/alibaba/graphscope/gremlin/antlr4x/GraphBuilderTest.java +++ b/interactive_engine/compiler/src/test/java/com/alibaba/graphscope/gremlin/antlr4x/GraphBuilderTest.java @@ -1702,6 +1702,16 @@ public void g_V_path_expand_until_age_gt_30_values_age() { node.explain().trim()); } + @Test + public void g_V_has_label_person_limit_10_as_a() { + RelNode node = eval("g.V().hasLabel('person').limit(10).as('a')"); + Assert.assertEquals( + "GraphLogicalProject($f0=[_], isAppend=[false])\n" + + " GraphLogicalSort(fetch=[10])\n" + + " GraphLogicalSource(tableConfig=[{isAll=false, tables=[person]}]," + + " alias=[a], opt=[VERTEX])", + node.explain().trim()); + @Test public void g_V_select_a_b_valueMap() { RelNode rel = @@ -1713,5 +1723,6 @@ public void g_V_select_a_b_valueMap() { + " MAP, ([CHAR(2), CHAR(4), CHAR(4), CHAR(12), CHAR(3)], [BIGINT, CHAR(1)," + " CHAR(1), DATE, INTEGER]) MAP]) MAP", rel.getRowType().getFieldList().get(0).getType().toString()); + } }