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

Make subchannels be ordered #1261

Closed
wants to merge 4 commits into from
Closed

Make subchannels be ordered #1261

wants to merge 4 commits into from

Conversation

ibc
Copy link
Member

@ibc ibc commented Dec 12, 2023

Details

  • Use an ordered set in DataConsumer in worker so subchannels will be always shown in numerical order (no need to sort them later in Node/Rust layer).
  • Also make dataConsumer.subchannels be immediately (and optimistically) updated before dataChannel.setSubchannels() completes.

ibc added 2 commits December 12, 2023 12:49
@ibc
Make subchannels be ordered
3d54749 
### Details

- Use an ordered set in `DataConsumer` in worker so `subchannels` will be always shown in numerical order (no need to sort them later in Node/Rust layer).
- Also make `dataConsumer.subchannels` be **immediately** (and optimistically) updated before `dataChannel.setSubchannels()` completes.
@ibc
Update CHANGELOG
be42f16
@ibc
Copy link
Member Author

ibc commented Dec 12, 2023

I'm closing this PR since, as @jmillan said, adding addSubchannel() and emoveSubchannel() makes more sense. Also, in other async methods (such as pause()) we don't update this.paused until we get the response fro the worker, so same must be done in setSubchannels().

@ibc ibc closed this Dec 12, 2023
@ibc ibc deleted the make-subchannels-be-ordered branch December 12, 2023 16:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jmillan jmillan jmillan approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ibc @jmillan