feat: support task auto manage by server role state machine by zyxxoo · Pull Request #2130 · apache/hugegraph

zyxxoo · 2023-02-26T11:25:32Z

No description provided.

github-actions

license-eye has totally checked 893 files.

Valid	Invalid	Ignored	Fixed
670	3	220	0

Click to see the invalid file list

hugegraph-api/src/main/java/org/apache/hugegraph/core/RoleTypeDataAdapterImpl.java
hugegraph-api/src/main/java/org/apache/hugegraph/core/StateMachineCallbackImpl.java
hugegraph-core/src/main/java/org/apache/hugegraph/election/HugeRoleStateMachineConfig.java

github-actions · 2023-02-26T11:26:33Z

@@ -0,0 +1,146 @@
+package org.apache.hugegraph.core;


Suggested change

package org.apache.hugegraph.core;

/*

* Licensed to the Apache Software Foundation (ASF) under one or more

* contributor license agreements. See the NOTICE file distributed with

* this work for additional information regarding copyright ownership.

* The ASF licenses this file to You under the Apache License, Version 2.0

* (the "License"); you may not use this file except in compliance with

* the License. You may obtain a copy of the License at

* http://www.apache.org/licenses/LICENSE-2.0

* Unless required by applicable law or agreed to in writing, software

* distributed under the License is distributed on an "AS IS" BASIS,

* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.

* See the License for the specific language governing permissions and

* limitations under the License.

*/

package org.apache.hugegraph.core;

github-actions · 2023-02-26T11:26:33Z

@@ -0,0 +1,74 @@
+package org.apache.hugegraph.core;


Suggested change

package org.apache.hugegraph.core;

/*

* Licensed to the Apache Software Foundation (ASF) under one or more

* contributor license agreements. See the NOTICE file distributed with

* this work for additional information regarding copyright ownership.

* The ASF licenses this file to You under the Apache License, Version 2.0

* (the "License"); you may not use this file except in compliance with

* the License. You may obtain a copy of the License at

* http://www.apache.org/licenses/LICENSE-2.0

* Unless required by applicable law or agreed to in writing, software

* distributed under the License is distributed on an "AS IS" BASIS,

* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.

* See the License for the specific language governing permissions and

* limitations under the License.

*/

package org.apache.hugegraph.core;

github-actions · 2023-02-26T11:26:33Z

@@ -0,0 +1,53 @@
+package org.apache.hugegraph.election;


Suggested change

package org.apache.hugegraph.election;

/*

* Licensed to the Apache Software Foundation (ASF) under one or more

* contributor license agreements. See the NOTICE file distributed with

* this work for additional information regarding copyright ownership.

* The ASF licenses this file to You under the Apache License, Version 2.0

* (the "License"); you may not use this file except in compliance with

* the License. You may obtain a copy of the License at

* http://www.apache.org/licenses/LICENSE-2.0

* Unless required by applicable law or agreed to in writing, software

* distributed under the License is distributed on an "AS IS" BASIS,

* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.

* See the License for the specific language governing permissions and

* limitations under the License.

*/

package org.apache.hugegraph.election;

codecov · 2023-02-26T12:12:34Z

Codecov Report

Merging #2130 (5523064) into master (76fa644) will increase coverage by 6.25%.
The diff coverage is 79.52%.

@@             Coverage Diff              @@
##             master    #2130      +/-   ##
============================================
+ Coverage     62.60%   68.86%   +6.25%     
- Complexity      728      977     +249     
============================================
  Files           481      488       +7     
  Lines         39704    40090     +386     
  Branches       5581     5608      +27     
============================================
+ Hits          24857    27607    +2750     
+ Misses        12389     9824    -2565     
- Partials       2458     2659     +201

