More hints?

[iartarisi]

Hey,

First off, thanks a lot for this project. It's a great help for learning exactly those things that I'm interested in learning more about with go!

Now I'm new to the language and I don't know most of the good practices and sometimes not even the basic concurrency primitives. I've gone through the first three problems and have some solutions to them in my own fork, but I don't know if the solutions I come up with are idiomatic, or even decent. It would be great if there was a way to find a hint for solutions, that was somehow hidden away from the main readme. Maybe something as basic as a spoiler.md file that would contain something like: "you can do this with three lines of code using a Mutex. Btw, you don't need to put the mutex around the whole function, just lines 11-13 works".

What do you think about including some sort of hints to help newbies know that they're on the right path and not just hacking poorly-thought solutions?

[DavidWittman]

I won't say my solutions are idiomatic or even 100% correct, but you can find them at https://github.com/DavidWittman/go-concurrency-exercises/tree/solutions if you want to compare to your own.

But :+1: to more hints in general.

[arashout]

Why not have a solutions folder as well? That way it would be easier to discuss whether a solution is idiomatic or not.

[kennykarnama]

Hi @DavidWittman , it is nice to have your solutions, so i can cross check about my answer. But, something is strange on the exercise 2-race-in-cache, may be because the additional code line

val = k.load(key)
        k.cache[key] = val
        k.pages.PushFront(key)

so just want to mention, (correct me if i am wrong), by using your solution, race condition still happends. So, i found that by adding mutex on beginning of the Get func like this :

    k.mut.Lock()
    defer k.mut.Unlock()
    val, ok := k.Cache[key]

    // Miss - load from database and save it in cache

    if !ok {
        val = k.Load(key)
        k.Cache[key] = val
        k.Pages.PushFront(key)

will solve the race condition

[DavidWittman]

@kennykarnama I haven't looked at this in a while, but I think I was only locking around the write to the cache, as indicated in the comment for that exercise:

Multiple readers are fine, but multiple writers are not.

My concurrency tests (with go run -race *.go) still succeed for me, but I'm on an older version of Go and a pretty old CPU, so I suppose there may be some situations in which it fails.

[ivanthewebber]

@kennykarnama your solution does solve the race condition in @DavidWittman's solution, but it also prevents all concurrency in that area of code. Having multiple thread-safe readers and non-thread-safe writer(s) is a common problem. You'll see the RWMutex supports this situation. Multiple threads can access areas protected by Rlock() & RUnlock(), but only one thread at a time can access an area protected by Lock() & Unlock(). To elaborate: when Lock() is called the current thread blocks until all threads that called Rlock() have called RUnlock() and any attempt to call Rlock() blocks until Unlock() is called.

I'm just learning this myself, but I hope it helps!

// Get gets the key from cache, loads it from the source if needed
func (k *KeyStoreCache) Get(key string) string {
    k.rw.RLock()
    val, ok := k.cache[key] // read is thread-safe
    k.rw.RUnlock() // defer here would cause a deadlock on miss (waiting on self)

    // Miss - load from database and save it in cache
    if !ok {
        k.rw.Lock()
        defer k.rw.Unlock()

        /* If multiple readers had requested the missing page each
        one would attempt to load it leading to duplicates. 
        Thus we should check to see if the resource was already loaded. */

        // catch baby swipers
        val, ok := k.cache[key]
        if ok {
            return val
        }

        val = k.load(key)

        k.cache[key] = val

        k.pages.PushFront(key)

        // if cache is full remove the least used item
        if len(k.cache) > CacheSize {
            delete(k.cache, k.pages.Back().Value.(string))
            k.pages.Remove(k.pages.Back())
        }

    }

    return val
}

[abshkbh]

@ivanthewebber your solutions also has a race. Please correct me if I'm wrong.

• Imagine the cache is full.
• Two threads T1 and T2 try to Get("key"). "key" isn't present in the cache.
• Both run concurrently (grabbing RLock one by one) and reach "if !ok"
• From this point on T1 inserts "key" in cache and removes LRU item from cache.
• T2 also does the same. So we ended up removing an extra item in the cache (2 items in total).
• Locking the whole function while inefficient prevents this scenario and would ensure T1 inserting key in cache and T2 "hitting" the cache when it tries to look up "key".

Any time there is a "test-and-set", it probably warrants doing the test and set atomically. Some cases given the domain and your definition of correctness, you can get way with more granular locking.

Just my 2 cents. Happy to discuss more.