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

proposal: bufio: Writer.SetFlushTimeout method #20353

Closed
rgooch opened this issue May 13, 2017 · 7 comments
Closed

proposal: bufio: Writer.SetFlushTimeout method #20353

rgooch opened this issue May 13, 2017 · 7 comments
Labels
FeatureRequest FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. Proposal
Milestone

Comments

@rgooch
Copy link

rgooch commented May 13, 2017

Proposal: add a Writer.SetFlushTimeout method which will set a timeout to flush the buffer after a period of inactivity.

Rationale: for performance reasons it's common to not call Flush after every Write. However, in some applications (such as writing messages to a connection or logfiles), timely flushing of data will improve responsiveness or ensure more log data are written prior to a fatal error/panic.

Proposed method signature:
func (w *Writer) SetFlushTimeout(timeout time.Duration) {}

@bradfitz
Copy link
Contributor

See https://golang.org/doc/faq#x_in_std

This is only a dozen some lines of code for people who need this. Does this already exist on GitHub?

We're well past just adding things to the standard library because we can. We prefer things to exist not in the standard library. Would this be used by anything already in the standard library?

@bradfitz bradfitz added FeatureRequest NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. labels May 13, 2017
@bradfitz bradfitz added this to the Unplanned milestone May 13, 2017
@rgooch
Copy link
Author

rgooch commented May 13, 2017

I don't understand why the threshold should be "would be be used in the standard library". That seems pretty circular.

@bradfitz
Copy link
Contributor

It's just a pretty good proxy for whether it'd also be useful outside of the standard library. There's enough code in the standard library to get an idea of whether it'd be useful.

See recent #20136 for a similar discussion.

@bradfitz
Copy link
Contributor

There's already an implementation of this of sorts in the standard library:

https://golang.org/src/net/http/httputil/reverseproxy.go

func (p *ReverseProxy) copyResponse(dst io.Writer, src io.Reader) {
        if p.FlushInterval != 0 {
                if wf, ok := dst.(writeFlusher); ok {
                        mlw := &maxLatencyWriter{
                                dst:     wf,
                                latency: p.FlushInterval,
                                done:    make(chan bool),
                        }
                        go mlw.flushLoop()
                        defer mlw.stop()
                        dst = mlw
                }
        }

....

type maxLatencyWriter struct { 
        dst     writeFlusher 
        latency time.Duration 
 
        mu   sync.Mutex // protects Write + Flush                                                                                                                        
        done chan bool 
} 
 
func (m *maxLatencyWriter) Write(p []byte) (int, error) { 
        m.mu.Lock() 
        defer m.mu.Unlock() 
        return m.dst.Write(p) 
} 
 
func (m *maxLatencyWriter) flushLoop() { 
        t := time.NewTicker(m.latency) 
        defer t.Stop() 
        for { 
                select { 
                case <-m.done: 
                        if onExitFlushLoop != nil { 
                                onExitFlushLoop() 
                        } 
                        return 
                case <-t.C: 
                        m.mu.Lock() 
                        m.dst.Flush() 
                        m.mu.Unlock() 
                } 
        } 
}

func (m *maxLatencyWriter) stop() { m.done <- true }

It's not quite the same, but pretty close.

So that's ~one caller.

Where else needs this, even outside the standard library? Do many people end up implementing this? Is there a current popular implementation people use?

@robpike
Copy link
Contributor

robpike commented May 13, 2017

I'd be pretty unhappy to see the notion of time introduced into a low-level package like bufio.

@robpike robpike closed this as completed May 13, 2017
@robpike robpike reopened this May 13, 2017
@rsc
Copy link
Contributor

rsc commented May 22, 2017

This is easily done in a third-party package, and it's far from clear we want to put time into bufio. Still waiting for any rationale about usage level.

@rsc rsc changed the title bufio: Add Writer.SetFlushTimeout method proposal: bufio: Writer.SetFlushTimeout method May 22, 2017
@rsc rsc modified the milestones: Proposal, Unplanned May 22, 2017
@rsc
Copy link
Contributor

rsc commented Jun 5, 2017

No objections raised, so closing.

@rsc rsc closed this as completed Jun 5, 2017
@golang golang locked and limited conversation to collaborators Jun 5, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FeatureRequest FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. Proposal
Projects
None yet
Development

No branches or pull requests

5 participants