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

Explain how filewatch and other examples ensure at most one concurrent writer #87

Closed
stevenh opened this issue Oct 14, 2015 · 12 comments
Closed

Comments

@stevenh
Copy link

stevenh commented Oct 14, 2015

While its not broken filewatch should really use WriteControl to send Ping messages.
https://github.com/gorilla/websocket/blob/master/examples/filewatch/main.go#L106

@garyburd
Copy link
Contributor

Why? The concurrency of WriteControl is not needed.

@stevenh
Copy link
Author

stevenh commented Oct 14, 2015

Best practice?

We used this as an example of how two add a pinger using websocket and it caused the issue as per: #83

@garyburd
Copy link
Contributor

garyburd commented Oct 14, 2015

Best practice in this case is to use WriteMessage.

@garyburd garyburd changed the title filewatch uses WriteMessage for ping Explain how filewatch and other examples ensure at most a single concurrent writer Oct 22, 2015
@garyburd garyburd changed the title Explain how filewatch and other examples ensure at most a single concurrent writer Explain how filewatch and other examples ensure at most one concurrent writer Oct 22, 2015
@garyburd
Copy link
Contributor

I am looking for someone to help on this issue.

@hetao29
Copy link

hetao29 commented Jun 23, 2017

Hi, garyburd, we are use this library for more than 1000 users chartroom program, concurrent writing is needed, 1000 users * 20 messsages per seconds, we must put this worker into goroutine, please tell me ,how to best practice.

Now I fork it and add writerlocker and readerlocker, but it's not the best way I think.

@garyburd
Copy link
Contributor

@hetao29 Use a sync.Mutex in your application to ensure that only one goroutine calls the write methods concurrently or use a single goroutine for all writes as is done in the chat example.

@hetao29
Copy link

hetao29 commented Jul 3, 2017

Hi, @GaryBud, In your chat example, high concurrent broadcast is problematic, a network delay of w.Write (message) (line 102, file client.go) will block all subsequent w.Write (message)

If you want to broadcast to 1000+ people at the same time:

"a single goroutine for all writes", if a person's network delay, will lead to all other people delay or block like your chat example;

"one goroutine calls the write methods concurrently", but will lead to a large number The goroutine generation, in the stress test, the memory and cpu will continue to rise.

@elithrar
Copy link
Contributor

elithrar commented Jul 4, 2017

@hetao29 Are you observing that?

Note that the chat example spins up a readPump and writePump per connection and then pushes messages into each clients' send channel - https://github.com/gorilla/websocket/blob/master/examples/chat/hub.go#L45

This channel is buffered - and the loop over all clients does not block based on a receiver.

@hetao29
Copy link

hetao29 commented Jul 6, 2017

@elithrar Yes, I have observing the buffered channel at client.go
&Client{hub: hub, conn: conn, send: make(chan []byte, 256)}
Theoretically more than 256, it will block, but I actually tested, this situation is still not easy to appear, thank you much!

@garyburd
Copy link
Contributor

garyburd commented Jul 8, 2017

@hetao29 Because hub uses select with default when sending the channel, the hub does not block in theory or in practice.

@ghost
Copy link

ghost commented Oct 2, 2019

The package documentation covers everything that a user of the package needs to know regarding concurrency. Is more documentation required?

@garyburd
Copy link
Contributor

The file watch example, the original subject of this issue, is correct. The concurrency requirements for the package is documented in the package comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants