Skip to content

feat: create server status plugin #1137

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions block-node/app/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ mainModuleInfo {
runtimeOnly("org.hiero.block.node.blocks.files.historic")
runtimeOnly("org.hiero.block.node.blocks.files.recent")
runtimeOnly("org.hiero.block.node.access.service")
runtimeOnly("org.hiero.block.node.server.status")
}

testModuleInfo {
Expand Down
19 changes: 19 additions & 0 deletions block-node/server-status/build.gradle.kts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// SPDX-License-Identifier: Apache-2.0
plugins { id("org.hiero.gradle.module.library") }

description = "Hiero Block Node - Block Node Server Status API Module"

// Remove the following line to enable all 'javac' lint checks that we have turned on by default
// and then fix the reported issues.
tasks.withType<JavaCompile>().configureEach { options.compilerArgs.add("-Xlint:-exports") }

mainModuleInfo {
runtimeOnly("com.swirlds.config.impl")
runtimeOnly("io.helidon.logging.jul")
runtimeOnly("com.hedera.pbj.grpc.helidon.config")
}

testModuleInfo {
requires("org.junit.jupiter.api")
requires("org.hiero.block.node.app.test.fixtures")
}
15 changes: 15 additions & 0 deletions block-node/server-status/src/main/java/module-info.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// SPDX-License-Identifier: Apache-2.0
import org.hiero.block.node.server.status.ServerStatusServicePlugin;

module org.hiero.block.node.server.status {
uses com.swirlds.config.api.spi.ConfigurationBuilderFactory;

requires transitive com.hedera.pbj.runtime;
requires transitive org.hiero.block.node.spi;
requires com.swirlds.metrics.api;
requires org.hiero.block.protobuf;
requires com.github.spotbugs.annotations;

provides org.hiero.block.node.spi.BlockNodePlugin with
ServerStatusServicePlugin;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
// SPDX-License-Identifier: Apache-2.0
package org.hiero.block.node.server.status;

import static java.lang.System.Logger.Level.DEBUG;
import static java.util.Objects.requireNonNull;

import com.hedera.pbj.runtime.grpc.GrpcException;
import com.hedera.pbj.runtime.grpc.Pipeline;
import com.hedera.pbj.runtime.grpc.Pipelines;
import com.hedera.pbj.runtime.grpc.ServiceInterface;
import com.hedera.pbj.runtime.io.buffer.Bytes;
import com.swirlds.metrics.api.Counter;
import edu.umd.cs.findbugs.annotations.NonNull;
import java.util.Arrays;
import java.util.List;
import org.hiero.block.api.ServerStatusRequest;
import org.hiero.block.api.ServerStatusResponse;
import org.hiero.block.api.protoc.BlockNodeServiceGrpc;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps take a quick look at Jasper's update for how to remove the use of protoc here?

import org.hiero.block.node.spi.BlockNodeContext;
import org.hiero.block.node.spi.BlockNodePlugin;
import org.hiero.block.node.spi.ServiceBuilder;
import org.hiero.block.node.spi.historicalblocks.HistoricalBlockFacility;

/**
* Plugin that implements the BlockNodeService and provides the 'serverStatus' RPC.
*/
public class ServerStatusServicePlugin implements BlockNodePlugin, ServiceInterface {
/** The logger for this class. */
private final System.Logger LOGGER = System.getLogger(getClass().getName());
/** The block provider */
private HistoricalBlockFacility blockProvider;
/** The block node context, used to provide access to facilities */
private BlockNodeContext context;

Check warning on line 33 in block-node/server-status/src/main/java/org/hiero/block/node/server/status/ServerStatusServicePlugin.java

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

block-node/server-status/src/main/java/org/hiero/block/node/server/status/ServerStatusServicePlugin.java#L33

Avoid unused private fields such as 'context'.

Check warning on line 33 in block-node/server-status/src/main/java/org/hiero/block/node/server/status/ServerStatusServicePlugin.java

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

block-node/server-status/src/main/java/org/hiero/block/node/server/status/ServerStatusServicePlugin.java#L33

Perhaps 'context' could be replaced by a local variable.
/** Counter for the number of requests */
private Counter requestCounter;

/**
* Handle a request for server status
*
* @param request the request containing the available blocks, state snapshot status and software version
* @return the response containing the block or an error status
*/
private ServerStatusResponse handleServiceStatusRequest(@NonNull final ServerStatusRequest request) {

Check warning on line 43 in block-node/server-status/src/main/java/org/hiero/block/node/server/status/ServerStatusServicePlugin.java

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

block-node/server-status/src/main/java/org/hiero/block/node/server/status/ServerStatusServicePlugin.java#L43

Avoid unused method parameters such as 'request'.
LOGGER.log(DEBUG, "Handling ServerStatusRequest");
requestCounter.increment();

final ServerStatusResponse.Builder serverStatusResponse = ServerStatusResponse.newBuilder();
final long firstAvailableBlock = blockProvider.availableBlocks().min();
final long lastAvailableBlock = blockProvider.availableBlocks().max();
Comment on lines +47 to +48
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a general question. Do we have to think about gaps? Here a hypothetical scenario:

  1. We have blocks available: 1, 2, 3, 5, 6 ,7
  2. Min will be 1
  3. Max will be 7
  4. If we query for 4 however, we will not see it

Should we generally extend these requests to return ranges in some form which then could be parsed?
This is something for the future, I was wondering if this is relevant in general?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, I think it's a valid consideration; however, this endpoint wouldn't care. I mean, the StatusServer query only aims to provide information about what should be available in the block-node. If something is missing, it's the responsibility of another plugin. There's currently an issue related to filling gaps—generally, if we say we have data between 0 and 7, then we should have it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. Gaps are the responsibility of the backfill plugin (to be developed). The status service is intended to provide the edges (earliest and latest); not every detail.

That said, nothing prevents us from adding an extension in the future if there is demand.


// TODO(#579) Should get from state config or status, which would be provided by the context from responsible
// facility
boolean onlyLatestState = false;
Comment on lines +50 to +52
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question: what is the meaning of this boolean?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is indicating whether this block node offers the latest state snapshot. However we don't have state implemented currently, so it's irrelevant. If we don't set it, it will automatically be false. I left it here for visibility with the TODO comment.


// TODO(#1139) Should get construct a block node version object from application config, which would be provided
// by
// the context from responsible facility
Comment on lines +54 to +56
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this plugin is the responsible facility.
The server status is the only place block node version is used, and should probably be filled in by the build from version.txt.

I don't think that becomes config so much as the build can insert the value into the code (as it's always constant for a given build output). That might require some build changes, however, so perhaps it would make sense to have it in config at least temporarily. Thoughts?


return serverStatusResponse
.firstAvailableBlock(firstAvailableBlock)
.lastAvailableBlock(lastAvailableBlock)
.onlyLatestState(onlyLatestState)
.build();
}

// ==== BlockNodePlugin Methods ====================================================================================
@Override
public String name() {
return "ServerStatusServicePlugin";
}

@Override
public void init(@NonNull final BlockNodeContext context, @NonNull final ServiceBuilder serviceBuilder) {
requireNonNull(serviceBuilder);
this.context = requireNonNull(context);
this.blockProvider = requireNonNull(context.historicalBlockProvider());

// Create the metrics
requestCounter = context.metrics()
.getOrCreate(new Counter.Config(METRICS_CATEGORY, "server-status-requests")
.withDescription("Number of server status requests"));

// Register this service
serviceBuilder.registerGrpcService(this);
}
// ==== ServiceInterface Methods ===================================================================================
/**
* BlockNodeService methods define the gRPC methods available on the BlockNodeService.
*/
enum BlockNodeServiceMethod implements Method {
/**
* The serverStatus method retrieves the status of this Block Node.
*/
serverStatus
}

/**
* {@inheritDoc}
*/
@NonNull
@Override
public String serviceName() {
String[] parts = fullName().split("\\.");
return parts[parts.length - 1];
Comment on lines +102 to +103
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've seen this in some other places, I think this should become an utility maybe?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, opened a ticket to track this #1158

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally we'd follow the pattern in the stream subscriber plugin and only calculate the value once when the class is first loaded; storing it in a static constant.

}

/**
* {@inheritDoc}
*/
@NonNull
@Override
public String fullName() {
return BlockNodeServiceGrpc.SERVICE_NAME;
}

/**
* {@inheritDoc}
*/
@NonNull
@Override
public List<Method> methods() {
return Arrays.asList(BlockNodeServiceMethod.values());
}

/**
* {@inheritDoc}
*
* This is called each time a new request is received.
*/
@NonNull
@Override
public Pipeline<? super Bytes> open(
@NonNull Method method, @NonNull RequestOptions requestOptions, @NonNull Pipeline<? super Bytes> pipeline)
throws GrpcException {
final BlockNodeServiceMethod blockNodeServiceMethod = (BlockNodeServiceMethod) method;
return switch (blockNodeServiceMethod) {
case serverStatus:
yield Pipelines.<ServerStatusRequest, ServerStatusResponse>unary()
.mapRequest(ServerStatusRequest.PROTOBUF::parse)
.method(this::handleServiceStatusRequest)
.mapResponse(ServerStatusResponse.PROTOBUF::toBytes)
.respondTo(pipeline)
.build();
};
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
// SPDX-License-Identifier: Apache-2.0
package org.hiero.block.node.server.status;

import static org.hiero.block.node.app.fixtures.TestUtils.enableDebugLogging;
import static org.hiero.block.node.app.fixtures.blocks.BlockItemUtils.toBlockItemsUnparsed;
import static org.hiero.block.node.app.fixtures.blocks.SimpleTestBlockItemBuilder.createNumberOfVerySimpleBlocks;
import static org.hiero.block.node.server.status.ServerStatusServicePlugin.BlockNodeServiceMethod.serverStatus;
import static org.hiero.block.node.spi.BlockNodePlugin.UNKNOWN_BLOCK_NUMBER;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;

import com.hedera.hapi.block.stream.BlockItem;
import com.hedera.pbj.runtime.ParseException;
import com.hedera.pbj.runtime.grpc.ServiceInterface;
import java.util.List;
import org.hiero.block.api.ServerStatusRequest;
import org.hiero.block.api.ServerStatusResponse;
import org.hiero.block.node.app.fixtures.plugintest.GrpcPluginTestBase;
import org.hiero.block.node.app.fixtures.plugintest.SimpleInMemoryHistoricalBlockFacility;
import org.hiero.block.node.spi.blockmessaging.BlockItems;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;

/**
* Tests for the ServerStatusServicePlugin class.
* Validates the functionality of the server status service and its responses
* under different conditions.
*/
public class ServerStatusServicePluginTest extends GrpcPluginTestBase<ServerStatusServicePlugin> {

public ServerStatusServicePluginTest() {
super();
start(new ServerStatusServicePlugin(), serverStatus, new SimpleInMemoryHistoricalBlockFacility());
}

/**
* Enable debug logging for each test.
*/
@BeforeEach
void setup() {
enableDebugLogging();
}

/**
* Verifies that the service interface correctly registers and exposes
* the server status method.
*/
@Test
@DisplayName("Should return correct method for ServerStatusServicePlugin")
void shouldReturnCorrectMethod() {
assertNotNull(serviceInterface);
List<ServiceInterface.Method> methods = serviceInterface.methods();
assertNotNull(methods);
assertEquals(1, methods.size());
assertEquals(serverStatus, methods.getFirst());
}

/**
* Tests that the server status response is valid when no blocks are available.
* Verifies the first and last available block numbers and other response properties.
*
* @throws ParseException if there is an error parsing the response
*/
@Test
@DisplayName("Should return valid Server Status when no blocks available")
void shouldReturnValidServerStatus() throws ParseException {
// sendBlocks(5);
final ServerStatusRequest request = ServerStatusRequest.newBuilder().build();
toPluginPipe.onNext(ServerStatusRequest.PROTOBUF.toBytes(request));
assertEquals(1, fromPluginBytes.size());

final ServerStatusResponse response = ServerStatusResponse.PROTOBUF.parse(fromPluginBytes.getFirst());

assertNotNull(response);
assertEquals(0, response.firstAvailableBlock());
assertEquals(0, response.lastAvailableBlock());
Comment on lines +77 to +78
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0 (zero) is a valid block number? If we test for no blocks available here, should we not expect something else, like -1?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, thank you for noticing. Should be -1L, yes. I forgot it from some previous testing that I did 👍

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally -1 represents "the highest possible block number" (because we prefer to use unsigned values in the protobuf, which are bitwise transliterated with Java long) and is presented to (non-java) API clients as uint64_max.

We use -1L in Java code because we should never reach that high of a block number (at least, not for an hundred billion years or so).

assertFalse(response.onlyLatestState());

// TODO() Remove when block node version information is implemented.
assertFalse(response.hasVersionInformation());
}

/**
* Tests the server status response after adding a new batch of blocks.
* Verifies that the first and last available block numbers are correctly updated.
*
* @throws ParseException if there is an error parsing the response
*/
@Test
@DisplayName("Should return valid Server Status, after new batch of blocks")
void shouldReturnValidServerStatusForNewBlockBatch() throws ParseException {
final int blocks = 5;
sendBlocks(blocks);
final ServerStatusRequest request = ServerStatusRequest.newBuilder().build();
toPluginPipe.onNext(ServerStatusRequest.PROTOBUF.toBytes(request));
assertEquals(1, fromPluginBytes.size());

final ServerStatusResponse response = ServerStatusResponse.PROTOBUF.parse(fromPluginBytes.getLast());

assertNotNull(response);
assertEquals(0, response.firstAvailableBlock());
assertEquals(blocks - 1, response.lastAvailableBlock());
assertFalse(response.onlyLatestState());

// TODO() Remove when block node version information is implemented.
assertFalse(response.hasVersionInformation());
}

/**
* Helper method to send a specified number of test blocks to the block messaging system.
*
* @param numberOfBlocks the number of test blocks to create and send
*/
private void sendBlocks(int numberOfBlocks) {
BlockItem[] blockItems = createNumberOfVerySimpleBlocks(numberOfBlocks);
// Send some blocks
for (BlockItem blockItem : blockItems) {
long blockNumber =
blockItem.hasBlockHeader() ? blockItem.blockHeader().number() : UNKNOWN_BLOCK_NUMBER;
blockMessaging.sendBlockItems(new BlockItems(toBlockItemsUnparsed(blockItem), blockNumber));
}
}
}
1 change: 1 addition & 0 deletions settings.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -28,5 +28,6 @@ javaModules {
module("block-providers/files.historic") { artifact = "block-node-blocks-file-historic" }
module("block-providers/files.recent") { artifact = "block-node-blocks-file-recent" }
module("block-access") { artifact = "block-access-service" }
module("server-status") { artifact = "block-node-server-status" }
}
}
12 changes: 12 additions & 0 deletions suites/src/main/java/org/hiero/block/suites/BaseSuite.java
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import java.util.concurrent.Future;
import java.util.concurrent.TimeUnit;
import org.hiero.block.api.protoc.BlockAccessServiceGrpc;
import org.hiero.block.api.protoc.BlockNodeServiceGrpc;
import org.hiero.block.simulator.BlockStreamSimulatorApp;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
Expand Down Expand Up @@ -47,6 +48,9 @@ public abstract class BaseSuite {
/** gRPC client stub for BlockAccessService */
protected static BlockAccessServiceGrpc.BlockAccessServiceBlockingStub blockAccessStub;

/** gRPC client stub for BlockNodeService */
protected static BlockNodeServiceGrpc.BlockNodeServiceBlockingStub blockServiceStub;

/**
* Default constructor for the BaseSuite class.
*
Expand All @@ -68,6 +72,7 @@ public void setup() {
blockNodeContainer.start();
executorService = new ErrorLoggingExecutor();
blockAccessStub = initializeBlockAccessGrpcClient();
blockServiceStub = initializeBlockNodeServiceGrpcClient();
}

/**
Expand Down Expand Up @@ -136,6 +141,13 @@ protected static BlockAccessServiceGrpc.BlockAccessServiceBlockingStub initializ
return BlockAccessServiceGrpc.newBlockingStub(channel);
}

protected static BlockNodeServiceGrpc.BlockNodeServiceBlockingStub initializeBlockNodeServiceGrpcClient() {
final String host = blockNodeContainer.getHost();
final int port = blockNodePort;
channel = ManagedChannelBuilder.forAddress(host, port).usePlaintext().build();
return BlockNodeServiceGrpc.newBlockingStub(channel);
}

/**
* Starts the block stream simulator in a separate thread.
*
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// SPDX-License-Identifier: Apache-2.0
package org.hiero.block.suites.server.status;

import org.hiero.block.suites.server.status.positive.PositiveServerStatusTests;
import org.junit.platform.suite.api.SelectClasses;
import org.junit.platform.suite.api.Suite;

@Suite
@SelectClasses({PositiveServerStatusTests.class})
public class ServerStatusTestSuites {
/**
* Default constructor for the {@link ServerStatusTestSuites} class.This constructor is
* empty as it does not need to perform any initialization.
*/
public ServerStatusTestSuites() {}

Check notice on line 15 in suites/src/main/java/org/hiero/block/suites/server/status/ServerStatusTestSuites.java

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

suites/src/main/java/org/hiero/block/suites/server/status/ServerStatusTestSuites.java#L15

Avoid unnecessary constructors - the compiler will generate these for you
}
Loading
Loading