calling file.Write() concurrently

[Pierre Durand]

Hello!

My code: https://play.golang.org/p/pg-p17UuEW

I'm trying to append lines to a file concurrently.

My first write() function is buggy, because WriteString() are called by several goroutines in unexpected order.

So, I've written writeMutex(), that uses a mutex.

It works as expected, but in my real code, I'd prefer to avoid mutex.

(in my real code write() can write to different files; the file's name is given as argument)

I've also written writeBuffer().

Is it OK to call f.Write() 1 time from several goroutines?

It looks like it works, but I'm not 100% sure this is correct.

[Dave Cheney]

Why do you want to avoid a mutex?

[Konstantin Khomoutov]

It's hard for me to discern your real problem from your statement, but
I'll do a shot in the dark: if your concern is that having a single
mutex will possibly create unnecessary contention when writing in
separate files, just have a separate mutex per file. If the entry
point to your writing machinery receives the file's name as its
argument, have a map which assigns mutexes to file names. Be sure to
also delete the matching entry from this map when a particular file is
finished dealing with.

You might also consider a different approach.
Say, you might have a manager goroutine running forever and doing two
things: maintaining a list of active file writes and an input channel
receiving tasks in the form of tuples (file_name, data_to_write).
If it detects there's no in progress write operation happens for the
file from the next task, it spawns a goroutine writing data to that
file and creates an entry in its internal map marking that file as
"busy". It then waits for the signal of that working goroutine that
it has finished work and removes the "busy mark".

Yet another approach is to have such manager goroutine fork a
long-running worker goroutine per named file: from each incoming task
it fetches the file's name and submits its data to write to appropriate
worker goroutine on a dedicated channel.

But before you "go parallel" take into account that writing data to
files is an I/O-bound operation and if your files reside on the same
filesystem (and especially if that filesystem is backed by rotating
medium), or on different filesystem backed by the same meduim, you
probably won't get the speedup you're seeking: the write operations
will be naturally serialized "back" by the filesystem layer of your OS.
Hence rather than have N "parallel" writes been stalled waiting on the
OS to cope with them, it might have more sense to just have a
single dedicated writing goroutine which will receive "writing
tasks" (see above) and perform they in FIFO order.

To reiterate: being able to perform certain operations in parallel or
concurrently is cool but always keep in mind that after "leaving" the
runtime they hit CPU and OS where things suddenly stop being "pure",
theoretical and ideal.

[Alex Bligh]

> On 5 Jul 2016, at 10:38, Pierre Durand <pierre...@gmail.com> wrote:
>
> Hello!
>
> My code: https://play.golang.org/p/pg-p17UuEW
> I'm trying to append lines to a file concurrently.
>
> My first write() function is buggy, because WriteString() are called by several goroutines in unexpected order.
>
> So, I've written writeMutex(), that uses a mutex.
> It works as expected, but in my real code, I'd prefer to avoid mutex.
> (in my real code write() can write to different files; the file's name is given as argument)

Use one mutex per file then, and make your write() lock the correct mutex. Note that if you have a map of mutexes, you may also need to protect that map with a mutex, but that only needs protecting whilst you access the map with the possibility of a concurrent write; however, be aware of the danger of lock inversion.

--
Alex Bligh

[Alex Bligh]

> On 5 Jul 2016, at 10:38, Pierre Durand <pierre...@gmail.com> wrote:
>

> So, I've written writeMutex(), that uses a mutex.
> It works as expected, but in my real code, I'd prefer to avoid mutex.
> (in my real code write() can write to different files; the file's name is given as argument)

... or more idiomatically, set up a channel per file you are writing, make write() write to that channel, and have a writer go-routine which reads from that channel and writes to the file itself.

--
Alex Bligh

[Pierre Durand]

@Dave Cheney: I want to avoid a global mutex, because my function can write to different files.

@Konstantin Khomoutov: You are right, I could use a map of mutexes.

As Alex Bligh said, I also need to add a mutex on this map, which is a bit complicated...

I'm not using concurrency to write faster.

I have several goroutines (started by HTTP request) that append strings to the same file.

@Konstantin Khomoutov + @Alex Bligh: I like this idea about using a channel + goroutine for each file.

It would also help me to not open/close file for each write.

My real question is: is there a problem in the writeBuffer() function ?

It is very simple, and it looks that it works.

[Dave Cheney]

A better solution would be to compose a new Writer wrapping the existing one with a mutex.

[Konstantin Khomoutov]

On Tue, 5 Jul 2016 05:46:17 -0700 (PDT)
Pierre Durand <pierre...@gmail.com> wrote:

[...]
> My real question is: is there a problem in the *writeBuffer()*

> function ? It is very simple, and it looks that it works.

I'd not reinvent the wheel and use bufio.NewWriter() to wrap your file
object to a buffered writer. The only possible "gotcha" is that once
you start using a buffered writer you have to call Flush() on it to
actually let the data hit the OS, and you have to check that method for
returning an error value just like you do with Write().

So, I'd change your writeBuffer() to something like

import (
"bufio"
)

func writeBuffer(s string) (err error) {
f, err := os.OpenFile("log.txt",
os.O_APPEND|os.O_WRONLY| os.O_CREATE, 0644) if err != nil {
return err
}
defer f.Close()

bw := bufio.NewWriter(f)
_, err = bw.WriteString(s)
if err != nil {
return
}
_, err = bw.WriteString("\n")
if err != nil {
return
}
return bw.Flush()
}

Please also note how deferred call to close the file is just f.Close()
instead of some weird dance involving a closure. ;-)

On the other hand, take into account that closing a file opened for
writing _may_ fail under certain circumstances (see `man 2 open`) so if
you're writing something robust you might want to have more involved
error checking strategy.

[Jan Mercl]

On Tue, Jul 5, 2016 at 11:38 AM Pierre Durand <pierre...@gmail.com> wrote:

> It works as expected, but in my real code, I'd prefer to avoid mutex.

Sounds like a case for using a writer goroutine and sending it the should-be-atomic log items via a chan string or chan []byte.

--

-j

[Pierre Durand]

My solution: https://play.golang.org/p/f5H0svLFE0

What do you think ?

[Pierre Durand]

Ooops, fixed my code: https://play.golang.org/p/vdwDT9WMVr

[Tamás Gulácsi]

Seems ok.

[Daniel K]

Looks good, but you could take advantage of the io writer interface.

I made a quick example, not sure if it runs (did not test)

https://play.golang.org/p/cYcz2ktoiG

[Pierre Durand]

@Tamás Gulácsi: thank you for your review.

@Daniel K: yes, it was my original idea, but I quickly discarded it, because I can't guarantee that LineAppender is safe for concurrent usage.

If you give it a wrong io.WriteCloser, it will be unsafe to use concurrently.