-
Notifications
You must be signed in to change notification settings - Fork 528
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: support task auto manage by server role state machine #2130
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
@@ -0,0 +1,146 @@ | |||
package org.apache.hugegraph.core; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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; |
@@ -0,0 +1,74 @@ | |||
package org.apache.hugegraph.core; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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; |
@@ -0,0 +1,53 @@ | |||
package org.apache.hugegraph.election; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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; |
c822b43
to
78cd8cd
Compare
Codecov Report
@@ 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
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
hugegraph-api/src/main/java/org/apache/hugegraph/auth/ConfigAuthenticator.java
Outdated
Show resolved
Hide resolved
hugegraph-api/src/main/java/org/apache/hugegraph/config/ServerOptions.java
Outdated
Show resolved
Hide resolved
hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java
Outdated
Show resolved
Hide resolved
hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java
Outdated
Show resolved
Hide resolved
hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java
Outdated
Show resolved
Hide resolved
|
||
private final TaskManager taskManager; | ||
|
||
boolean isMaster = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mark private volatile?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the filed only use on one thread, so don't volatile
hugegraph-core/src/main/java/org/apache/hugegraph/StandardHugeGraph.java
Outdated
Show resolved
Hide resolved
import org.apache.hugegraph.util.Log; | ||
import org.slf4j.Logger; | ||
|
||
public class StateMachineCallbackImpl implements StateMachineCallback { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not submit StateMachineCallback file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file is mainly responsible for state machine and external interactions, so it was not previously committed
hugegraph-api/src/main/java/org/apache/hugegraph/core/StateMachineCallbackImpl.java
Outdated
Show resolved
Hide resolved
hugegraph-api/src/main/java/org/apache/hugegraph/core/StateMachineCallbackImpl.java
Outdated
Show resolved
Hide resolved
hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java
Outdated
Show resolved
Hide resolved
hugegraph-api/src/main/java/org/apache/hugegraph/core/RoleTypeDataAdapterImpl.java
Outdated
Show resolved
Hide resolved
hugegraph-api/src/main/java/org/apache/hugegraph/core/RoleTypeDataAdapterImpl.java
Outdated
Show resolved
Hide resolved
import org.apache.tinkerpop.gremlin.structure.T; | ||
import org.apache.tinkerpop.gremlin.structure.Vertex; | ||
|
||
public class RoleTypeDataAdapterImpl implements RoleTypeDataAdapter { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RoleTypeDataAdapter means RoleTypeStore?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, the adapter responsibility mainly applies to storage
f7c2e6a
to
5f17594
Compare
hugegraph-core/src/main/java/org/apache/hugegraph/StandardHugeGraph.java
Outdated
Show resolved
Hide resolved
hugegraph-core/src/main/java/org/apache/hugegraph/config/CoreOptions.java
Outdated
Show resolved
Hide resolved
hugegraph-core/src/main/java/org/apache/hugegraph/config/CoreOptions.java
Outdated
Show resolved
Hide resolved
hugegraph-core/src/main/java/org/apache/hugegraph/election/StandardRoleTypeDataAdapter.java
Outdated
Show resolved
Hide resolved
E.checkArgument(this.roleStateWorker == null, "Repetition init"); | ||
this.applyThread = Executors.newSingleThreadExecutor(); | ||
this.roleStateWorker = this.authenticator().graph().roleElectionStateMachine(); | ||
this.applyThread.execute(() -> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can move applyThread to RoleElectionStateMachine class?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's easier to write a single test with asynchronous threads outside
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'. |
yes, we will support redirect request to master node in this feature |
32075c4
to
fab5a17
Compare
hugegraph-core/src/main/java/org/apache/hugegraph/election/StandardRoleTypeDataAdapter.java
Outdated
Show resolved
Hide resolved
hugegraph-core/src/main/java/org/apache/hugegraph/election/GlobalMasterInfo.java
Outdated
Show resolved
Hide resolved
|
||
@Override | ||
public void filter(ContainerRequestContext requestContext) throws IOException { | ||
GlobalMasterInfo instance = GlobalMasterInfo.instance(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
get from graphManager.auth.masterInfo?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have change to "graphManager.globalMasterInfo"
hugegraph-api/src/main/java/org/apache/hugegraph/api/filter/RedirectFilter.java
Outdated
Show resolved
Hide resolved
"server.role.node_external_url", | ||
"The url of external accessibility", | ||
disallowEmpty(), | ||
"127.0.0.1:8080" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can pass it from restserver config to graph config, like rpc url
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
5d065c8
to
85b722a
Compare
...graph-core/src/main/java/org/apache/hugegraph/election/StandardRoleElectionStateMachine.java
Outdated
Show resolved
Hide resolved
hugegraph-api/src/main/java/org/apache/hugegraph/api/filter/RedirectFilter.java
Outdated
Show resolved
Hide resolved
@@ -53,6 +53,7 @@ public class RebuildAPI extends API { | |||
@Status(Status.ACCEPTED) | |||
@Produces(APPLICATION_JSON_WITH_CHARSET) | |||
@RolesAllowed({"admin", "$owner=$graph $action=index_write"}) | |||
@RedirectFilter.RedirectMasterRole |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe ACCEPTED and RedirectMasterRole often appear simultaneously
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, if ACCEPTED appear, the RedirectMasterRole also appear
hugegraph-core/src/main/java/org/apache/hugegraph/election/StandardRoleTypeDataAdapter.java
Outdated
Show resolved
Hide resolved
hugegraph-core/src/main/java/org/apache/hugegraph/election/StandardRoleTypeDataAdapter.java
Outdated
Show resolved
Hide resolved
hugegraph-core/src/main/java/org/apache/hugegraph/election/StandardRoleTypeDataAdapter.java
Outdated
Show resolved
Hide resolved
hugegraph-core/src/main/java/org/apache/hugegraph/election/StandardRoleTypeDataAdapter.java
Outdated
Show resolved
Hide resolved
...graph-core/src/main/java/org/apache/hugegraph/election/StandardRoleElectionStateMachine.java
Outdated
Show resolved
Hide resolved
...graph-core/src/main/java/org/apache/hugegraph/election/StandardRoleElectionStateMachine.java
Outdated
Show resolved
Hide resolved
hugegraph-core/src/main/java/org/apache/hugegraph/election/RoleTypeData.java
Outdated
Show resolved
Hide resolved
68e186e
to
ae7954a
Compare
hugegraph-core/src/main/java/org/apache/hugegraph/masterelection/RoleElectionOptions.java
Outdated
Show resolved
Hide resolved
hugegraph-core/src/main/java/org/apache/hugegraph/masterelection/RoleElectionOptions.java
Outdated
Show resolved
Hide resolved
hugegraph-core/src/main/java/org/apache/hugegraph/masterelection/RoleElectionOptions.java
Outdated
Show resolved
Hide resolved
hugegraph-core/src/main/java/org/apache/hugegraph/masterelection/RoleElectionOptions.java
Outdated
Show resolved
Hide resolved
hugegraph-core/src/main/java/org/apache/hugegraph/masterelection/RoleElectionOptions.java
Outdated
Show resolved
Hide resolved
...graph-core/src/main/java/org/apache/hugegraph/masterelection/HugeRoleStateMachineConfig.java
Outdated
Show resolved
Hide resolved
...graph-core/src/main/java/org/apache/hugegraph/masterelection/HugeRoleStateMachineConfig.java
Outdated
Show resolved
Hide resolved
hugegraph-core/src/main/java/org/apache/hugegraph/masterelection/RoleElectionOptions.java
Outdated
Show resolved
Hide resolved
hugegraph-core/src/main/java/org/apache/hugegraph/masterelection/StandardClusterRoleStore.java
Outdated
Show resolved
Hide resolved
hugegraph-core/src/main/java/org/apache/hugegraph/masterelection/RoleElectionOptions.java
Show resolved
Hide resolved
@@ -49,7 +53,11 @@ public void apply(StateMachineCallback stateMachineCallback) { | |||
while (!this.shutdown) { | |||
E.checkArgumentNotNull(this.state, "State don't be null"); | |||
try { | |||
RoleState pre = this.state; | |||
this.state = state.transform(context); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: state.transform
is not an atomic operation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the machine run in single thread
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
taskManager and globalMadterInfo are use lock to sync
@@ -234,7 +240,7 @@ public boolean updateIfNodePresent(RoleTypeData stateData) { | |||
if (Objects.equals(value.node(), copy.node()) && |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if the value could be null
here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the value dont become null
不过目前来看, g.V().limit() 这条语句应该涉及不到 task, 这里应该有 bug |
我理解 |
ok, 看着这里执行任务应该创建了一个task 执行这条语句一直失败吗?后端用什么存储呢? |
一直失败,使用rocksdb存储。报错都是 |
@wuchaojing , 可以在检查下上面配置,应该是 ok的,拦截器没有漏 |
No description provided.