Skip to content
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

refactor: Arch changes to address performance bottlenecks #171

Merged
merged 63 commits into from
Sep 19, 2024

Conversation

mattp-swirldslabs
Copy link
Contributor

@mattp-swirldslabs mattp-swirldslabs commented Sep 10, 2024

Changes:

  • Block item persistence is now a subscriber to the LiveStreamMediator to make it async
  • Now a single response is sent for the entire block rather than for each block item
  • Added an additional RingBuffer on the Notifier to handle publishing responses to many producers
  • Centralized ringbuffer subscription handling in base class
  • Adjusted the previous exception handling to trigger events in the Notifier
  • Added configuration and dagger injection mechanisms for new components
  • Improvements to subscription handling
  • Added metrics for the new response flow and for errors
  • Added and adjusted test coverage

Closes #168

  • Documented (Code comments, README, etc.)
  • Tested (unit, integration, etc.)

@mattp-swirldslabs mattp-swirldslabs added the Feature Enhancement Enhancing an existing feature driven by business requirements. Typically backwards compatible. label Sep 10, 2024
@mattp-swirldslabs mattp-swirldslabs added this to the 0.1.0 milestone Sep 10, 2024
@mattp-swirldslabs mattp-swirldslabs self-assigned this Sep 10, 2024
@mattp-swirldslabs mattp-swirldslabs changed the title refactor: 00168 ref async refactor: Arch changes to address performance bottlenecks Sep 17, 2024
@mattp-swirldslabs mattp-swirldslabs marked this pull request as ready for review September 17, 2024 18:54
@mattp-swirldslabs mattp-swirldslabs requested a review from a team as a code owner September 17, 2024 18:54
georgi-l95
georgi-l95 previously approved these changes Sep 18, 2024
Copy link
Contributor

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

Looks good to me, some questions, other than that is fine IMO.

AlfredoG87
AlfredoG87 previously approved these changes Sep 18, 2024
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.

Good Job, leaved some comments, mostly nits some questions and probably future considerations that can be addressed in a future PR.

AlfredoG87
AlfredoG87 previously approved these changes Sep 18, 2024
Signed-off-by: Matt Peterson <[email protected]>
Signed-off-by: Matt Peterson <[email protected]>
Signed-off-by: Matt Peterson <[email protected]>
Signed-off-by: Matt Peterson <[email protected]>
Signed-off-by: Matt Peterson <[email protected]>
Signed-off-by: Matt Peterson <[email protected]>
Signed-off-by: Matt Peterson <[email protected]>
Signed-off-by: Matt Peterson <[email protected]>
Signed-off-by: Matt Peterson <[email protected]>
Signed-off-by: Matt Peterson <[email protected]>
Signed-off-by: Matt Peterson <[email protected]>
Signed-off-by: Matt Peterson <[email protected]>
Signed-off-by: Matt Peterson <[email protected]>
… and the app.properties to work with containers

Signed-off-by: Matt Peterson <[email protected]>
Signed-off-by: Matt Peterson <[email protected]>
Signed-off-by: Matt Peterson <[email protected]>
… resources app.properties

Signed-off-by: Matt Peterson <[email protected]>
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.

Nice! 👍

@mattp-swirldslabs mattp-swirldslabs merged commit f3e1b62 into main Sep 19, 2024
9 checks passed
@mattp-swirldslabs mattp-swirldslabs deleted the 00168-ref-async branch September 19, 2024 16:04
Copy link

codecov bot commented Sep 19, 2024

Codecov Report

Attention: Patch coverage is 99.14530% with 2 lines in your changes missing coverage. Please review.

Project coverage is 99.73%. Comparing base (5ab76fd) to head (33eb36b).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...hedera/block/server/service/ServiceStatusImpl.java 77.77% 2 Missing ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##                main     #171      +/-   ##
=============================================
- Coverage     100.00%   99.73%   -0.27%     
- Complexity       160      208      +48     
=============================================
  Files             35       44       +9     
  Lines            616      763     +147     
  Branches          37       48      +11     
=============================================
+ Hits             616      761     +145     
- Misses             0        2       +2     
Files with missing lines Coverage Δ
...ain/java/com/hedera/block/server/BlockNodeApp.java 100.00% <ø> (ø)
...dera/block/server/BlockNodeAppInjectionModule.java 100.00% <ø> (ø)
...va/com/hedera/block/server/BlockStreamService.java 100.00% <100.00%> (ø)
.../block/server/config/BlockNodeConfigExtension.java 100.00% <ø> (ø)
...era/block/server/config/ConfigInjectionModule.java 100.00% <100.00%> (ø)
...m/hedera/block/server/consumer/ConsumerConfig.java 100.00% <100.00%> (ø)
...erver/consumer/ConsumerStreamResponseObserver.java 100.00% <100.00%> (ø)
...era/block/server/events/BlockNodeEventHandler.java 100.00% <100.00%> (ø)
...hedera/block/server/events/LivenessCalculator.java 100.00% <100.00%> (ø)
...va/com/hedera/block/server/events/ObjectEvent.java 100.00% <ø> (ø)
... and 17 more

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature Enhancement Enhancing an existing feature driven by business requirements. Typically backwards compatible.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

refactor: adjust current architecture to increase performance
4 participants