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

tools: replace upload_metrics #5470

Merged
merged 1 commit into from Jun 16, 2023
Merged

Conversation

shiqizng
Copy link
Contributor

@shiqizng shiqizng commented Jun 14, 2023

adding a python script to print TPS from the block generator reports to stdout. the output is formatted in a way that can be copied and pasted to google sheets to create bar charts.
And we decided not to use datadog for displaying the perf results; removing upload_metrics script.

output:

Scenario,Conduit_Version,Empty(no_reset)
payment.jumbo(25000),Conduit_Dev_Build_(May22),16982.46
asset.xfer(5000),Conduit_Dev_Build_(May22),10681.88
mixed(5000),Conduit_Dev_Build_(May22),11453.49
mixed.jumbo(25000),Conduit_Dev_Build_(May22),13390.13
payment.small(100),Conduit_Dev_Build_(May22),10010.13
payment.full(5000),Conduit_Dev_Build_(May22),16700.66
asset.destroy(5000),Conduit_Dev_Build_(May22),11481.67
asset.close(5000),Conduit_Dev_Build_(May22),15179.67

@codecov
Copy link

codecov bot commented Jun 14, 2023

Codecov Report

Merging #5470 (fdc5ba5) into master (5ff0c22) will decrease coverage by 9.14%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #5470      +/-   ##
==========================================
- Coverage   55.60%   46.47%   -9.14%     
==========================================
  Files         447      447              
  Lines       63395    63395              
==========================================
- Hits        35253    29461    -5792     
- Misses      25760    31425    +5665     
- Partials     2382     2509     +127     

see 203 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@shiqizng shiqizng marked this pull request as ready for review June 16, 2023 14:28
@shiqizng shiqizng requested review from tzaffi and winder June 16, 2023 14:29
Copy link
Contributor

@tzaffi tzaffi left a comment

Choose a reason for hiding this comment

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

Approving as none of my proposed changes have any impact on function.

Comment on lines +45 to +48
table_header = (
f"Scenario,Conduit_Version,{args.database_description}"
)
print(table_header)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
table_header = (
f"Scenario,Conduit_Version,{args.database_description}"
)
print(table_header)
print(f"Scenario,Conduit_Version,{args.database_description}")

ignoreable nit

return data


def pretty_print(data):
Copy link
Contributor

@tzaffi tzaffi Jun 16, 2023

Choose a reason for hiding this comment

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

I recommend a comment here explaining the conventions:

"""
* scenario config files with "sm" in their name are assumed to have `TXN_PER_BLOCK_SM` transactions per block
* scenario config files with "jumbo" in their names are assumed to have `TXN_PER_BLOCK_JUMBO` transactions per block
* others are assumed to have `TXN_PER_BLOCK` transactions per block
"""

for d in data:
scenario = d["scenario"].split("config.")[1]
scenario_parsed = scenario.split(".yml")[0]
txn_per_block = TXN_PER_BLOCK
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this is an issue, but just noting the fact that I'm about to change this exactness of these numbers going forward. For example, formerly the small scenarios had exactly TXN_PER_BLOCK_SM (100) but now they may have $100 + \epsilon$ .

@winder winder merged commit ae02370 into algorand:master Jun 16, 2023
25 of 29 checks passed
iansuvak pushed a commit to iansuvak/go-algorand that referenced this pull request Jun 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants