Skip to content

Comments

feat: Create new stream subscriber plugin#1058

Merged
Nana-EC merged 1 commit intomainfrom
1051-create-stream-subscriber-plugin
Apr 26, 2025
Merged

feat: Create new stream subscriber plugin#1058
Nana-EC merged 1 commit intomainfrom
1051-create-stream-subscriber-plugin

Conversation

@jsync-swirlds
Copy link
Contributor

@jsync-swirlds jsync-swirlds commented Apr 23, 2025

Reviewer Notes

Adding a new plugin

  • Instead of beating on the old implementation, implement a new subscriber plugin
    that uses a single client thread and feeds that alternately from messaging or history
    This avoids the countdown latch and state transition logic.
    Also avoids a state machine, just take the next block from one source or the other,
    always getting just the next block to send the client.
    Does require a secondary queue from the messaging, but it's small and simple.
  • Adjust the subscriber "method" class to be reused, so we track sessions properly
  • Added a latch so the pipeline thread holds until the session is registered with
    messaging before returning. This has minimal impact in production, and makes
    it possible to test reliably.
  • Added live block push to queue, live blocks stream correctly.
  • Added handling for request ahead of live condition
    • Session just skips over live blocks until live reaches the start block.
  • Added proper shutdown of all sessions if the plugin is stopped.
  • History and Live both work, and the test disables history to prove it
    • This is necessary because otherwise the test "history" plugin is too fast
      and we always serve history.

TODO

  • Add more unit tests
  • Add testing that forces a client to fall behind and catch up
  • Add some end-to-end test suites to exercise this plugin better
  • Add more and better javadoc
  • Clean up the (excessive) trace logging
  • Add better metrics, the "transition" metrics aren't super helpful.
  • Suggested metrics
    • Live batches sent to client (would be good if we can tag this with a client ID)
    • History blocks sent to client (would be good if we can tag this with a client ID)
    • Count of times the client thread must wait for live blocks, and time spent waiting
    • Count of times the client cannot read from history, but subsequently reads from live
    • Client connection and time from connect to session established

Related Issue(s)

Resolves: #1051

@jsync-swirlds jsync-swirlds added the Block Node Issues/PR related to the Block Node. label Apr 23, 2025
@jsync-swirlds jsync-swirlds added this to the 0.10.0 milestone Apr 23, 2025
@jsync-swirlds jsync-swirlds self-assigned this Apr 23, 2025
@jsync-swirlds jsync-swirlds force-pushed the 1051-create-stream-subscriber-plugin branch 6 times, most recently from 28d6262 to c56d814 Compare April 25, 2025 01:49
@georgi-l95 georgi-l95 added the Subscriber Plugin Issue related to Subscriber Plugin label Apr 25, 2025
Copy link
Member

@georgi-l95 georgi-l95 left a comment

Choose a reason for hiding this comment

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

Also we'd need to replace in block-node/app/build.gradle.kts:
runtimeOnly("org.hiero.block.node.subscriber")
with:
runtimeOnly("org.hiero.block.node.stream.subscriber")

Looking good, done some manual testing, which will be transformed into E2E tests. Those included:

  • Live streaming - Send request with -1 start block to -1 end block. ✅
  • Historic streaming - Send request with 0 start block to 10 end block. ✅
  • Historic -> Live -> Historic -> Live streaming - Send request with 0 start block to -1 end block. We started receiving blocks, then (with some modifications on simulator side) waited couple of seconds to get behind, then the plugin switched us to historic and when we caught up, we switched to live. ✅

Things left to try:

  • Live streaming on two or more consumers
  • Historic streaming on two or more consumers
  • Historic -> Live -> Historic -> Live streaming on two or more consumers

@jsync-swirlds jsync-swirlds force-pushed the 1051-create-stream-subscriber-plugin branch 2 times, most recently from a4668ac to 64819d4 Compare April 25, 2025 14:51
 * Instead of beating on the old implementation, implement a new subscriber plugin
   that uses a single client thread and feeds that alternately from messaging or history
   This avoids the countdown latch and state transition logic.
   Also avoids a state machine, just take the next block from one source or the other,
   always getting just the next block to send the client.
   Does require a secondary queue from the messaging, but it's small and simple.
* Adjust the subscriber "method" class to be reused, so we track sessions properly
* Added a latch so the pipeline thread holds until the session is registered with
  messaging before returning. This has minimal impact in production, and makes
  it possible to test reliably.
* Added live block push to queue, live blocks stream correctly.
* Added handling for request ahead of live condition
    * Session just skips over live blocks until live reaches the start block.
* Added proper shutdown of all sessions if the plugin is stopped.
* History and Live both work, and the test disables history to prove it
    * This is necessary because otherwise the test "history" plugin is too fast and we always serve history.

* TODO:
    * Add more unit tests
    * Add testing that forces a client to fall behind and catch up
    * Add some end-to-end test suites to exercise this plugin better
    * Add more and better javadoc
    * Clean up the (excessive) trace logging
    * Add better metrics, the "transition" metrics aren't super helpful.
    * Suggested metrics
        * Live batches sent to client (would be good if we can tag this with a client ID)
        * History blocks sent to client (would be good if we can tag this with a client ID)
        * Count of times the client thread must wait for live blocks, and time spent waiting
        * Count of times the client cannot read from history, but subsequently reads from live
        * Client connection and time from connect to session established

Signed-off-by: Joseph Sinclair <121976561+jsync-swirlds@users.noreply.github.com>
@jsync-swirlds jsync-swirlds force-pushed the 1051-create-stream-subscriber-plugin branch from 64819d4 to 393aa2f Compare April 25, 2025 15:24
@jsync-swirlds jsync-swirlds marked this pull request as ready for review April 25, 2025 15:24
@jsync-swirlds jsync-swirlds requested review from a team as code owners April 25, 2025 15:24
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.

@jsync-swirlds looks good! Looking forward to see the improvements done based on the todo. 🔥

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.

Nice work.
Made it a 1/3 of the way through and will circle back

Copy link
Contributor

@AlfredoG87 AlfredoG87 left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@AlfredoG87 AlfredoG87 left a comment

Choose a reason for hiding this comment

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

Just a nit.

@jasperpotts
Copy link
Contributor

I want to go through in detail but don't want to hold up as I will not get a chance till after devcon. So can merge and I will review later.

@Nana-EC Nana-EC merged commit 6c4a193 into main Apr 26, 2025
14 of 15 checks passed
@Nana-EC Nana-EC deleted the 1051-create-stream-subscriber-plugin branch April 26, 2025 02:10
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. Subscriber Plugin Issue related to Subscriber Plugin

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Enhance/Extend subscriber to feature-match pre-modular version

6 participants