Skip to content

Conversation

@gammazero
Copy link
Contributor

It is possible for changes to get our of order when they are submitted by goroutines for non-blocking operations. Instead of goroutines, use an asynchronous queue to maintain order of events.

This may prevent provlems caused by mis-ordering of events shuch as connect and disconnect events.

@gammazero gammazero requested a review from a team as a code owner July 18, 2025 03:19
@codecov
Copy link

codecov bot commented Jul 18, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 61.53%. Comparing base (2c5f80a) to head (c15e74f).
Report is 4 commits behind head on main.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #981      +/-   ##
==========================================
+ Coverage   61.51%   61.53%   +0.01%     
==========================================
  Files         254      254              
  Lines       31494    31490       -4     
==========================================
+ Hits        19373    19376       +3     
+ Misses      10533    10527       -6     
+ Partials     1588     1587       -1     
Files with missing lines Coverage Δ
...tswap/client/internal/session/sessionwantsender.go 97.33% <100.00%> (+0.45%) ⬆️

... and 9 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Contributor

@guillaumemichel guillaumemichel left a comment

Choose a reason for hiding this comment

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

It makes sense!

for range sws.changes.Len() {
select {
case next := <-sws.changes:
case next := <-sws.changes.Out():
Copy link
Contributor

Choose a reason for hiding this comment

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

@gammazero you wrote the library so you probably know better.

Are there any benefits of doing like above?

changes := sws.changes.Out()
for range sws.changes.Len() {
	select {
	case next := <-changes:
...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe slightly? The function call should get inlined anyway, so is it worth the extra line of code? 🤷

@hsanjuan hsanjuan merged commit 2cf77c4 into main Jul 18, 2025
13 checks passed
@gammazero gammazero deleted the queue-changes branch September 10, 2025 00:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants