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

Improve monitor startup performance (0.38.1) #2397

Merged
merged 1 commit into from
Aug 11, 2021

Conversation

steven-sheehy
Copy link
Member

Description:

  • Improve monitor startup performance by moving publish and subscribe flow initialization off main thread
  • Increase node validation timeout to 30s
  • Adjust monitor probe delay to reflect startup improvement

Related issue(s):

Notes for reviewer:
Cherry pick of #2395 to 0.38.1

Checklist

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

* Improve monitor startup performance by moving publish and subscribe flow initialization off main thread
* Increase node validation timeout to 30s
* Adjust monitor probe delay to reflect startup improvement

Signed-off-by: Steven Sheehy <steven.sheehy@hedera.com>
@steven-sheehy steven-sheehy added bug Type: Something isn't working P1 performance monitor Area: Monitoring and dashboard labels Aug 11, 2021
@steven-sheehy steven-sheehy added this to the Mirror 0.38.1 milestone Aug 11, 2021
@steven-sheehy steven-sheehy requested a review from a team August 11, 2021 18:59
@steven-sheehy steven-sheehy self-assigned this Aug 11, 2021
@steven-sheehy steven-sheehy changed the title Improve monitor startup performance (#2395) Improve monitor startup performance (0.38.1) Aug 11, 2021
@codecov
Copy link

codecov bot commented Aug 11, 2021

Codecov Report

Merging #2397 (923329e) into release/0.38 (4f84baa) will decrease coverage by 0.03%.
The diff coverage is 60.00%.

Impacted file tree graph

@@                Coverage Diff                 @@
##             release/0.38    #2397      +/-   ##
==================================================
- Coverage           84.35%   84.32%   -0.04%     
+ Complexity           2309     2308       -1     
==================================================
  Files                 439      439              
  Lines               11982    11983       +1     
  Branches             1020     1020              
==================================================
- Hits                10108    10105       -3     
- Misses               1556     1560       +4     
  Partials              318      318              
Impacted Files Coverage Δ
...ra/mirror/monitor/config/MonitorConfiguration.java 0.00% <0.00%> (ø)
...a/mirror/monitor/publish/TransactionPublisher.java 92.78% <100.00%> (ø)
...porter/migration/MissingAddressBooksMigration.java 83.33% <0.00%> (-16.67%) ⬇️
...epository/upsert/AbstractUpsertQueryGenerator.java 89.91% <0.00%> (-0.85%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4f84baa...923329e. Read the comment docs.

@sonarcloud
Copy link

sonarcloud bot commented Aug 11, 2021

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

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.

LGTM

@@ -186,8 +186,8 @@ private boolean validateNode(Client client, NodeProperties node) {
new TransferTransaction()
.addHbarTransfer(nodeAccountId, hbar)
.addHbarTransfer(client.getOperatorAccountId(), hbar.negated())
.execute(client, Duration.ofSeconds(10L))
.getReceipt(client, Duration.ofSeconds(10L));
.execute(client, Duration.ofSeconds(30L))
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: if you circle - move to shared constant.
Meant to say this in main before you merged.

@steven-sheehy steven-sheehy merged commit 2303dd3 into release/0.38 Aug 11, 2021
@steven-sheehy steven-sheehy deleted the monitor-startup-38 branch August 11, 2021 19:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Type: Something isn't working monitor Area: Monitoring and dashboard P1 performance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants