Skip to content

fix: Improve queue transactions handling#4947

Merged
mversic merged 4 commits intohyperledger-iroha:mainfrom
dima74:diralik/fix-unstable-tps-transactions-queue
Aug 15, 2024
Merged

fix: Improve queue transactions handling#4947
mversic merged 4 commits intohyperledger-iroha:mainfrom
dima74:diralik/fix-unstable-tps-transactions-queue

Conversation

@dima74
Copy link
Copy Markdown
Contributor

@dima74 dima74 commented Aug 6, 2024

Description

Currently in some cases tps numbers (transactions per second) may be very unstable for network of a single peer. See #4727 (comment) and #4727 (comment) for detailed description of the reasons. This PR adjusts how transactions are handled in the queue, so tps will be more stable. Note that average tps should not change (maybe slightly improvement).

Here is comparison of tps:

Before After
1600_main 1600_fixed

Linked issue

Related: #4727

Benefits

Improve stability of the tps in case when there are a lot of requests and transaction queue is full

Checklist

  • I've read CONTRIBUTING.md
  • I've used the standard signed-off commit format (or will squash just before merging)
  • All applicable CI checks pass (or I promised to make them pass later)
  • (optional) I've written unit tests for the code changes
  • I replied to all comments after code review, marking all implemented changes with thumbs up

@dima74 dima74 self-assigned this Aug 6, 2024
@github-actions github-actions bot added the config-changes Changes in configuration and start up of the Iroha label Aug 6, 2024
@github-actions
Copy link
Copy Markdown

github-actions bot commented Aug 6, 2024

@BAStos525

@Erigara Erigara self-assigned this Aug 7, 2024
Comment thread core/src/sumeragi/main_loop.rs Outdated
Copy link
Copy Markdown
Contributor

@Erigara Erigara left a comment

Choose a reason for hiding this comment

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

LGTM, other than small comment

@dima74 dima74 force-pushed the diralik/fix-unstable-tps-transactions-queue branch from 1eeda85 to 847f89b Compare August 7, 2024 12:16
@dima74 dima74 requested a review from Erigara August 7, 2024 12:16
Erigara
Erigara previously approved these changes Aug 7, 2024
@mversic mversic force-pushed the diralik/fix-unstable-tps-transactions-queue branch from 847f89b to 970cc02 Compare August 9, 2024 16:38
@mversic mversic enabled auto-merge (squash) August 9, 2024 16:49
Comment thread core/src/queue.rs
Comment thread core/src/queue.rs
@dima74 dima74 force-pushed the diralik/fix-unstable-tps-transactions-queue branch from 970cc02 to f919b68 Compare August 14, 2024 11:26
@dima74 dima74 requested review from Erigara and mversic August 14, 2024 11:26
Comment thread core/src/queue.rs Outdated
@dima74 dima74 requested a review from mversic August 14, 2024 15:00
Comment thread core/src/queue.rs
@mversic mversic self-requested a review August 14, 2024 15:25
Copy link
Copy Markdown
Contributor

@SamHSmith SamHSmith left a comment

Choose a reason for hiding this comment

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

LGTM

@mversic
Copy link
Copy Markdown
Contributor

mversic commented Aug 15, 2024

failing tests

dima74 added 4 commits August 15, 2024 13:05
Signed-off-by: Dmitry Murzin <diralik@yandex.ru>
Signed-off-by: Dmitry Murzin <diralik@yandex.ru>
Signed-off-by: Dmitry Murzin <diralik@yandex.ru>
Signed-off-by: Dmitry Murzin <diralik@yandex.ru>
@dima74 dima74 force-pushed the diralik/fix-unstable-tps-transactions-queue branch from fecfb81 to d6800e3 Compare August 15, 2024 10:05
@mversic mversic merged commit b9f2844 into hyperledger-iroha:main Aug 15, 2024
@dima74 dima74 deleted the diralik/fix-unstable-tps-transactions-queue branch August 15, 2024 13:47
mversic pushed a commit that referenced this pull request Aug 30, 2024
Signed-off-by: Dmitry Murzin <diralik@yandex.ru>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

config-changes Changes in configuration and start up of the Iroha

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants