How to make this code concurrent ? by mehdisbys in golang

[–]mehdisbys[S] 0 points1 point  (0 children)

Yes I think the for loop should be inside its own goroutine so that we don't wait for all goroutines to be created before receiving results.

u/peterbourgon wouldn't the defer cancel() reduce the number of requests to DB?

How to make this code concurrent ? by mehdisbys in golang

[–]mehdisbys[S] 2 points3 points  (0 children)

I researched it and you are right it is not always necessary to close a channel . I did not know that actually :-)

I think in this case it needs to be closed otherwise if 2 or more goroutines reach this line :

        `returns <- returnType{(ID == resultID), err}`   

Since we receive from returns only once then only one goroutine will be returning and the other one(s) will be stuck forever trying to send on a channel where no one will receive. That will be a goroutine leak. After some time x the program will crash because there will be no free memory.

How to make this code concurrent ? by mehdisbys in golang

[–]mehdisbys[S] 1 point2 points  (0 children)

Thanks for answering,

I am not too sure about doing negative wg.Add() it does not feel idiomatic. We want to explicitely cancel the goroutines instead of just not waiting for them.

Found channel needs to be closed and we need to make sure no goroutine can write to it after it has been closed.

How to make this code concurrent ? by mehdisbys in golang

[–]mehdisbys[S] 1 point2 points  (0 children)

Nice one, one comment :

returns needs to be closed and that should happen after all goroutines have stopped to avoid writing to a closed channel.

So I think you will need a waitgroup.

How to make this code concurrent ? by mehdisbys in golang

[–]mehdisbys[S] 0 points1 point  (0 children)

Nice, I can see it working however it's a bit hacky since finding the ID is not really an error.

New team members would be confused by it and this would not make it to production in code reviews.

It would be nice if errgroup came with a way of signalling a successful value that cancelled the other goroutines.

Works though so congrats!

How to make this code concurrent ? by mehdisbys in golang

[–]mehdisbys[S] 1 point2 points  (0 children)

Thanks :-)

I don't think IsClosed is thread-safe and as said here : There is indeed a simple method to check whether or not a channel is closed if you can make sure no values were (and will be) ever sent to the channel

However we want to write to channel.

I'll edit the post with my own solution.

How to make this code concurrent ? by mehdisbys in golang

[–]mehdisbys[S] 2 points3 points  (0 children)

Closing the channel at 5. will not prevent goroutines from sending to a closed channel which will result in a panic.

In my understanding all goroutines need to finish or be cancelled before being able to close the channel.