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

fix: slow tx task termination #402

Merged
merged 1 commit into from
May 1, 2023

Conversation

myungjin
Copy link
Contributor

Description

tx task termination is slow because putting EMPTY_PAYLOAD was never executed. This was because channel.remove(end) is called that end is going to be removed. Hence, the length of channel._ends is always zero. To address the issue, in channel.remove(), puting EMPTY_PAYLOAD into the tx queue is implemented. With this, every time channel.remove() is called, the given end's tx task is safely terminated.

Type of Change

  • Bug Fix
  • New Feature
  • Breaking Change
  • Refactor
  • Documentation
  • Other (please describe)

Checklist

  • I have read the contributing guidelines
  • Existing issues have been referenced (where applicable)
  • I have verified this change is not present in other open pull requests
  • Functionality is documented
  • All code style checks pass
  • New code contribution is covered by automated tests
  • All new and existing tests pass

tx task termination is slow because putting EMPTY_PAYLOAD was never
executed. This was because channel.remove(end) is called that end is
going to be removed. Hence, the length of channel._ends is always
zero. To address the issue, in channel.remove(), puting EMPTY_PAYLOAD
into the tx queue is implemented. With this, every time
channel.remove() is called, the given end's tx task is safely
terminated.
@codecov-commenter
Copy link

Codecov Report

Merging #402 (6f107a6) into main (7f6b2b8) will not change coverage.
The diff coverage is n/a.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@           Coverage Diff           @@
##             main     #402   +/-   ##
=======================================
  Coverage   15.15%   15.15%           
=======================================
  Files          48       48           
  Lines        2778     2778           
=======================================
  Hits          421      421           
  Misses       2328     2328           
  Partials       29       29           

Copy link
Collaborator

@GustavBaumgart GustavBaumgart left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Collaborator

@jaemin-shin jaemin-shin left a comment

Choose a reason for hiding this comment

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

LGTM!

@myungjin myungjin merged commit 80963e4 into cisco-open:main May 1, 2023
@myungjin myungjin deleted the channel_remove_revision branch May 1, 2023 18:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants