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.

Sign up for GitHub

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

Jump to bottom

[indexer grpc] add retry and transaction count for grpc. #8169

Merged
merged 1 commit into from
May 17, 2023
Merged

[indexer grpc] add retry and transaction count for grpc. #8169

merged 1 commit into from
May 17, 2023

Conversation

larry-aptos
Copy link
Contributor

@larry-aptos larry-aptos commented May 12, 2023 •
edited
Loading

Description

  • Send response item into channel with timeout
  • Implement transaction count for grpc: once enough transactions are streamed, end it.

Test Plan

Locally tested; looks good.

@larry-aptos larry-aptos requested review from rtso and bowenyang007 May 12, 2023 00:59
@larry-aptos larry-aptos changed the title add retry and transaction count for grpc. [indexer grpc] add retry and transaction count for grpc. May 12, 2023
rustielin
rustielin reviewed May 16, 2023
View reviewed changes
ecosystem/indexer-grpc/indexer-grpc-data-service/src/lib.rs Outdated
anyhow::bail!("Failed to send response to a full channel")
}
current_item = failed_item;
tokio::time::sleep(Duration::from_millis(CHANNEL_RETRY_SLEEP_DURATION_MS)).await;
Copy link
Contributor

Choose a reason for hiding this comment

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

why not exponential backoff?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yea i figured out i can use send_timeout instead; updated the code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated.

@larry-aptos larry-aptos force-pushed the respect-thetransaction-count-setup branch from c148d2c to a619156 Compare May 17, 2023 01:37
@larry-aptos larry-aptos force-pushed the respect-thetransaction-count-setup branch from a619156 to 957b496 Compare May 17, 2023 01:57
@larry-aptos larry-aptos enabled auto-merge (squash) May 17, 2023 06:29
@github-actions

This comment has been minimized.

Sign in to view
@github-actions

This comment has been minimized.

Sign in to view
@github-actions

This comment has been minimized.

Sign in to view
@github-actions
Copy link
Contributor

✅ Forge suite compat success on testnet_2d8b1b57553d869190f61df1aaf7f31a8fc19a7b ==> 957b496891f6f1faba609077fc3003a8fa687d56

Compatibility test results for testnet_2d8b1b57553d869190f61df1aaf7f31a8fc19a7b ==> 957b496891f6f1faba609077fc3003a8fa687d56 (PR)
1. Check liveness of validators at old version: testnet_2d8b1b57553d869190f61df1aaf7f31a8fc19a7b
compatibility::simple-validator-upgrade::liveness-check : 10187 TPS, 3709 ms latency, 6500 ms p99 latency,no expired txns
2. Upgrading first Validator to new version: 957b496891f6f1faba609077fc3003a8fa687d56
compatibility::simple-validator-upgrade::single-validator-upgrade : 5692 TPS, 6956 ms latency, 9300 ms p99 latency,no expired txns
3. Upgrading rest of first batch to new version: 957b496891f6f1faba609077fc3003a8fa687d56
compatibility::simple-validator-upgrade::half-validator-upgrade : 5878 TPS, 6554 ms latency, 8600 ms p99 latency,no expired txns
4. upgrading second batch to new version: 957b496891f6f1faba609077fc3003a8fa687d56
compatibility::simple-validator-upgrade::rest-validator-upgrade : 8259 TPS, 4564 ms latency, 8000 ms p99 latency,no expired txns
5. check swarm health
Compatibility test for testnet_2d8b1b57553d869190f61df1aaf7f31a8fc19a7b ==> 957b496891f6f1faba609077fc3003a8fa687d56 passed
Test Ok

@github-actions
Copy link
Contributor

✅ Forge suite land_blocking success on 957b496891f6f1faba609077fc3003a8fa687d56

performance benchmark : 5932 TPS, 6663 ms latency, 27400 ms p99 latency,(!) expired 86 out of 2533340 txns
Test Ok

@github-actions
Copy link
Contributor

✅ Forge suite framework_upgrade success on aptos-node-v1.3.0_3fc3d42b6cfe27460004f9a0326451bcda840a60 ==> 957b496891f6f1faba609077fc3003a8fa687d56

Compatibility test results for aptos-node-v1.3.0_3fc3d42b6cfe27460004f9a0326451bcda840a60 ==> 957b496891f6f1faba609077fc3003a8fa687d56 (PR)
Upgrade the nodes to version: 957b496891f6f1faba609077fc3003a8fa687d56
framework_upgrade::framework-upgrade::full-framework-upgrade : 6073 TPS, 6513 ms latency, 10500 ms p99 latency,no expired txns
5. check swarm health
Compatibility test for aptos-node-v1.3.0_3fc3d42b6cfe27460004f9a0326451bcda840a60 ==> 957b496891f6f1faba609077fc3003a8fa687d56 passed
Test Ok

@larry-aptos larry-aptos merged commit e6a6d2e into main May 17, 2023
@larry-aptos larry-aptos deleted the respect-thetransaction-count-setup branch May 17, 2023 07:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rustielin rustielin rustielin approved these changes

@bowenyang007 bowenyang007 bowenyang007 approved these changes

@rtso rtso Awaiting requested review from rtso rtso is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@larry-aptos @bowenyang007 @rustielin