javeme commented on code in PR #2130:
URL:
https://github.com/apache/incubator-hugegraph/pull/2130#discussion_r1121690572
##########
hugegraph-core/src/main/java/org/apache/hugegraph/election/StandardRoleElectionStateMachine.java:
##########
@@ -49,7 +53,9 @@ 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);
+ LOG.trace("server {} epoch {} role state change {} to {}",
context.node(), context.epoch(), pre.getClass().getSimpleName(),
this.state.getClass().getSimpleName());
Review Comment:
is the line too long
##########
hugegraph-api/src/main/java/org/apache/hugegraph/api/job/RebuildAPI.java:
##########
@@ -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
Review Comment:
maybe ACCEPTED and RedirectMasterRole often appear simultaneously
##########
hugegraph-api/src/main/java/org/apache/hugegraph/api/filter/RedirectFilter.java:
##########
@@ -22,20 +22,43 @@
import java.lang.annotation.RetentionPolicy;
import java.net.URI;
import java.net.URISyntaxException;
+import java.util.Date;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
import jakarta.ws.rs.NameBinding;
+import jakarta.ws.rs.client.Client;
+import jakarta.ws.rs.client.ClientBuilder;
+import jakarta.ws.rs.client.Entity;
+import jakarta.ws.rs.client.Invocation;
import jakarta.ws.rs.container.ContainerRequestContext;
import jakarta.ws.rs.container.ContainerRequestFilter;
+import jakarta.ws.rs.core.MultivaluedMap;
import jakarta.ws.rs.core.Response;
import org.apache.commons.lang3.StringUtils;
import org.apache.http.client.utils.URIBuilder;
import org.apache.hugegraph.election.GlobalMasterInfo;
import org.apache.hugegraph.util.Log;
+import org.glassfish.jersey.message.internal.HeaderUtils;
import org.slf4j.Logger;
public class RedirectFilter implements ContainerRequestFilter {
private static final Logger LOG = Log.logger(RedirectFilter.class);
+ public static final String X_HG_REDIRECT = "x-hg-redirect";
Review Comment:
let public member first, move to line 49?
##########
hugegraph-core/src/main/java/org/apache/hugegraph/election/StandardRoleTypeDataAdapter.java:
##########
@@ -106,15 +118,26 @@ private BackendEntry constructEntry(RoleTypeData
stateData) {
@Override
public Optional<RoleTypeData> query() {
Optional<Vertex> vertex = this.queryVertex();
+ if (!vertex.isPresent() && !this.first) {
+ // If query nothing, retry once
+ try {
+ Thread.sleep(200);
Review Comment:
add a const var for 200?
##########
hugegraph-core/src/main/java/org/apache/hugegraph/election/StandardRoleTypeDataAdapter.java:
##########
@@ -34,46 +34,55 @@
import org.apache.hugegraph.type.HugeType;
import org.apache.hugegraph.type.define.DataType;
import org.apache.hugegraph.type.define.HugeKeys;
+import org.apache.hugegraph.util.Log;
import org.apache.tinkerpop.gremlin.structure.Graph;
import org.apache.tinkerpop.gremlin.structure.T;
import org.apache.tinkerpop.gremlin.structure.Vertex;
+import org.slf4j.Logger;
public class StandardRoleTypeDataAdapter implements RoleTypeDataAdapter {
private final HugeGraphParams graphParams;
private final Schema schema;
+ private static final Logger LOG =
Log.logger(StandardRoleTypeDataAdapter.class);
+
+ private boolean first;
Review Comment:
prefer firstTime
##########
hugegraph-core/src/main/java/org/apache/hugegraph/election/StandardRoleTypeDataAdapter.java:
##########
@@ -0,0 +1,212 @@
+/*
+ * 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;
+
+import java.util.ArrayList;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Objects;
+import java.util.Optional;
+
+import org.apache.hugegraph.HugeGraphParams;
+import org.apache.hugegraph.auth.SchemaDefine;
+import org.apache.hugegraph.backend.query.Condition;
+import org.apache.hugegraph.backend.query.ConditionQuery;
+import org.apache.hugegraph.backend.store.BackendEntry;
+import org.apache.hugegraph.backend.tx.GraphTransaction;
+import org.apache.hugegraph.schema.VertexLabel;
+import org.apache.hugegraph.structure.HugeVertex;
+import org.apache.hugegraph.type.HugeType;
+import org.apache.hugegraph.type.define.DataType;
+import org.apache.hugegraph.type.define.HugeKeys;
+import org.apache.hugegraph.util.Log;
+import org.apache.tinkerpop.gremlin.structure.Graph;
+import org.apache.tinkerpop.gremlin.structure.T;
+import org.apache.tinkerpop.gremlin.structure.Vertex;
+import org.slf4j.Logger;
+
+public class StandardRoleTypeDataAdapter implements RoleTypeDataAdapter {
+
+ private final HugeGraphParams graphParams;
+ private final Schema schema;
+
+ private static final Logger LOG =
Log.logger(StandardRoleTypeDataAdapter.class);
+
+ private boolean first;
+
+ public StandardRoleTypeDataAdapter(HugeGraphParams graph) {
+ this.graphParams = graph;
+ this.schema = new Schema(graph);
+ this.schema.initSchemaIfNeeded();
+ this.first = true;
+ }
+
+ @Override
+ public boolean updateIfNodePresent(RoleTypeData stateData) {
+ // if epoch increase, update and return true
+ // if epoch equal, ignore different node, return false
+ Optional<Vertex> oldTypeDataOpt = this.queryVertex();
+ if (oldTypeDataOpt.isPresent()) {
+ RoleTypeData oldTypeData = this.from(oldTypeDataOpt.get());
+ if (stateData.epoch() < oldTypeData.epoch()) {
+ return false;
+ }
+
+ if (stateData.epoch() == oldTypeData.epoch() &&
+ !Objects.equals(stateData.node(), oldTypeData.node())) {
+ return false;
+ }
+ LOG.trace("Server {} epoch {} begin remove data old epoch {}, ",
stateData.node(), stateData.epoch(), oldTypeData.epoch());
Review Comment:
expect wrap line
##########
hugegraph-core/src/main/java/org/apache/hugegraph/election/StandardRoleElectionStateMachine.java:
##########
@@ -241,10 +250,13 @@ public CandidateState(Integer epoch) {
public RoleState transform(StateMachineContext context) {
RoleState.randomPark(context);
int epoch = this.epoch == null ? 1 : this.epoch;
- RoleTypeData roleTypeData = new
RoleTypeData(context.config().node(), epoch);
+ RoleTypeData roleTypeData = new
RoleTypeData(context.config().node(),
+
context.config().url(), epoch);
//failover to master success
context.epoch(roleTypeData.epoch());
if (context.adapter().updateIfNodePresent(roleTypeData)) {
+ context.master(new
StateMachineContextImpl.MasterServerInfoImpl(
Review Comment:
remove "StateMachineContextImpl." prefix since they are in the same file
##########
hugegraph-core/src/main/java/org/apache/hugegraph/election/StandardRoleElectionStateMachine.java:
##########
@@ -241,10 +250,13 @@ public CandidateState(Integer epoch) {
public RoleState transform(StateMachineContext context) {
RoleState.randomPark(context);
int epoch = this.epoch == null ? 1 : this.epoch;
- RoleTypeData roleTypeData = new
RoleTypeData(context.config().node(), epoch);
+ RoleTypeData roleTypeData = new
RoleTypeData(context.config().node(),
+
context.config().url(), epoch);
//failover to master success
Review Comment:
expect style "// xx"
and improve it like "The master failover completed"
##########
hugegraph-core/src/main/java/org/apache/hugegraph/election/RoleTypeData.java:
##########
@@ -25,12 +25,15 @@ public class RoleTypeData {
private long clock;
private int epoch;
- public RoleTypeData(String node, int epoch) {
- this(node, epoch, 1);
+ private String url;
+
+ public RoleTypeData(String node, String url, int epoch) {
Review Comment:
prefer to rename to ClusterRole or ClusterEpoch
##########
hugegraph-core/src/main/java/org/apache/hugegraph/election/StandardRoleTypeDataAdapter.java:
##########
@@ -43,9 +43,9 @@ public class StandardRoleTypeDataAdapter implements
RoleTypeDataAdapter {
private final HugeGraphParams graphParams;
private final Schema schema;
- public StandardRoleTypeDataAdapter(HugeGraphParams graphParams) {
- this.graphParams = graphParams;
- this.schema = new Schema(graphParams);
+ public StandardRoleTypeDataAdapter(HugeGraphParams graph) {
+ this.graphParams = graph;
Review Comment:
also rename this.graphParams to graph
##########
hugegraph-core/src/main/java/org/apache/hugegraph/election/StandardRoleTypeDataAdapter.java:
##########
@@ -34,46 +34,55 @@
import org.apache.hugegraph.type.HugeType;
import org.apache.hugegraph.type.define.DataType;
import org.apache.hugegraph.type.define.HugeKeys;
+import org.apache.hugegraph.util.Log;
import org.apache.tinkerpop.gremlin.structure.Graph;
import org.apache.tinkerpop.gremlin.structure.T;
import org.apache.tinkerpop.gremlin.structure.Vertex;
+import org.slf4j.Logger;
public class StandardRoleTypeDataAdapter implements RoleTypeDataAdapter {
private final HugeGraphParams graphParams;
private final Schema schema;
+ private static final Logger LOG =
Log.logger(StandardRoleTypeDataAdapter.class);
Review Comment:
move const member to the beginning
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@hugegraph.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
