tronprotocol / java-tron

Java implementation of the Tron whitepaper
GNU Lesser General Public License v3.0
3.66k stars 1.37k forks source link

API Services Fail to Start Silently #5820

Closed halibobo1205 closed 2 weeks ago

halibobo1205 commented 2 months ago

Software Versions

OS: Linux JVM: Oracle Corporation 1.8.0_161 AMD64 Version: 4.7.4 Code: 18260

Expected behavior

An exception should be thrown and the node service should exit when the API services fail to start.

Actual behavior

Node starts with no API services and no exception about it thrown.

Steps to reproduce the behavior

  1. Make gRPC service port 50051 occupied in advance
  2. Start the node
  3. Try to access the API endpoint

Backtrace

14:54:41.976 ERROR [rpc] RpcApiService
java.io.IOException: Failed to bind to address 0.0.0.0/0.0.0.0:50051
    at io.grpc.netty.NettyServer.start(NettyServer.java:328)
Caused by: java.net.BindException: Address already in use
14:54:41.983 INFO [app] All api services started.
UNIMPLEMENTED: Method not found: protocol.Wallet/GetAccount
io.grpc.StatusRuntimeException: UNIMPLEMENTED: Method not found: protocol.Wallet/GetAccount
    at io.grpc.stub.ClientCalls.toStatusRuntimeException(ClientCalls.java:271)
    at io.grpc.stub.ClientCalls.getUnchecked(ClientCalls.java:252)
    at io.grpc.stub.ClientCalls.blockingUnaryCall(ClientCalls.java:165)
zeusoo001 commented 2 months ago

@halibobo1205 In addition to api services, do other services also have such demands?

halibobo1205 commented 2 months ago

@zeusoo001, IMHO: external services should not silently fail to start, such as event-plugins.

halibobo1205 commented 2 months ago

external services :

TYPE SERVICE default available note
API RpcApiService :warning: force-enabled FullNode\SolidityNode  GRPC
API RpcApiServiceOnSolidity :warning: force-enabled FullNode  GRPC
API RpcApiServiceOnPBFT :warning: force-enabled   FullNode  GRPC
API FullNodeHttpApiService :white_check_mark: enabled FullNode  HTTP
API HttpApiOnSolidityService :white_check_mark: enabled   FullNode  HTTP
API HttpApiOnPBFTService :warning: force-enabled   FullNode  HTTP
API SolidityNodeHttpApiService :x: disabled SolidityNode  HTTP
API FullNodeJsonRpcHttpService :x: disabled FullNode  JSON-RPC
API JsonRpcServiceOnSolidity :x: disabled FullNode  JSON-RPC
API JsonRpcServiceOnPBFT :x: disabled FullNode  JSON-RPC
metric MetricsUtil :x: disabled FullNode\SolidityNode  influxdb
metric Metrics :x: disabled FullNode\SolidityNode  Prometheus
EVENT EventPluginLoader :x: disabled FullNode\SolidityNode event subscribe
NET TronNetService :white_check_mark: enabled FullNode\SolidityNode  
ZK-SNARK ZksnarkInitService :white_check_mark: enabled FullNode\SolidityNode  shield transaction
halibobo1205 commented 2 months ago

Force-enabled services: RpcApiService, RpcApiServiceOnSolidity, RpcApiServiceOnPBFT, and HttpApiOnPBFTService need to be adjusted to optional before the fix.

lxcmyf commented 2 months ago

Do you mean that if API services fail to start, they need to throw real-time exception errors instead of only reporting errors when accessing the interface? This may require adjusting the priority of each service startup. It would be great if the startup mechanism could be optimized and error feedback could be provided.

halibobo1205 commented 2 months ago

This may require adjusting the priority of each service startup. @lxcmyf why it needs to be adjusted?

lxcmyf commented 2 months ago

@halibobo1205 I would like to say that various services may be divided into many types according to their functions, but there are also similar services that can be classified under the same category. Real time startup errors can be classified by type, making the sorting of errors clearer.

317787106 commented 2 months ago

@halibobo1205 Should you check whether some configured ports are occupied before the main thread starts ?

halibobo1205 commented 2 months ago

@halibobo1205 Should you check whether some configured ports are occupied before the main thread starts ?

@317787106 There is no need to do this, just throw the associated error. See besu how to deal with this:

  private void waitForServiceToStart(
      final String serviceName, final CompletableFuture<?> startFuture) {
    do {
      try {
        startFuture.get(60, TimeUnit.SECONDS);
      } catch (final InterruptedException e) {
        Thread.currentThread().interrupt();
        throw new IllegalStateException("Interrupted while waiting for service to start", e);
      } catch (final ExecutionException e) {
        throw new IllegalStateException("Service " + serviceName + " failed to start", e);
      } catch (final TimeoutException e) {
        LOG.warn("Service {} is taking an unusually long time to start", serviceName);
      }
    } while (!startFuture.isDone());
  }
halibobo1205 commented 2 months ago

Real time startup errors can be classified by type, making the sorting of errors clearer.

@lxcmyf I don't think it's necessary, the error is even less clear, and the original error is still needed, you can see the sample above.

halibobo1205 commented 2 months ago

An exception should be thrown and the node service should exit when all external services fail to start.

Working on this.

halibobo1205 commented 2 months ago

An exception should be thrown and the node service should exit when the API services fail to start.

Is it necessary to exit the node service? PTAL @317787106 @lxcmyf @tomatoishealthy

tomatoishealthy commented 2 months ago

At present, some products I have used will not interrupt the main process when encountering some non-core service exceptions. Maybe TRON does not need to be forced to stop in this scenario?

But this is a very open topic, and it is also reasonable to stop the service when an exception is thrown.

I'd probably prefer to keep the service running, but I'd be fine with either.

halibobo1205 commented 2 months ago

Final decision: exit the node service when API services fail to start. cc @tomatoishealthy