Impacted Files	Coverage Δ
...a/org/apache/hugegraph/api/gremlin/GremlinAPI.java	`100.00% <ø> (ø)`
...ava/org/apache/hugegraph/api/job/AlgorithmAPI.java	`0.00% <ø> (ø)`
...java/org/apache/hugegraph/api/job/ComputerAPI.java	`0.00% <ø> (ø)`
.../java/org/apache/hugegraph/api/job/GremlinAPI.java	`50.72% <ø> (ø)`
.../java/org/apache/hugegraph/api/job/RebuildAPI.java	`42.85% <ø> (ø)`
...ain/java/org/apache/hugegraph/api/job/TaskAPI.java	`52.08% <ø> (ø)`
...in/java/org/apache/hugegraph/api/raft/RaftAPI.java	`17.30% <ø> (ø)`
...org/apache/hugegraph/api/schema/IndexLabelAPI.java	`75.30% <0.00%> (-0.95%)`	⬇️
...ava/org/apache/hugegraph/api/schema/SchemaAPI.java	`100.00% <ø> (ø)`
...rg/apache/hugegraph/api/schema/VertexLabelAPI.java	`59.72% <0.00%> (-1.71%)`	⬇️
... and 82 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

javeme · 2023-02-26T13:09:59Z

+
+    private final TaskManager taskManager;
+
+    boolean isMaster = false;


mark private volatile?

the filed only use on one thread, so don't volatile

javeme · 2023-02-26T13:15:19Z

