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

Cleanup TRANSACTION_SIMULATION_ENHANCEMENT feature flag #15358

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

vusirikala
Copy link
Contributor

@vusirikala vusirikala commented Nov 21, 2024

Description

Cleaning up and deprecating old code that runs only when the TRANSACTION_SIMULATION_ENHANCEMENT feature flag is disabled.

How Has This Been Tested?

Key Areas to Review

Type of Change

  • New feature
  • Bug fix
  • Breaking change
  • Performance improvement
  • Refactoring
  • Dependency update
  • Documentation update
  • Tests

Which Components or Systems Does This Change Impact?

  • Validator Node
  • Full Node (API, Indexer, etc.)
  • Move/Aptos Virtual Machine
  • Aptos Framework
  • Aptos CLI/SDK
  • Developer Infrastructure
  • Move Compiler
  • Other (specify)

Checklist

  • I have read and followed the CONTRIBUTING doc
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I identified and added all stakeholders and component owners affected by this change as reviewers
  • I tested both happy and unhappy path of the functionality
  • I have made corresponding changes to the documentation

Copy link

trunk-io bot commented Nov 21, 2024

@vusirikala vusirikala requested review from junkil-park and movekevin and removed request for davidiw, wrwg and zekun000 November 21, 2024 23:00
@vusirikala vusirikala added the CICD:run-e2e-tests when this label is present github actions will run all land-blocking e2e tests from the PR label Nov 21, 2024
Copy link
Contributor

@georgemitenkov georgemitenkov left a comment

Choose a reason for hiding this comment

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

Amazing! Thanks!

This comment has been minimized.

This comment has been minimized.

Copy link
Contributor

✅ Forge suite realistic_env_max_load success on 0496ca5356f10919a0f09c22c91bcb8e2f823e88

two traffics test: inner traffic : committed: 14490.23 txn/s, latency: 2745.12 ms, (p50: 2700 ms, p70: 2700, p90: 2900 ms, p99: 3000 ms), latency samples: 5509520
two traffics test : committed: 100.02 txn/s, latency: 1470.98 ms, (p50: 1400 ms, p70: 1400, p90: 1500 ms, p99: 7400 ms), latency samples: 1720
Latency breakdown for phase 0: ["MempoolToBlockCreation: max: 1.970, avg: 1.456", "ConsensusProposalToOrdered: max: 0.327, avg: 0.292", "ConsensusOrderedToCommit: max: 0.373, avg: 0.356", "ConsensusProposalToCommit: max: 0.661, avg: 0.648"]
Max non-epoch-change gap was: 0 rounds at version 0 (avg 0.00) [limit 4], 0.82s no progress at version 2925293 (avg 0.20s) [limit 15].
Max epoch-change gap was: 0 rounds at version 0 (avg 0.00) [limit 4], 8.49s no progress at version 2925291 (avg 8.49s) [limit 15].
Test Ok

Copy link
Contributor

✅ Forge suite compat success on a734d235af377347196f25d90291df51a23915b5 ==> 0496ca5356f10919a0f09c22c91bcb8e2f823e88

Compatibility test results for a734d235af377347196f25d90291df51a23915b5 ==> 0496ca5356f10919a0f09c22c91bcb8e2f823e88 (PR)
1. Check liveness of validators at old version: a734d235af377347196f25d90291df51a23915b5
compatibility::simple-validator-upgrade::liveness-check : committed: 13641.80 txn/s, latency: 2464.91 ms, (p50: 1800 ms, p70: 1800, p90: 2700 ms, p99: 23200 ms), latency samples: 532820
2. Upgrading first Validator to new version: 0496ca5356f10919a0f09c22c91bcb8e2f823e88
compatibility::simple-validator-upgrade::single-validator-upgrading : committed: 7100.69 txn/s, latency: 4013.42 ms, (p50: 4500 ms, p70: 4900, p90: 5000 ms, p99: 5000 ms), latency samples: 131640
compatibility::simple-validator-upgrade::single-validator-upgrade : committed: 6593.25 txn/s, latency: 4597.88 ms, (p50: 4800 ms, p70: 4800, p90: 4900 ms, p99: 6400 ms), latency samples: 243760
3. Upgrading rest of first batch to new version: 0496ca5356f10919a0f09c22c91bcb8e2f823e88
compatibility::simple-validator-upgrade::half-validator-upgrading : committed: 8275.72 txn/s, latency: 3416.79 ms, (p50: 3800 ms, p70: 3900, p90: 4100 ms, p99: 4400 ms), latency samples: 151420
compatibility::simple-validator-upgrade::half-validator-upgrade : committed: 6196.20 txn/s, latency: 5155.77 ms, (p50: 4000 ms, p70: 4100, p90: 12600 ms, p99: 14000 ms), latency samples: 232000
4. upgrading second batch to new version: 0496ca5356f10919a0f09c22c91bcb8e2f823e88
compatibility::simple-validator-upgrade::rest-validator-upgrading : committed: 11780.77 txn/s, latency: 2315.22 ms, (p50: 2300 ms, p70: 2500, p90: 3500 ms, p99: 3800 ms), latency samples: 202800
compatibility::simple-validator-upgrade::rest-validator-upgrade : committed: 11313.58 txn/s, latency: 2760.18 ms, (p50: 2600 ms, p70: 2700, p90: 4800 ms, p99: 6000 ms), latency samples: 365140
5. check swarm health
Compatibility test for a734d235af377347196f25d90291df51a23915b5 ==> 0496ca5356f10919a0f09c22c91bcb8e2f823e88 passed
Test Ok

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CICD:run-e2e-tests when this label is present github actions will run all land-blocking e2e tests from the PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants