Skip to content

Conversation

AlfredoG87
Copy link
Contributor

@AlfredoG87 AlfredoG87 commented Jul 10, 2025

Reviewer Notes

  • Added a new sub-project backfill[block-node-backfill]
    • This includes changes to gradle settings, new folder, its own build.gradle.kts, module-info.java file, BackfillConfiguraion.java, initial BackfillPlugin.java and added to block-node/app/build.gradle.kts as a runtimeOnly dependency so it can be picked up by the plugin system.
  • Added proto definition to match block-node.json file example from CN and MN, to use same schema and file and wired up the PBJ Compiler for that proto and generate "POJO" Classes BlockNodeSources and BlockNodeConfig that can be used to serialize and deseralize its corresponding json files.
    • added an example of such file to be used in testing resources of the backfill plugin project: block-node.json
    • includes the configuration of the PBJ Compiler on the build.gradle.kts file of the backfill plugin.
  • gRPC Client or "Blocks Fetcher", this is the classes that allow the backfill plugin to connect to the list of block-nodes, and probe for the requested blocks using the serverStatus API endpoint and then the subscribeBlockStream endpoint to fetch the actual blocks.
    • BackfillGrpcClient: is the top level "gRPC Client" class, it contains the BlockNodeSources list and is responsible for instantiating new BlockNodeClients and managing their lifetime cycle, and current status, as well as to implement retry mechanism with exponential backoff.
    • BlockNodeClient: the class represents a given BN, holds a single GrpcClient that gets re-used over the lifetime of the application, and shared across all the different services that the BlockNodeClient uses, in this case 2:
    • BlockNodeServerStatusClient: is a convenient abstraction that uses the same GrpcClient for the host but provides access to serverStatus endpoint.
    • BlockNodeSubscribeClient, same as above but for subscribeBlockStream API service endpoint. abstracts away receiving the blocks and exposes an intuitive api that based on the request start and end returns a list of the blocks requested.
  • Add new notification type: BackfilledBlockNotification
  • Add BlockSource Enum to distinguish between Verification and Persistence Notifications by source
  • extend VerificationPlugin & PersistencePlugin to respect BlockSource enum (no breaking changes for publishers)
  • added autonomous loop scheduler, when starting the backfill plugin it schedules an autonomous task that will be running and trying to detect missing gaps and backfill them if found.
  • Initial set of metrics defined in design document (however on a follow-up PR when doing dashboard we might review, improve and add more metrics)

Testing:

  • BN Server Mock on text fixtures to allow the retrieval of blocks from another BN on the test.
  • Fixed SimpleBlockRangeSet used in Testing Fixtures and UTs, specifically the method streamRanges that was not working as expected.
  • Happy Path Unit Test
  • Missing E2E tests, this will be added on a folllow-up PR.

Related Issue(s)

Fixes #1351
Fixes #1352
Fixes #1353
Fixes #1354

@AlfredoG87 AlfredoG87 self-assigned this Jul 10, 2025
@AlfredoG87 AlfredoG87 added New Feature A new feature, service, or documentation. Major changes that are not backwards compatible. Block Node Issues/PR related to the Block Node. labels Jul 10, 2025
@AlfredoG87 AlfredoG87 added this to the 0.15.0 milestone Jul 10, 2025
@AlfredoG87 AlfredoG87 changed the title feat(backfill): Backfill Plugin (Draft) feat(backfill): Backfill Plugin - Part 1 Autonomous Flow Jul 19, 2025
@AlfredoG87 AlfredoG87 marked this pull request as ready for review July 21, 2025 15:07
@AlfredoG87 AlfredoG87 requested review from a team as code owners July 21, 2025 15:07
@Nana-EC Nana-EC requested review from ata-nas, jsync-swirlds and Copilot and removed request for mattp-swirldslabs July 21, 2025 16:59
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces a comprehensive backfill plugin system that enables block nodes to automatically detect and fill gaps in their historical block data by fetching missing blocks from other block nodes in the network. The implementation includes autonomous gap detection, gRPC client infrastructure for inter-node communication, and extends the existing block messaging system to support backfill operations.

  • Adds a new backfill plugin module with autonomous gap detection and block fetching capabilities
  • Introduces a new BlockSource enum and BackfilledBlockNotification type to distinguish between publisher and backfill operations
  • Extends existing verification and persistence plugins to handle backfilled blocks appropriately

Reviewed Changes

Copilot reviewed 41 out of 41 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
settings.gradle.kts Adds backfill module to the build configuration
block-node/spi/src/main/java/org/hiero/block/node/spi/blockmessaging/* Introduces BlockSource enum and BackfilledBlockNotification types
block-node/messaging/src/main/java/* Updates messaging infrastructure to support backfilled block notifications
block-node/verification/src/main/java/* Extends verification plugin to handle backfilled blocks
block-node/backfill/* Complete new backfill plugin implementation with gRPC client and configuration
Comments suppressed due to low confidence (1)

@jsync-swirlds
Copy link
Contributor

If you're going to experiment with copilot as reviewer, please do so on a separate PR to reduce the noise for everyone else.
Of 4 comments, 3 are partly false (describing a different typo than the one corrected) and the 4th is just a bad change (it should have just suggested replacing a comma with a period).
This is wasteful of time and attention.

Copy link
Contributor

@ata-nas ata-nas left a comment

Choose a reason for hiding this comment

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

Great progress @AlfredoG87!

I leave comments and considerations to help you cleanup. I have only reviewed
/block-node/app and /block-node/backfill as a first pass. No tests reviewed as well.

More reviews to come. 🙌

Copy link
Contributor

@Nana-EC Nana-EC left a comment

Choose a reason for hiding this comment

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

Made it halfway, good start.
Will circle back for a 2nd pass

Copy link
Contributor

@ata-nas ata-nas left a comment

Choose a reason for hiding this comment

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

Good job @AlfredoG87!

I leave a couple of more comments after going through all the rest of the files.

@AlfredoG87 AlfredoG87 requested a review from ata-nas July 23, 2025 19:11
ata-nas
ata-nas previously approved these changes Jul 23, 2025
Copy link
Contributor

@ata-nas ata-nas left a comment

Choose a reason for hiding this comment

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

Unblocking this to make progress. @AlfredoG87 will address concerns that were captured in follow ups.
Great work!

Nana-EC
Nana-EC previously approved these changes Jul 23, 2025
Copy link
Contributor

@Nana-EC Nana-EC left a comment

Choose a reason for hiding this comment

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

Unblocking based on offline sync.
Non-functional suggestions will be addressed by @AlfredoG87 in 0.16.

Copy link
Contributor

@jsync-swirlds jsync-swirlds left a comment

Choose a reason for hiding this comment

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

Good work.
A couple items that probably should be addressed here, but pretty much everything could be addressed in follow-up work.

Includes Configuration, module-info.java, gradle project and BlockGap Record.

Signed-off-by: Alfredo Gutierrez Grajeda <[email protected]>

- proto structure for json file as it is being used by CN and MN: block_node_source.proto
- block-nodes.json sources file example on test resources
- Added PBJ Compiler plugin and task to generate BlockNodeSources classes

Signed-off-by: Alfredo Gutierrez Grajeda <[email protected]>

- Added BlockNode ServerStatus Service Client for fetching the serverStatus method
- Added BlockNode Subscribe Service Client for fetching blocks using blockStreamSubscribe method
- Added BlockNode Client that re-uses a single webClient within all the service clients.
- Added BackfillGrpcClient with retry logic, gap check and Json parse and Backfill gRPC client related logic
- needed Helidon dependencies and module-info.java package changes

Signed-off-by: Alfredo Gutierrez Grajeda <[email protected]>

- UT skeleton for backfill plugin
- Some logic related to gap detection and blocks fetching, still very raw and uncompleted

Signed-off-by: Alfredo Gutierrez Grajeda <[email protected]>

- fixed module-info.java

Signed-off-by: Alfredo Gutierrez Grajeda <[email protected]>

- stylefix

Signed-off-by: Alfredo Gutierrez Grajeda <[email protected]>

- Add new notification type: BackfilledBlockNotification
- Refactor BlockNodeSubscribeClient to work with UnparsedBlock Responses instead of parsed.
- Other general improvements

Signed-off-by: Alfredo Gutierrez Grajeda <[email protected]>

stylefix

Signed-off-by: Alfredo Gutierrez Grajeda <[email protected]>

Adding a lifetime cycle management to the BackfillGrpcCLient so it can re-use BlockNodeClient classes, optimizing the use of resources throughout the life of the service.

Signed-off-by: Alfredo Gutierrez Grajeda <[email protected]>

nit import optimization and logger level change

Signed-off-by: Alfredo Gutierrez Grajeda <[email protected]>

- Add BlockSource Enum to distinguish between Verification and Persistence Notifications by source
- extend VerificationPlugin & PersistencePlugin to respect BlockSource enum (no breaking changes for publishers).

Signed-off-by: Alfredo Gutierrez Grajeda <[email protected]>

- added autonomous loop scheduler
- improve metrics and logs
- added countdownLatch to wait for last batch to finish processing before starting a new one.
- added cooldown period between batches to give system and network time to catch its breath
- added persistence and verification block notification handlers to work together with the countdownLatch mechanism

Signed-off-by: Alfredo Gutierrez Grajeda <[email protected]>

- added functionality for backfill retries metrics counter
- fixed persistence and verification handlers to ignore block notifications that are not originating from backfill itself.

Signed-off-by: Alfredo Gutierrez Grajeda <[email protected]>

- renamed status to Status, comply with codacy suggestions

Signed-off-by: Alfredo Gutierrez Grajeda <[email protected]>

- removed BlockGap in favor of LongRange, in preparations for further improvements also for simplification

Signed-off-by: Alfredo Gutierrez Grajeda <[email protected]>

clean up, simplification and improvement in readability overall housekeeping

Signed-off-by: Alfredo Gutierrez Grajeda <[email protected]>

- Some improvements on BackfillPlugin that came up during Happy Path UT
- BN Serve Mock on text fixtures to allow the retrival of blocks from another BN on the test.
- Optimizations of module-info.java files
- Happy Path Unit Test
- Added Configuration for initial delay of first task execution.

Signed-off-by: Alfredo Gutierrez Grajeda <[email protected]>

- fixed UT

Signed-off-by: Alfredo Gutierrez Grajeda <[email protected]>

- fixed UT path to relative

Signed-off-by: Alfredo Gutierrez Grajeda <[email protected]>

- added more UTs and fixed retry logic since it was taking too long due to the exponential nature of increasing, so I made it linear.

Signed-off-by: Alfredo Gutierrez Grajeda <[email protected]>

adding stop after the test.

Signed-off-by: Alfredo Gutierrez Grajeda <[email protected]>

added a third block node mock server for an online but without the needed gap example.

Signed-off-by: Alfredo Gutierrez Grajeda <[email protected]>

Added a new UT when not a single block-node is available

Signed-off-by: Alfredo Gutierrez Grajeda <[email protected]>

We only need 1 unavailable BN for this test

Signed-off-by: Alfredo Gutierrez Grajeda <[email protected]>

adding some test coverage to the messaging facility impl changes

Signed-off-by: Alfredo Gutierrez Grajeda <[email protected]>

Copilot code review suggestions

Signed-off-by: Alfredo Gutierrez Grajeda <[email protected]>

Addressed all PR review comments

Signed-off-by: Alfredo Gutierrez Grajeda <[email protected]>

Addressed all PR review comments and some more so CI passes again.

Signed-off-by: Alfredo Gutierrez Grajeda <[email protected]>

some more PR comments addressed around BlockSource not using null but UNKNOWN

Signed-off-by: Alfredo Gutierrez Grajeda <[email protected]>

Configuration improvements

Signed-off-by: Alfredo Gutierrez Grajeda <[email protected]>

improving comment around batch latch

Signed-off-by: Alfredo Gutierrez Grajeda <[email protected]>

PR review comments improvements

Signed-off-by: Alfredo Gutierrez Grajeda <[email protected]>
Signed-off-by: Alfredo Gutierrez Grajeda <[email protected]>
Nana-EC
Nana-EC previously approved these changes Jul 24, 2025
Signed-off-by: Alfredo Gutierrez Grajeda <[email protected]>
Signed-off-by: Alfredo Gutierrez Grajeda <[email protected]>
@AlfredoG87 AlfredoG87 merged commit a7070d0 into main Jul 24, 2025
13 of 14 checks passed
@AlfredoG87 AlfredoG87 deleted the backfill-plugin branch July 24, 2025 21:43
Copy link

codecov bot commented Jul 24, 2025

Codecov Report

Attention: Patch coverage is 78.97196% with 90 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
.../org/hiero/block/node/backfill/BackfillPlugin.java 78.61% 24 Missing and 10 partials ⚠️
...node/backfill/client/BlockNodeSubscribeClient.java 70.78% 18 Missing and 8 partials ⚠️
...e/backfill/client/BlockNodeServerStatusClient.java 68.51% 13 Missing and 4 partials ⚠️
...k/node/verification/VerificationServicePlugin.java 62.50% 6 Missing ⚠️
.../hiero/block/node/backfill/BackfillGrpcClient.java 94.11% 3 Missing and 1 partial ⚠️
...ock/node/messaging/BlockMessagingFacilityImpl.java 40.00% 3 Missing ⚠️

❌ Your patch status has failed because the patch coverage (78.97%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage.

@@             Coverage Diff              @@
##               main    #1384      +/-   ##
============================================
- Coverage     79.55%   79.49%   -0.07%     
- Complexity      964     1022      +58     
============================================
  Files           114      122       +8     
  Lines          4383     4804     +421     
  Branches        461      495      +34     
============================================
+ Hits           3487     3819     +332     
- Misses          716      782      +66     
- Partials        180      203      +23     
Files with missing lines Coverage Δ Complexity Δ
...ero/block/node/backfill/BackfillConfiguration.java 100.00% <100.00%> (ø) 1.00 <1.00> (?)
...ro/block/node/backfill/client/BlockNodeClient.java 100.00% <100.00%> (ø) 3.00 <3.00> (?)
...ocks/files/historic/BlocksFilesHistoricPlugin.java 84.96% <100.00%> (ø) 24.00 <0.00> (ø)
...e/blocks/files/recent/BlocksFilesRecentPlugin.java 71.07% <100.00%> (+0.24%) 15.00 <3.00> (ø)
...ock/node/messaging/BlockNotificationRingEvent.java 100.00% <100.00%> (ø) 7.00 <2.00> (+2.00)
...pi/blockmessaging/BackfilledBlockNotification.java 100.00% <100.00%> (ø) 1.00 <1.00> (?)
...e/spi/blockmessaging/BlockNotificationHandler.java 100.00% <100.00%> (+50.00%) 3.00 <1.00> (+2.00)
...ero/block/node/spi/blockmessaging/BlockSource.java 100.00% <100.00%> (ø) 1.00 <1.00> (?)
...node/spi/blockmessaging/PersistedNotification.java 50.00% <ø> (ø) 1.00 <0.00> (ø)
...e/spi/blockmessaging/VerificationNotification.java 100.00% <ø> (ø) 1.00 <0.00> (ø)
... and 7 more
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Block Node Issues/PR related to the Block Node. New Feature A new feature, service, or documentation. Major changes that are not backwards compatible.
Projects
None yet
4 participants