junkaixue commented on code in PR #2840: URL: https://github.com/apache/helix/pull/2840#discussion_r1687146859
########## helix-gateway/src/main/java/org/apache/helix/gateway/service/GatewayServiceManager.java: ########## @@ -0,0 +1,65 @@ +package org.apache.helix.gateway.service; + +import java.util.Map; + +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.atomic.AtomicBoolean; +import org.apache.helix.gateway.grpcservice.HelixGatewayServiceService; + + +/** + * A top layer class that send/receive messages from Grpc end point, and dispatch them to corrsponding gateway services. + * 1. get event from Grpc service + * 2. Maintain a gateway service registry, one gateway service maps to one Helix cluster + * 3. On init connect, create the participant manager + * 4. For ST reply message, update the tracker + */ + +public class GatewayServiceManager { + + HelixGatewayServiceService _helixGatewayServiceService; + + HelixGatewayServiceProcessor _helixGatewayServiceProcessor; + + Map<String, HelixGatewayService> _helixGatewayServiceMap; + + // TODO: add thread pool for init + // single thread tp for update + + public enum EventType { + INIT, // init connection to gateway service + UPDATE, // update state transition result + SHUTDOWN // shutdown connection to gateway service. Review Comment: how about use: connect, disconnect, update. Because sometimes lost connection cannot say it as shutdown but disconnect. ########## helix-gateway/src/main/java/org/apache/helix/gateway/util/StateTransitionMessageTranslateUtil.java: ########## @@ -0,0 +1,8 @@ +package org.apache.helix.gateway.util; + +public class StateTransitionMessageTranslateUtil { + + public static void translateSTMsgToProto() { Review Comment: Should return proto object? ########## helix-gateway/src/main/java/org/apache/helix/gateway/util/StateTransitionMessageTranslateUtil.java: ########## @@ -0,0 +1,8 @@ +package org.apache.helix.gateway.util; + +public class StateTransitionMessageTranslateUtil { Review Comment: make it final ########## helix-gateway/src/main/java/org/apache/helix/gateway/grpcservice/HelixGatewayServiceService.java: ########## @@ -16,6 +27,13 @@ public StreamObserver<proto.org.apache.helix.gateway.HelixGatewayServiceOuterCla public void onNext(ShardStateMessage request) { // called when a client sends a message //.... + String instanceName = request.getInstanceName(); + if (_observerMap.containsValue(instanceName)) { + // this is from an existing client + } else { + updateObserver(instanceName, responseObserver); + // update state map + } Review Comment: Let's do this: if (! contains) { update } do regular thing. ########## helix-gateway/src/main/java/org/apache/helix/gateway/util/StateTransitionMessageTranslateUtil.java: ########## @@ -0,0 +1,8 @@ +package org.apache.helix.gateway.util; + +public class StateTransitionMessageTranslateUtil { + + public static void translateSTMsgToProto() { Review Comment: What about proto to ST? -- 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: reviews-unsubscr...@helix.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: reviews-unsubscr...@helix.apache.org For additional commands, e-mail: reviews-h...@helix.apache.org