Skip to content

Use channels to do the write coordination in the writerCoalescer struct #1192

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

Closed
wants to merge 2 commits into from

Conversation

AlexisMontagne
Copy link

Related to #1191

buffers = append(buffers, w.buf)
resChs = append(resChs, w.resCh)
default:
done = true
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 just break here? Why separate done handling?

Copy link
Author

Choose a reason for hiding this comment

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

"break" refer to the closest "for", "switch", or "select". It would never exit the for loop L636

Copy link
Contributor

Choose a reason for hiding this comment

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

oh well, of course...don't know what I was thinking, ignore :-)

Choose a reason for hiding this comment

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

you can label the loop and then break the loop from the select, e.g.:

loop:
	for {

and

		default:
			break loop

Copy link
Contributor

Choose a reason for hiding this comment

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

@nussjustin true, but in this case that would hardly improve readability.

conn_test.go Outdated
return
case <-t.C:
w.flush()
}
}
}()

if buf.Len() != 0 {
Copy link
Contributor

Choose a reason for hiding this comment

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

Now as you're writing and flushing concurrently this check doesn't really make sense...is it possible to move the write operations before the flusher?

What we basically want with this check is to make sure that writes are coalesced and happen only after the flushing, and not right away.

Thanks!

@Zariel
Copy link
Contributor

Zariel commented Sep 20, 2018

I thought it would be simpler to use another cond to conditionally start the flush loop #1194

@Zariel Zariel closed this Sep 23, 2018
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