feat: create server status plugin #1137

georgi-l95 · 2025-05-13T12:11:32Z

Reviewer Notes

This PR aims to:

Create new plugin responsible for opening a unary endpoint for serving ServerStatusRequest.
Handling of this requests is responsibility of the plugin, which returns:

message ServerStatusResponse {
    uint64 first_available_block = 1;
    uint64 last_available_block = 2;
    bool only_latest_state = 3;
    BlockNodeVersions version_information = 4;
}

Add unit tests validating the correct behavior, using already existing test fixtures.
Add E2E tests validating the behaviour of the endpoint.

Left out for next PRs:

Handling of only_latest_state information, for now we just set it to false.
Handling the construction of BlockNodeVersions object. We currently does not have this information at hand. Will require some change in order to be availble across the project.

Related Issue(s)

Fixes #1111
Fixes #781

Signed-off-by: georgi-l95 <glazarov95@gmail.com>

codecov · 2025-05-13T13:21:58Z

Codecov Report

Attention: Patch coverage is 88.88889% with 4 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
.../node/server/status/ServerStatusServicePlugin.java	88.88%	4 Missing ⚠️

@@             Coverage Diff              @@
##               main    #1137      +/-   ##
============================================
+ Coverage     84.86%   84.90%   +0.03%     
- Complexity      916      921       +5     
============================================
  Files           104      105       +1     
  Lines          3719     3755      +36     
  Branches        396      396              
============================================
+ Hits           3156     3188      +32     
- Misses          411      415       +4     
  Partials        152      152

Files with missing lines	Coverage Δ	Complexity Δ
.../node/server/status/ServerStatusServicePlugin.java	`88.88% <88.88%> (ø)`	`5.00 <5.00> (?)`

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Signed-off-by: georgi-l95 <glazarov95@gmail.com>

ata-nas

@georgi-l95 looks good. A couple of general questions.

ata-nas · 2025-05-15T13:39:48Z

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

+        String[] parts = fullName().split("\\.");
+        return parts[parts.length - 1];


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

Good point, opened a ticket to track this #1158

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.

ata-nas · 2025-05-15T13:42:39Z

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

+        final long firstAvailableBlock = blockProvider.availableBlocks().min();
+        final long lastAvailableBlock = blockProvider.availableBlocks().max();


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

We have blocks available: 1, 2, 3, 5, 6 ,7

Min will be 1

Max will be 7

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?

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.

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.

ata-nas · 2025-05-15T13:43:40Z

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

+        // TODO(#579) Should get from state config or status, which would be provided by the context from responsible
+        // facility
+        boolean onlyLatestState = false;


Question: what is the meaning of this boolean?

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.

ata-nas · 2025-05-15T13:44:50Z

...r-status/src/test/java/org/hiero/block/node/server/status/ServerStatusServicePluginTest.java

+        assertEquals(0, response.firstAvailableBlock());
+        assertEquals(0, response.lastAvailableBlock());


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

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

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).

Signed-off-by: georgi-l95 <glazarov95@gmail.com>

jsync-swirlds · 2025-05-16T17:21:57Z

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

+import java.util.List;
+import org.hiero.block.api.ServerStatusRequest;
+import org.hiero.block.api.ServerStatusResponse;
+import org.hiero.block.api.protoc.BlockNodeServiceGrpc;


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

jsync-swirlds · 2025-05-16T17:27:41Z

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

+        // TODO(#1139) Should get construct a block node version object from application config, which would be provided
+        // by
+        // the context from responsible facility


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?

georgi-l95 added 2 commits May 12, 2025 12:57

feat: add service status plugin

65fe74b

Signed-off-by: georgi-l95 <glazarov95@gmail.com>

test: add unit tests

cd7b5e9

Signed-off-by: georgi-l95 <glazarov95@gmail.com>

georgi-l95 added this to the 0.11.0 milestone May 13, 2025

georgi-l95 self-assigned this May 13, 2025

georgi-l95 added New Feature A new feature, service, or documentation. Major changes that are not backwards compatible. Server Status Plugin Issue related to Server Status Plugin labels May 13, 2025

georgi-l95 linked an issue May 13, 2025 that may be closed by this pull request

Create Server Status Plugin #1111

Open

georgi-l95 added 2 commits May 13, 2025 16:22

chore: add issue numbers to todos

e5c98dc

Signed-off-by: georgi-l95 <glazarov95@gmail.com>

test: add e2e test

1950594

Signed-off-by: georgi-l95 <glazarov95@gmail.com>

ata-nas reviewed May 15, 2025

View reviewed changes

chore: update pbj version

523c61a

Signed-off-by: georgi-l95 <glazarov95@gmail.com>

georgi-l95 marked this pull request as ready for review May 16, 2025 13:43

georgi-l95 requested review from a team as code owners May 16, 2025 13:43

georgi-l95 requested a review from jasperpotts May 16, 2025 13:43

jsync-swirlds requested a review from AlfredoG87 May 16, 2025 17:04

jsync-swirlds reviewed May 16, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: create server status plugin #1137

feat: create server status plugin #1137

georgi-l95 commented May 13, 2025 •

edited

Loading

codecov bot commented May 13, 2025

ata-nas left a comment

ata-nas May 15, 2025

georgi-l95 May 16, 2025

jsync-swirlds May 16, 2025

ata-nas May 15, 2025

georgi-l95 May 16, 2025

jsync-swirlds May 16, 2025

ata-nas May 15, 2025

georgi-l95 May 16, 2025

ata-nas May 15, 2025

georgi-l95 May 16, 2025

jsync-swirlds May 16, 2025

jsync-swirlds May 16, 2025

jsync-swirlds May 16, 2025

		String[] parts = fullName().split("\\.");
		return parts[parts.length - 1];

		final long firstAvailableBlock = blockProvider.availableBlocks().min();
		final long lastAvailableBlock = blockProvider.availableBlocks().max();

		assertEquals(0, response.firstAvailableBlock());
		assertEquals(0, response.lastAvailableBlock());

feat: create server status plugin #1137

Are you sure you want to change the base?

feat: create server status plugin #1137

Conversation

georgi-l95 commented May 13, 2025 • edited Loading

Reviewer Notes

Related Issue(s)

codecov bot commented May 13, 2025

Codecov Report

ata-nas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

georgi-l95 commented May 13, 2025 •

edited

Loading