+import org.apache.hugegraph.util.Log;
+import org.slf4j.Logger;
+
+public class StateMachineCallbackImpl implements StateMachineCallback {


not submit StateMachineCallback file?

This file is mainly responsible for state machine and external interactions, so it was not previously committed

javeme · 2023-02-26T13:37:14Z

+import org.apache.tinkerpop.gremlin.structure.T;
+import org.apache.tinkerpop.gremlin.structure.Vertex;
+
+public class RoleTypeDataAdapterImpl implements RoleTypeDataAdapter {


RoleTypeDataAdapter means RoleTypeStore?

yes, the adapter responsibility mainly applies to storage

javeme · 2023-02-27T15:03:54Z

+        E.checkArgument(this.roleStateWorker == null, "Repetition init");
+        this.applyThread = Executors.newSingleThreadExecutor();
+        this.roleStateWorker = this.authenticator().graph().roleElectionStateMachine();
+        this.applyThread.execute(() -> {


can move applyThread to RoleElectionStateMachine class?

It's easier to write a single test with asynchronous threads outside

simon824 · 2023-02-28T01:51:19Z

Hi @zyxxoo , in the previous version, master schedule tasks and if the task is sent to the worker, will cause Exception 'Can't schedule task on non-master server'.
Will the worker forward the task to the master in this feature？ because the master role will change, it is difficult to determine the master ip.

zyxxoo · 2023-02-28T02:04:43Z

Hi @zyxxoo , in the previous version, master schedule tasks and if the task is sent to the worker, will cause Exception 'Can't schedule task on non-master server'. Will the worker forward the task to the master in this feature？ because the master role will change, it is difficult to determine the master ip.

yes, we will support redirect request to master node in this feature

javeme · 2023-02-28T14:13:43Z

+
+    @Override
+    public void filter(ContainerRequestContext requestContext) throws IOException {
+        GlobalMasterInfo instance = GlobalMasterInfo.instance();


get from graphManager.auth.masterInfo?

I have change to "graphManager.globalMasterInfo"

javeme · 2023-02-28T14:16:49Z

+                    "server.role.node_external_url",
+                    "The url of external accessibility",
+                    disallowEmpty(),
+                    "127.0.0.1:8080"


we can pass it from restserver config to graph config, like rpc url

I'm a little confused about the question raised
now, I put the config to resetserver, but the stateMachine put in graph instance, so these config must be copy to graph config

javeme · 2023-03-01T13:03:26Z

    @Status(Status.ACCEPTED)
    @Produces(APPLICATION_JSON_WITH_CHARSET)
    @RolesAllowed({"admin", "$owner=$graph $action=index_write"})
+    @RedirectFilter.RedirectMasterRole


maybe ACCEPTED and RedirectMasterRole often appear simultaneously

yes, if ACCEPTED appear, the RedirectMasterRole also appear

imbajin · 2023-03-07T11:09:28Z

            E.checkArgumentNotNull(this.state, "State don't be null");
            try {
+                RoleState pre = this.state;
                this.state = state.transform(context);


Note: state.transform is not an atomic operation

the machine run in single thread

taskManager and globalMadterInfo are use lock to sync

imbajin · 2023-03-07T11:21:20Z

@@ -234,7 +240,7 @@ public boolean updateIfNodePresent(RoleTypeData stateData) {
                    if (Objects.equals(value.node(), copy.node()) &&


if the value could be null here?

the value dont become null

wuchaojing · 2023-04-11T06:53:24Z

use hubble, submit task still fail on the worker, but success on the master

zyxxoo · 2023-04-12T02:55:33Z

use hubble, submit task still fail on the worker, but success on the master

目前在拦截器这一层做了转发，但是这期间如果发生了 master 切换，因为缓存原因还是会转发到旧的节点，这时候就会有这个错误，这里考虑性能并不会转发多次，可以考虑重试下这个语句应该就不会报错了

zyxxoo · 2023-04-12T03:01:33Z

不过目前来看， g.V().limit() 这条语句应该涉及不到 task, 这里应该有 bug

wuchaojing · 2023-04-12T03:05:24Z

use hubble, submit task still fail on the worker, but success on the master

目前在拦截器这一层做了转发，但是这期间如果发生了 master 切换，因为缓存原因还是会转发到旧的节点，这时候就会有这个错误，这里考虑性能并不会转发多次，可以考虑重试下这个语句应该就不会报错了

集群run起来之后没有发生master切换，多次尝试g.V().limit()还是会失败

wuchaojing · 2023-04-12T03:06:14Z

不过目前来看， g.V().limit() 这条语句应该涉及不到 task, 这里应该有 bug

我理解执行任务就会生成一个task吧，跟语句本身没有关系吧

zyxxoo · 2023-04-12T03:18:43Z

不过目前来看， g.V().limit() 这条语句应该涉及不到 task, 这里应该有 bug

我理解执行任务就会生成一个task吧，跟语句本身没有关系吧

ok, 看着这里执行任务应该创建了一个task

执行这条语句一直失败吗？后端用什么存储呢？

zyxxoo · 2023-04-12T03:23:05Z

use hubble, submit task still fail on the worker, but success on the master

目前在拦截器这一层做了转发，但是这期间如果发生了 master 切换，因为缓存原因还是会转发到旧的节点，这时候就会有这个错误，这里考虑性能并不会转发多次，可以考虑重试下这个语句应该就不会报错了

集群run起来之后没有发生master切换，多次尝试g.V().limit()还是会失败

估计是拦截器漏加了，这个是指定方法才会生效

wuchaojing · 2023-04-12T03:26:38Z

不过目前来看， g.V().limit() 这条语句应该涉及不到 task, 这里应该有 bug

我理解执行任务就会生成一个task吧，跟语句本身没有关系吧

ok, 看着这里执行任务应该创建了一个task

执行这条语句一直失败吗？后端用什么存储呢？

一直失败，使用rocksdb存储。报错都是Can't schedule task on non-master server

zyxxoo · 2023-04-12T04:45:38Z

不过目前来看， g.V().limit() 这条语句应该涉及不到 task, 这里应该有 bug

我理解执行任务就会生成一个task吧，跟语句本身没有关系吧

ok, 看着这里执行任务应该创建了一个task
执行这条语句一直失败吗？后端用什么存储呢？

一直失败，使用rocksdb存储。报错都是Can't schedule task on non-master server

use hubble, submit task still fail on the worker, but success on the master

目前在拦截器这一层做了转发，但是这期间如果发生了 master 切换，因为缓存原因还是会转发到旧的节点，这时候就会有这个错误，这里考虑性能并不会转发多次，可以考虑重试下这个语句应该就不会报错了

集群run起来之后没有发生master切换，多次尝试g.V().limit()还是会失败

估计是拦截器漏加了，这个是指定方法才会生效

还有，必须配置了授权才能生效，配置auth.authenticator=org.apache.hugegraph.auth.StandardAuthenticator 了吗？

zyxxoo · 2023-04-12T04:55:45Z

@wuchaojing , 可以在检查下上面配置，应该是 ok的，拦截器没有漏

github-actions Bot reviewed Feb 26, 2023

View reviewed changes

zyxxoo force-pushed the zy_dev branch 4 times, most recently from c822b43 to 78cd8cd Compare February 26, 2023 11:52

zyxxoo force-pushed the zy_dev branch from 78cd8cd to 4a832ef Compare February 26, 2023 12:56

javeme reviewed Feb 26, 2023

View reviewed changes

Comment thread hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java Resolved

javeme reviewed Feb 26, 2023

View reviewed changes

zyxxoo force-pushed the zy_dev branch 3 times, most recently from f7c2e6a to 5f17594 Compare February 27, 2023 03:27

javeme reviewed Feb 27, 2023

View reviewed changes

zyxxoo force-pushed the zy_dev branch from 5acd244 to ac55871 Compare February 28, 2023 01:59

zyxxoo force-pushed the zy_dev branch 5 times, most recently from 32075c4 to fab5a17 Compare February 28, 2023 11:06

javeme reviewed Feb 28, 2023

View reviewed changes

zyxxoo force-pushed the zy_dev branch 4 times, most recently from 5d065c8 to 85b722a Compare March 1, 2023 13:23

javeme reviewed Mar 1, 2023

View reviewed changes

imbajin mentioned this pull request Mar 2, 2023

add github token for license check comment #2139

Merged

zyxxoo force-pushed the zy_dev branch from 81e83a1 to af9eaf8 Compare March 3, 2023 08:52

chore: improve code

c5db1cf

zyxxoo force-pushed the zy_dev branch 3 times, most recently from 68e186e to ae7954a Compare March 5, 2023 04:20

chore: improve code

d7832cf

zyxxoo force-pushed the zy_dev branch from ae7954a to d7832cf Compare March 5, 2023 04:22

zyxxoo added 2 commits March 5, 2023 13:33

chore: improve

a9e4c24

chore: improve code

3b09579

javeme reviewed Mar 5, 2023

View reviewed changes

chore: improve

67b8404

zyxxoo force-pushed the zy_dev branch from c2ccfac to 67b8404 Compare March 6, 2023 03:03

zyxxoo requested a review from imbajin March 6, 2023 07:33

javeme previously approved these changes Mar 6, 2023

View reviewed changes

optimize all imports & tiny improve

5523064

imbajin dismissed javeme’s stale review via 5523064 March 7, 2023 11:43

imbajin reviewed Mar 7, 2023

View reviewed changes

imbajin approved these changes Mar 7, 2023

View reviewed changes

javeme approved these changes Mar 8, 2023

View reviewed changes

zyxxoo merged commit 31c6c6d into master Mar 8, 2023

zyxxoo deleted the zy_dev branch March 8, 2023 15:20

-package org.apache.hugegraph.core;
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hugegraph.core;


		private final TaskManager taskManager;

		boolean isMaster = false;

		@@ -234,7 +240,7 @@ public boolean updateIfNodePresent(RoleTypeData stateData) {
		if (Objects.equals(value.node(), copy.node()) &&

Conversation

zyxxoo commented Feb 26, 2023

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Feb 26, 2023

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Feb 26, 2023

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Feb 26, 2023

Choose a reason for hiding this comment

Uh oh!

codecov Bot commented Feb 26, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

simon824 commented Feb 28, 2023

Uh oh!

zyxxoo commented Feb 28, 2023

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zyxxoo Mar 3, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov Bot commented Feb 26, 2023 •

edited

Loading

zyxxoo Mar 3, 2023 •

edited

Loading

imbajin Mar 7, 2023 •

edited

Loading

wuchaojing commented Apr 11, 2023 •

edited

Loading

wuchaojing commented Apr 12, 2023 •

edited

Loading